From 37af6d9c525c64fc8d3db0f7b27a8daeff8681b9 Mon Sep 17 00:00:00 2001 From: Sirz Benjie <142067137+SirzBenjie@users.noreply.github.com> Date: Mon, 9 Jun 2025 14:12:26 -0500 Subject: [PATCH] Use InstanceType for proper narrowing --- src/@types/move-types.ts | 32 ++++++++++++++++++----- src/data/abilities/ability.ts | 14 +++------- src/data/battle-anims.ts | 11 ++++---- src/data/moves/move.ts | 45 ++++++++++++++++++++++----------- src/phases/move-effect-phase.ts | 3 +-- 5 files changed, 65 insertions(+), 40 deletions(-) diff --git a/src/@types/move-types.ts b/src/@types/move-types.ts index 7aeb70876d2..28d603d21bb 100644 --- a/src/@types/move-types.ts +++ b/src/@types/move-types.ts @@ -14,21 +14,39 @@ export type * from "#app/data/moves/move"; /** * Map of move subclass names to their respective classes. + * Does not include the ChargeMove subclasses. For that, use `ChargingMoveClassMap`. + * + * @privateremarks + * The `Never` field is necessary to ensure typescript does not improperly narrow a failed `is` guard to `never`. + * + * For example, if we did not have the never, and wrote + * ``` + * function Foo(move: Move) { + * if (move.is("AttackMove")) { + * + * } else if (move.is("StatusMove")) { // typescript errors on the `is`, saying that `move` is `never` + * + * } + * ``` */ export type MoveClassMap = { - AttackMove: typeof AttackMove; - StatusMove: typeof StatusMove; - SelfStatusMove: typeof SelfStatusMove; - ChargingAttackMove: typeof ChargingAttackMove; - ChargingSelfStatusMove: typeof ChargingSelfStatusMove; - ChargeMove: typeof ChargingAttackMove | typeof ChargingSelfStatusMove; + AttackMove: AttackMove; + StatusMove: StatusMove; + SelfStatusMove: SelfStatusMove; }; +/* + * Without the `Never: never` field, typescript will + */ + /** * Union type of all move subclass names */ -export type MoveClass = keyof MoveClassMap; +export type MoveKindString = "AttackMove" | "StatusMove" | "SelfStatusMove"; +/** + * Map of move attribute names to attribute instances. + */ export type MoveAttrMap = { [K in keyof MoveAttrConstructorMap]: InstanceType; }; diff --git a/src/data/abilities/ability.ts b/src/data/abilities/ability.ts index c4fef7a8c29..128e772217f 100644 --- a/src/data/abilities/ability.ts +++ b/src/data/abilities/ability.ts @@ -2527,17 +2527,11 @@ export class AllyStatMultiplierAbAttr extends AbAttr { * @extends AbAttr */ export class ExecutedMoveAbAttr extends AbAttr { - canApplyExecutedMove( - _pokemon: Pokemon, - _simulated: boolean, - ): boolean { + canApplyExecutedMove(_pokemon: Pokemon, _simulated: boolean): boolean { return true; } - applyExecutedMove( - _pokemon: Pokemon, - _simulated: boolean, - ): void {} + applyExecutedMove(_pokemon: Pokemon, _simulated: boolean): void {} } /** @@ -2545,7 +2539,7 @@ export class ExecutedMoveAbAttr extends AbAttr { * @extends ExecutedMoveAbAttr */ export class GorillaTacticsAbAttr extends ExecutedMoveAbAttr { - constructor(showAbility: boolean = false) { + constructor(showAbility = false) { super(showAbility); } @@ -7773,7 +7767,7 @@ export function applyPreAttackAbAttrs( export function applyExecutedMoveAbAttrs( attrType: Constructor, pokemon: Pokemon, - simulated: boolean = false, + simulated = false, ...args: any[] ): void { applyAbAttrsInternal( diff --git a/src/data/battle-anims.ts b/src/data/battle-anims.ts index 8aaae66e954..0b40469b255 100644 --- a/src/data/battle-anims.ts +++ b/src/data/battle-anims.ts @@ -446,12 +446,11 @@ export function initMoveAnim(move: MoveId): Promise { } } else { moveAnims.set(move, null); - const defaultMoveAnim = - allMoves[move].is("AttackMove") - ? MoveId.TACKLE - : allMoves[move].is("SelfStatusMove") - ? MoveId.FOCUS_ENERGY - : MoveId.TAIL_WHIP; + const defaultMoveAnim = allMoves[move].is("AttackMove") + ? MoveId.TACKLE + : allMoves[move].is("SelfStatusMove") + ? MoveId.FOCUS_ENERGY + : MoveId.TAIL_WHIP; const fetchAnimAndResolve = (move: MoveId) => { globalScene diff --git a/src/data/moves/move.ts b/src/data/moves/move.ts index 4b6c805f1aa..57660b51391 100644 --- a/src/data/moves/move.ts +++ b/src/data/moves/move.ts @@ -123,7 +123,7 @@ import { MoveEffectTrigger } from "#enums/MoveEffectTrigger"; import { MultiHitType } from "#enums/MultiHitType"; import { invalidAssistMoves, invalidCopycatMoves, invalidMetronomeMoves, invalidMirrorMoveMoves, invalidSleepTalkMoves } from "./invalid-moves"; import { SelectBiomePhase } from "#app/phases/select-biome-phase"; -import { ChargingMove, MoveAttrMap, MoveAttrString, MoveClass, MoveClassMap } from "#app/@types/move-types"; +import { ChargingMove, MoveAttrMap, MoveAttrString, MoveKindString, MoveClassMap } from "#app/@types/move-types"; import { applyMoveAttrs } from "./apply-attrs"; import { frenzyMissFunc, getMoveTargets } from "./move-utils"; @@ -153,10 +153,13 @@ export default abstract class Move implements Localizable { /** * Check if the move is of the given subclass without requiring `instanceof`. * + * ⚠️ Does _not_ work for {@linkcode ChargingAttackMove} and {@linkcode ChargingSelfStatusMove} subclasses. For those, + * use {@linkcode isChargingMove} instead. + * * @param moveKind - The string name of the move to check against * @returns Whether this move is of the provided type. */ - public abstract is(moveKind: K): this is MoveClassMap[K]; + public abstract is(moveKind: K): this is MoveClassMap[K]; constructor(id: MoveId, type: PokemonType, category: MoveCategory, defaultMoveTarget: MoveTarget, power: number, accuracy: number, pp: number, chance: number, priority: number, generation: number) { this.id = id; @@ -976,6 +979,10 @@ export default abstract class Move implements Localizable { } export class AttackMove extends Move { + /** This field does not exist at runtime and must not be used. + * Its sole purpose is to ensure that typescript is able to properly narrow when the `is` method is called. + */ + declare private _: never; override is(moveKind: K): this is MoveClassMap[K] { return moveKind === "AttackMove"; } @@ -1026,21 +1033,29 @@ export class AttackMove extends Move { } export class StatusMove extends Move { + /** This field does not exist at runtime and must not be used. + * Its sole purpose is to ensure that typescript is able to properly narrow when the `is` method is called. + */ + declare private _: never; constructor(id: MoveId, type: PokemonType, accuracy: number, pp: number, chance: number, priority: number, generation: number) { super(id, type, MoveCategory.STATUS, MoveTarget.NEAR_OTHER, -1, accuracy, pp, chance, priority, generation); } - override is(moveKind: K): this is MoveClassMap[K] { + override is(moveKind: K): this is MoveClassMap[K] { return moveKind === "StatusMove"; } } export class SelfStatusMove extends Move { + /** This field does not exist at runtime and must not be used. + * Its sole purpose is to ensure that typescript is able to properly narrow when the `is` method is called. + */ + declare private _: never; constructor(id: MoveId, type: PokemonType, accuracy: number, pp: number, chance: number, priority: number, generation: number) { super(id, type, MoveCategory.STATUS, MoveTarget.USER, -1, accuracy, pp, chance, priority, generation); } - override is(moveKind: K): this is MoveClassMap[K] { + override is(moveKind: K): this is MoveClassMap[K] { return moveKind === "SelfStatusMove"; } } @@ -1067,11 +1082,6 @@ function ChargeMove(Base: TBase, nameAppend: string) { return true; } - override is(moveKind: K): this is MoveClassMap[K] { - // Anything subclassing this is a charge move. - return moveKind === "ChargeMove" || moveKind === nameAppend || super.is(moveKind); - } - /** * Sets the text to be displayed during this move's charging phase. * References to the user Pokemon should be written as "{USER}", and @@ -1151,14 +1161,14 @@ export abstract class MoveAttr { public selfTarget: boolean; /** - * Returns whether this attribute is of the given type. - * + * Return whether this attribute is of the given type. + * * @remarks * Used to avoid requring the caller to have imported the specific attribute type, avoiding circular dependencies. * @param attr - The attribute to check against - * @returns Whether the attribute is an instanceof the given type. + * @returns Whether the attribute is an instance of the given type. */ - public is(attr: T): this is MoveAttrConstructorMap[MoveAttrString] { + public is(attr: T): this is MoveAttrMap[T] { const targetAttr = MoveAttrs[attr]; if (!targetAttr) { return false; @@ -3096,7 +3106,11 @@ export class WeatherInstantChargeAttr extends InstantChargeAttr { } export class OverrideMoveEffectAttr extends MoveAttr { - apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean { + /** This field does not exist at runtime and must not be used. + * Its sole purpose is to ensure that typescript is able to properly narrow when the `is` method is called. + */ + declare private _: never; + override apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean { return true; } } @@ -8401,7 +8415,8 @@ const MoveAttrs = Object.freeze({ ResistLastMoveTypeAttr, ExposedMoveAttr, }); -/** Map of names */ + +/** Map of of move attribute names to their constructors */ export type MoveAttrConstructorMap = typeof MoveAttrs; export const selfStatLowerMoves: MoveId[] = []; diff --git a/src/phases/move-effect-phase.ts b/src/phases/move-effect-phase.ts index 739f8a323eb..68c96ddce1e 100644 --- a/src/phases/move-effect-phase.ts +++ b/src/phases/move-effect-phase.ts @@ -28,7 +28,6 @@ import { } from "#app/data/battler-tags"; import { BattlerTagLapseType } from "#enums/battler-tag-lapse-type"; import type { MoveAttr } from "#app/data/moves/move"; -import { MoveEffectAttr } from "#app/data/moves/move"; import { getMoveTargets } from "#app/data/moves/move-utils"; import { applyFilteredMoveAttrs, applyMoveAttrs } from "#app/data/moves/apply-attrs"; import { MoveEffectTrigger } from "#enums/MoveEffectTrigger"; @@ -746,7 +745,7 @@ export class MoveEffectPhase extends PokemonPhase { ): void { applyFilteredMoveAttrs( (attr: MoveAttr) => - attr instanceof MoveEffectAttr && + attr.is("MoveEffectAttr") && attr.trigger === triggerType && (isNullOrUndefined(selfTarget) || attr.selfTarget === selfTarget) && (!attr.firstHitOnly || this.firstHit) &&