From 72f834c9463704ed76b83c313b8d2e2a6b4f3ad9 Mon Sep 17 00:00:00 2001 From: Bertie690 Date: Tue, 27 May 2025 12:20:54 -0400 Subject: [PATCH] Applied PR reviews --- src/battle-scene.ts | 4 +- src/data/abilities/ability.ts | 52 +++++++++++-------- src/data/arena-tag.ts | 5 +- src/data/moves/move.ts | 36 +++++++------ .../utils/encounter-pokemon-utils.ts | 2 +- src/field/arena.ts | 2 - src/field/pokemon.ts | 10 ++-- src/modifier/modifier.ts | 22 ++------ 8 files changed, 63 insertions(+), 70 deletions(-) diff --git a/src/battle-scene.ts b/src/battle-scene.ts index 9def79c333d..d0379c5bf3c 100644 --- a/src/battle-scene.ts +++ b/src/battle-scene.ts @@ -875,7 +875,7 @@ export default class BattleScene extends SceneBase { * Returns an array of Pokemon on both sides of the battle - player first, then enemy. * Does not actually check if the pokemon are on the field or not, and always has length 4 regardless of battle type. * @param activeOnly - Whether to consider only active pokemon (as described by {@linkcode Pokemon.isActive()}); default `false`. - * If `true`, will also elide all `null` values from the array. + * If `true`, will also remove all `null` values from the array. * @returns An array of {@linkcode Pokemon}, as described above. */ public getField(activeOnly = false): Pokemon[] { @@ -3354,6 +3354,8 @@ export default class BattleScene extends SceneBase { return (player ? this.modifiers : this.enemyModifiers).filter((m): m is T => m instanceof modifierType); } + // TODO: Add overload signature to `findModifier` and `findModifiers` for functions of the form `(x): X is T` + /** * Get all of the modifiers that pass the `modifierFilter` function * @param modifierFilter - The function used to filter a target's modifiers diff --git a/src/data/abilities/ability.ts b/src/data/abilities/ability.ts index 60f3f194c21..3e3fbde7793 100644 --- a/src/data/abilities/ability.ts +++ b/src/data/abilities/ability.ts @@ -1202,7 +1202,10 @@ export class FieldPreventExplosiveMovesAbAttr extends AbAttr { export class FieldMultiplyStatAbAttr extends AbAttr { private stat: Stat; private multiplier: number; - /** Whether this ability can stack with others of the same type for this stat; default `false` */ + /** + * Whether this ability can stack with others of the same type for this stat. + * @defaultValue `false` + */ private canStack: boolean; constructor(stat: Stat, multiplier: number, canStack = false) { @@ -1214,8 +1217,12 @@ export class FieldMultiplyStatAbAttr extends AbAttr { } canApplyFieldStat(_pokemon: Pokemon, _passive: boolean, _simulated: boolean, stat: Stat, _statValue: NumberHolder, checkedPokemon: Pokemon, hasApplied: BooleanHolder, _args: any[]): boolean { - return (this.canStack || !hasApplied.value) - && this.stat === stat && checkedPokemon.getAbilityAttrs(FieldMultiplyStatAbAttr).every(attr => (attr as FieldMultiplyStatAbAttr).stat !== stat); + return ( + (this.canStack || !hasApplied.value) + && this.stat === stat + // targets with the same stat-changing ability as this are unaffected + && checkedPokemon.getAbilityAttrs(FieldMultiplyStatAbAttr).every(attr => attr.stat !== stat) + ); } /** @@ -1246,7 +1253,7 @@ export class MoveTypeChangeAbAttr extends PreAttackAbAttr { /** * Determine if the move type change attribute can be applied. - * + * * Can be applied if: * - The ability's condition is met, e.g. pixilate only boosts normal moves, * - The move is not forbidden from having its type changed by an ability, e.g. {@linkcode Moves.MULTI_ATTACK} @@ -1341,37 +1348,35 @@ export class PokemonTypeChangeAbAttr extends PreAttackAbAttr { /** * Class for abilities that add additional strikes to single-target moves. * Used by {@linkcode Moves.PARENTAL_BOND | Parental Bond}. + * + * @param damageMultiplier - The damage multiplier for the added strike, relative to the first. */ export class AddSecondStrikeAbAttr extends PreAttackAbAttr { private damageMultiplier: number; - - /** - * @param damageMultiplier - The damage multiplier for the added strike, relative to the first. - */ constructor(damageMultiplier: number) { super(false); this.damageMultiplier = damageMultiplier; } - override canApplyPreAttack(pokemon: Pokemon, passive: boolean, simulated: boolean, defender: Pokemon | null, move: Move, args: any[]): boolean { + override canApplyPreAttack(pokemon: Pokemon, _passive: boolean, _simulated: boolean, _defender: Pokemon | null, move: Move, _args: [NumberHolder?, NumberHolder?]): boolean { return move.canBeMultiStrikeEnhanced(pokemon, true); } /** - * Applies the ability attribute by increasing hit count + * Increases the move's hit count or modifies the move's damage. * @param pokemon - The {@linkcode Pokemon} using the move * @param passive - Unused * @param defender - Unused * @param move - The {@linkcode Move} being used - * @param args: + * @param args - * - `[0]` - A {@linkcode NumberHolder} holding the move's current strike count * - `[1]` - A {@linkcode NumberHolder} holding the current strike's damage multiplier. */ - // TODO: Why can these be nullish? - override applyPreAttack(pokemon: Pokemon, passive: boolean, simulated: boolean, defender: Pokemon, move: Move, args: any[]): void { - const hitCount = args[0] as NumberHolder; - const multiplier = args[1] as NumberHolder; + // TODO: Review if these args can be undefined when called (and remove the ? if not) + override applyPreAttack(pokemon: Pokemon, _passive: boolean, _simulated: boolean, _defender: Pokemon, _move: Move, args: [NumberHolder?, NumberHolder?]): void { + const hitCount = args[0]; + const multiplier = args[1]; if (hitCount?.value) { hitCount.value += 1; } @@ -3147,10 +3152,11 @@ export class ProtectStatAbAttr extends PreStatStageChangeAbAttr { } /** - * Attribute to apply confusion to the target whenever the user - * directly statuses them with a move. + * Attribute to apply confusion to the target whenever the user directly statuses them with a move. + * * Used by {@linkcode Abilities.POISON_PUPPETEER} - * Called in {@linkcode StatusEffectAttr}. + * + * Called in {@linkcode StatusEffectAttr} * @extends PostAttackAbAttr * @see {@linkcode applyPostAttack} */ @@ -3338,8 +3344,8 @@ export class ConditionalUserFieldProtectStatAbAttr extends PreStatStageChangeAbA * @param simulated - Unused * @param stat - The {@linkcode Stat} being affected * @param cancelled - {@linkcode BooleanHolder} containing whether the stat change was already prevented - * @param args `[0]` - The {@linkcode Pokemon} recieving the stat change - * @returns `true` if the ability can be applied + * @param args - `[0]`: The {@linkcode Pokemon} receiving the stat change + * @returns `true` if the ability can be applied */ override canApplyPreStatStageChange(pokemon: Pokemon, passive: boolean, simulated: boolean, stat: BattleStat, cancelled: BooleanHolder, args: [Pokemon]): boolean { const target = args[0]; @@ -3534,9 +3540,9 @@ export class ConditionalCritAbAttr extends AbAttr { * @param simulated - Unused * @param cancelled - Unused * @param args - - * - [0] A {@linkcode BooleanHolder} containing whether critical hits are guaranteed. - * - [1] The {@linkcode Pokemon} being targeted (unused) - * - [2] The {@linkcode Move} used by the ability holder (unused) + * - `[0]` A {@linkcode BooleanHolder} containing whether critical hits are guaranteed. + * - `[1]` The {@linkcode Pokemon} being targeted (unused) + * - `[2]` The {@linkcode Move} used by the ability holder (unused) */ override apply(pokemon: Pokemon, passive: boolean, simulated: boolean, cancelled: BooleanHolder, args: [BooleanHolder, Pokemon, Move, ...any]): void { (args[0] as BooleanHolder).value = true; diff --git a/src/data/arena-tag.ts b/src/data/arena-tag.ts index 4ddac7e63a8..fa74ba60bbf 100644 --- a/src/data/arena-tag.ts +++ b/src/data/arena-tag.ts @@ -69,9 +69,9 @@ export abstract class ArenaTag { /** * Trigger this {@linkcode ArenaTag}'s effect, reducing its duration as applicable. - * Will ignore duration of all tags with durations alwrea + * Will ignore durations of all tags with durations `<=0`. * @param _arena - The {@linkcode Arena} at the moment the tag is being lapsed. - * Unused by default but can be used by super classes. + * Unused by default but can be used by sub-classes. * @returns `true` if this tag should be kept; `false` if it should be removed. */ lapse(_arena: Arena): boolean { @@ -881,7 +881,6 @@ class ToxicSpikesTag extends ArenaTrapTag { * Arena Tag class for delayed attacks, such as {@linkcode Moves.FUTURE_SIGHT} or {@linkcode Moves.DOOM_DESIRE}. * Delays the attack's effect by a set amount of turns, usually 3 (including the turn the move is used), * and deals damage after the turn count is reached. - // TODO: Add class for tags that can have multiple instances up and edit `arena.addTag` appropriately */ export class DelayedAttackTag extends ArenaTag { public targetIndex: BattlerIndex; diff --git a/src/data/moves/move.ts b/src/data/moves/move.ts index f06397d0596..281ad564ea4 100644 --- a/src/data/moves/move.ts +++ b/src/data/moves/move.ts @@ -201,7 +201,7 @@ export default class Move implements Localizable { /** * Check if a move has an attribute that matches `attrType` * @param attrType - The constructor of a {@linkcode MoveAttr} - * @returns Whether if the move has an attribute that is/extends `attrType` + * @returns Whether this move has an attribute matching `attrType` */ hasAttr(attrType: Constructor): boolean { return this.attrs.some((attr) => attr instanceof attrType); @@ -209,11 +209,13 @@ export default class Move implements Localizable { /** * Find the first attribute that matches a given predicate function. - * @param attrPredicate - The predicate function to search `MoveAttr`s by. - * @returns The first {@linkcode MoveAttr} for which `attrPredicate` returns a value coercible to the boolean value `true`. + * @param attrPredicate - The predicate function to search `MoveAttr`s by. + * @returns The first {@linkcode MoveAttr} for which `attrPredicate` returns `true`. */ findAttr(attrPredicate: (attr: MoveAttr) => boolean): MoveAttr { - return this.attrs.find(attrPredicate)!; // TODO: is the bang correct? + // TODO: Remove bang and make return type `MoveAttr | undefined`, + // as well as add overload for functions of type `x is T` + return this.attrs.find(attrPredicate)!; } /** @@ -241,7 +243,7 @@ export default class Move implements Localizable { * Adds a new MoveAttr to this move (appends to the attr array). * If the MoveAttr also comes with a condition, it is added to its {@linkcode MoveCondition} array. * - * Almost identical to {@linkcode attr}, except taking already instantized {@linkcode MoveAttr} object + * Similar to {@linkcode attr}, except this takes an already instantiated {@linkcode MoveAttr} object * as opposed to a constructor and its arguments. * @param attrAdd - The {@linkcode MoveAttr} to add * @returns `this` @@ -371,7 +373,7 @@ export default class Move implements Localizable { /** * Adds a condition to this move (in addition to any provided by its prior {@linkcode MoveAttr}s). - * The move will fail upon use if at least 1 of its conditions is not met. + * The move will fail upon use if at least 1 of its conditions is not met. * @param condition - The {@linkcode MoveCondition} or {@linkcode MoveConditionFunc} to add to the conditions array. * @returns `this` */ @@ -386,8 +388,8 @@ export default class Move implements Localizable { /** * Mark a move as having one or more edge cases. - * The move may lack certain niche interactions with other moves/abilities, but still functions - * as intended in most cases. + * The move may lack certain niche interactions with other moves/abilities, + * but still functions as intended in most cases. * * When using this, **make sure to document the edge case** (or else this becomes pointless). * @returns `this` @@ -397,8 +399,8 @@ export default class Move implements Localizable { } /** - * Mark a move as partially implemented. - * Partial moves are expected to have their core functionality implemented, but may lack + * Mark this move as partially implemented. + * Partial moves are expected to have some core functionality implemented, but may lack * certain notable features or interactions with other moves or abilities. * @returns `this` */ @@ -408,9 +410,9 @@ export default class Move implements Localizable { } /** - * Mark a move as unimplemented. + * Mark this move as unimplemented. * Unimplemented moves are ones which have _none_ of their basic functionality enabled, - * and are effectively barred from use by the AI. + * and will fail upon use. * @returns `this` */ unimplemented(): this { @@ -976,10 +978,8 @@ export class AttackMove extends Move { constructor(id: Moves, type: PokemonType, category: MoveCategory, power: number, accuracy: number, pp: number, chance: number, priority: number, generation: number) { super(id, type, category, MoveTarget.NEAR_OTHER, power, accuracy, pp, chance, priority, generation); - /** - * {@link https://bulbapedia.bulbagarden.net/wiki/Freeze_(status_condition)} - * All damaging Fire-type moves can thaw a frozen target, regardless of whether or not they have a chance to burn - */ + // > All damaging Fire-type moves can... thaw a frozen target, regardless of whether or not they have a chance to burn. + // - https://bulbapedia.bulbagarden.net/wiki/Freeze_(status_condition) if (this.type === PokemonType.FIRE) { this.addAttr(new HealStatusEffectAttr(false, StatusEffect.FREEZE)); } @@ -2865,6 +2865,8 @@ export class HealStatusEffectAttr extends MoveEffectAttr { * Attribute to add the {@linkcode BattlerTagType.BYPASS_SLEEP | BYPASS_SLEEP Battler Tag} for 1 turn to the user before move use. * Used by {@linkcode Moves.SNORE} and {@linkcode Moves.SLEEP_TALK}. */ +// TODO: Should this use a battler tag? +// TODO: Give this `userSleptOrComatoseCondition` export class BypassSleepAttr extends MoveAttr { apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean { if (user.status?.effect === StatusEffect.SLEEP) { @@ -2882,7 +2884,7 @@ export class BypassSleepAttr extends MoveAttr { * @param move */ getUserBenefitScore(user: Pokemon, target: Pokemon, move: Move): number { - return user.status && user.status.effect === StatusEffect.SLEEP ? 200 : -10; + return user.status?.effect === StatusEffect.SLEEP ? 200 : -10; } } diff --git a/src/data/mystery-encounters/utils/encounter-pokemon-utils.ts b/src/data/mystery-encounters/utils/encounter-pokemon-utils.ts index 243eab58b27..39d753bd816 100644 --- a/src/data/mystery-encounters/utils/encounter-pokemon-utils.ts +++ b/src/data/mystery-encounters/utils/encounter-pokemon-utils.ts @@ -404,7 +404,7 @@ export async function applyModifierTypeToPlayerPokemon( // Check if the Pokemon has max stacks of that item already const modifier = modType.newModifier(pokemon); const existing = globalScene.findModifier( - m => + (m): m is PokemonHeldItemModifier => m instanceof PokemonHeldItemModifier && m.type.id === modType.id && m.pokemonId === pokemon.id && diff --git a/src/field/arena.ts b/src/field/arena.ts index bcd1d32586f..f083180490b 100644 --- a/src/field/arena.ts +++ b/src/field/arena.ts @@ -691,7 +691,6 @@ export class Arena { targetIndex?: BattlerIndex, ): boolean { const existingTag = this.getTagOnSide(tagType, side); - // TODO: Change this for future sight if (existingTag) { existingTag.onOverlap(this, globalScene.getPokemonById(sourceId)); @@ -704,7 +703,6 @@ export class Arena { } // creates a new tag object - // TODO: check if we can remove the `|| 0` since turn count should never be nullish const newTag = getArenaTag(tagType, turnCount || 0, sourceMove, sourceId, targetIndex, side); if (newTag) { newTag.onAdd(this, quiet); diff --git a/src/field/pokemon.ts b/src/field/pokemon.ts index 46b6a6771c2..46ebd355d88 100644 --- a/src/field/pokemon.ts +++ b/src/field/pokemon.ts @@ -586,7 +586,7 @@ export default abstract class Pokemon extends Phaser.GameObjects.Container { * @returns The {@linkcode PokeballType} that will be shown when this Pokemon is sent out into battle. */ getPokeball(useIllusion = false): PokeballType { - return useIllusion && this.summonData?.illusion + return useIllusion && this.summonData.illusion ? this.summonData.illusion.pokeball : this.pokeball; } @@ -645,7 +645,7 @@ export default abstract class Pokemon extends Phaser.GameObjects.Container { /** * Checks if a pokemon is fainted (ie: its `hp <= 0`). - * Usually should not be called directly in favor of consulting {@linkcode isAllowedInBattle()}. + * Usually should not be called directly in favor of calling {@linkcode isAllowedInBattle()}. * @param checkStatus - Whether to also check that the pokemon's status is {@linkcode StatusEffect.FAINT}; default `false` * @returns Whether this Pokemon is fainted, as described above. */ @@ -2270,9 +2270,9 @@ export default abstract class Pokemon extends Phaser.GameObjects.Container { /** * Get this Pokemon's non-passive {@linkcode Ability}, factoring in fusions, overrides and ability-changing effects. - * Should rarely be called directky in favor of {@linkcode hasAbility} or {@linkcode hasAbilityWithAttr}, + * Should rarely be called directly in favor of {@linkcode hasAbility} or {@linkcode hasAbilityWithAttr}, * both of which check both ability slots and account for suppression. - * @see {@linkcode hasAbility}and {@linkcode hasAbilityWithAttr} Intended ways to check abilities in most cases + * @see {@linkcode hasAbility} and {@linkcode hasAbilityWithAttr} are the intended ways to check abilities in most cases * @param ignoreOverride - Whether to ignore any overrides caused by {@linkcode Moves.TRANSFORM | Transform}; default `false` * @returns The non-passive {@linkcode Ability} of this Pokemon. */ @@ -6424,7 +6424,7 @@ export default abstract class Pokemon extends Phaser.GameObjects.Container { /** * Reduces one of this Pokemon's held item stacks by 1, removing it if applicable. * Does nothing if this Pokemon is somehow not the owner of the held item. - * @param heldItem The item stack to be reduced. + * @param heldItem - The item stack to be reduced. * @param forBattle - Whether to trigger in-battle effects (such as Unburden) after losing the item. Default: `true` * Should be `false` for all item loss occurring outside of battle (MEs, etc.). * @returns Whether the item was removed successfully. diff --git a/src/modifier/modifier.ts b/src/modifier/modifier.ts index fa8a2d400d3..c653cd70cbe 100644 --- a/src/modifier/modifier.ts +++ b/src/modifier/modifier.ts @@ -62,25 +62,11 @@ const iconOverflowIndex = 24; export const modifierSortFunc = (a: Modifier, b: Modifier): number => { const itemNameMatch = a.type.name.localeCompare(b.type.name); const typeNameMatch = a.constructor.name.localeCompare(b.constructor.name); - const aId = a instanceof PokemonHeldItemModifier && a.pokemonId ? a.pokemonId : 4294967295; - const bId = b instanceof PokemonHeldItemModifier && b.pokemonId ? b.pokemonId : 4294967295; + const aId = a instanceof PokemonHeldItemModifier ? a.pokemonId : -1; + const bId = b instanceof PokemonHeldItemModifier ? b.pokemonId : -1; - // First sort by pokemonID - if (aId < bId) { - return 1; - } - if (aId > bId) { - return -1; - } - if (aId === bId) { - //Then sort by item type - if (typeNameMatch === 0) { - return itemNameMatch; - //Finally sort by item name - } - return typeNameMatch; - } - return 0; + // First sort by pokemon ID, then by item type and then name + return aId - bId || typeNameMatch || itemNameMatch; }; export class ModifierBar extends Phaser.GameObjects.Container {