[Misc] Improve type inference on PositionalTagManager#addTag (#6676)

* Improve type inference on `PositionalTagManager#addTag`

- Remove unused `AddPositionalTagAttr`

* Improved tests

* fixed type errors

* Update move.ts

Co-authored-by: NightKev <34855794+DayKev@users.noreply.github.com>

---------

Co-authored-by: NightKev <34855794+DayKev@users.noreply.github.com>

src/data/moves/move.ts

@@ -3384,7 +3384,10 @@ export class WeatherInstantChargeAttr extends InstantChargeAttr {
   }
 }
 
-export class OverrideMoveEffectAttr extends MoveAttr {
+/**
+ * Abstract class used for `MoveAttr`s whose effect application can override normal move effect processing.
+ */
+abstract class OverrideMoveEffectAttr extends MoveAttr {
   /** 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.
    */
@@ -3404,41 +3407,37 @@ export class OverrideMoveEffectAttr extends MoveAttr {
   }
 }
 
-/** Abstract class for moves that add {@linkcode PositionalTag}s to the field. */
-abstract class AddPositionalTagAttr extends OverrideMoveEffectAttr {
-  protected abstract readonly tagType: PositionalTagType;
-
-  public override getCondition(): MoveConditionFunc {
-    // Check the arena if another similar positional tag is active and affecting the same slot
-    return (_user, target, move) => globalScene.arena.positionalTagManager.canAddTag(this.tagType, target.getBattlerIndex())
-  }
-}
-
 /**
  * Attribute to implement delayed attacks, such as {@linkcode MoveId.FUTURE_SIGHT} or {@linkcode MoveId.DOOM_DESIRE}.
  * Delays the attack's effect with a {@linkcode DelayedAttackTag},
  * activating against the given slot after the given turn count has elapsed.
  */
 export class DelayedAttackAttr extends OverrideMoveEffectAttr {
-  public chargeAnim: ChargeAnim;
-  private chargeText: string;
-
   /**
-   * @param chargeAnim - The {@linkcode ChargeAnim | charging animation} used for the move's charging phase.
+   * The {@linkcode ChargeAnim | charging animation} used for the move's charging phase.
-   * @param chargeKey - The `i18next` locales **key** to show when the delayed attack is used.
+   *
+   * Rendered public to allow for charge animation code to function
+   */
+  public readonly chargeAnim: ChargeAnim;
+  /**
+   * The `i18next` locales key to show when the delayed attack is queued
+   * (**not** when it activates)! \
    * In the displayed text, `{{pokemonName}}` will be populated with the user's name.
    */
+  private readonly chargeKey: string;
+
   constructor(chargeAnim: ChargeAnim, chargeKey: string) {
     super();
 
     this.chargeAnim = chargeAnim;
-    this.chargeText = chargeKey;
+    this.chargeKey = chargeKey;
   }
 
   public override apply(user: Pokemon, target: Pokemon, move: Move, args: [overridden: BooleanHolder, useMode: MoveUseMode]): boolean {
     const useMode = args[1];
     if (useMode === MoveUseMode.DELAYED_ATTACK) {
       // don't trigger if already queueing an indirect attack
+      // TODO: There should be a cleaner way of doing this...
       return false;
     }
 
@@ -3449,14 +3448,14 @@ export class DelayedAttackAttr extends OverrideMoveEffectAttr {
     globalScene.phaseManager.unshiftNew("MoveAnimPhase", new MoveChargeAnim(this.chargeAnim, move.id, user));
     globalScene.phaseManager.queueMessage(
       i18next.t(
-        this.chargeText,
+        this.chargeKey,
         { pokemonName: getPokemonNameWithAffix(user) }
       )
     )
 
     user.pushMoveHistory({move: move.id, targets: [target.getBattlerIndex()], result: MoveResult.OTHER, useMode, turn: globalScene.currentBattle.turn})
-    // Queue up an attack on the given slot.
+    // Queue up an attack on the given slot
-    globalScene.arena.positionalTagManager.addTag<PositionalTagType.DELAYED_ATTACK>({
+    globalScene.arena.positionalTagManager.addTag({
       tagType: PositionalTagType.DELAYED_ATTACK,
       sourceId: user.id,
       targetIndex: target.getBattlerIndex(),
@@ -3474,11 +3473,12 @@ export class DelayedAttackAttr extends OverrideMoveEffectAttr {
 
 /**
  * Attribute to queue a {@linkcode WishTag} to activate in 2 turns.
- * The tag whill heal
+ * The tag will heal whichever Pokemon remains in the given slot for 50% of the user's
+ * maximum HP.
  */
 export class WishAttr extends MoveEffectAttr {
-  public override apply(user: Pokemon, target: Pokemon, _move: Move): boolean {
+  public override apply(user: Pokemon, target: Pokemon): boolean {
-    globalScene.arena.positionalTagManager.addTag<PositionalTagType.WISH>({
+    globalScene.arena.positionalTagManager.addTag({
       tagType: PositionalTagType.WISH,
       healHp: toDmgValue(user.getMaxHp() / 2),
       targetIndex: target.getBattlerIndex(),
@@ -3489,7 +3489,7 @@ export class WishAttr extends MoveEffectAttr {
   }
 
   public override getCondition(): MoveConditionFunc {
-    // Check the arena if another wish is active and affecting the same slot
+    // Check the arena if another similar move is active and affecting the same slot
     return (_user, target) => globalScene.arena.positionalTagManager.canAddTag(PositionalTagType.WISH, target.getBattlerIndex())
   }
 }

src/data/positional-tags/load-positional-tag.ts

@@ -14,7 +14,7 @@ import type { ObjectValues } from "#types/type-helpers";
 export function loadPositionalTag<T extends PositionalTagType>({
   tagType,
   ...args
-}: serializedPosTagMap[T]): posTagInstanceMap[T];
+}: toSerializedPosTag<T>): posTagInstanceMap[T];
 /**
  * Load the attributes of a {@linkcode PositionalTag}.
  * @param tag - The {@linkcode SerializedPositionalTag} to instantiate
@@ -26,7 +26,7 @@ export function loadPositionalTag(tag: SerializedPositionalTag): PositionalTag;
 export function loadPositionalTag<T extends PositionalTagType>({
   tagType,
   ...rest
-}: serializedPosTagMap[T]): posTagInstanceMap[T] {
+}: toSerializedPosTag<T>): posTagInstanceMap[T] {
   // Note: We need 2 type assertions here:
   // 1 because TS doesn't narrow the type of TagClass correctly based on `T`.
   // It converts it into `new (DelayedAttackTag | WishTag) => DelayedAttackTag & WishTag`
@@ -58,12 +58,19 @@ type posTagParamMap = {
   [k in PositionalTagType]: ConstructorParameters<posTagMap[k]>[0];
 };
 
+/**
+ * Generic type to convert a {@linkcode PositionalTagType} into the serialized representation of its corresponding class instance.
+ *
+ * Used in place of a mapped type to work around Typescript deficiencies in function type signatures.
+ */
+export type toSerializedPosTag<T extends PositionalTagType> = posTagParamMap[T] & { readonly tagType: T };
+
 /**
  * Type mapping all positional tag types to their constructors' parameters, alongside the `tagType` selector.
  * Equivalent to their serialized representations.
  */
-export type serializedPosTagMap = {
+type serializedPosTagMap = {
-  [k in PositionalTagType]: posTagParamMap[k] & { tagType: k };
+  [k in PositionalTagType]: toSerializedPosTag<k>;
 };
 
 /** Union type containing all serialized {@linkcode PositionalTag}s. */

src/data/positional-tags/positional-tag-manager.ts

@@ -1,4 +1,4 @@
-import { loadPositionalTag } from "#data/positional-tags/load-positional-tag";
+import { loadPositionalTag, type toSerializedPosTag } from "#data/positional-tags/load-positional-tag";
 import type { PositionalTag } from "#data/positional-tags/positional-tag";
 import type { BattlerIndex } from "#enums/battler-index";
 import type { PositionalTagType } from "#enums/positional-tag-type";
@@ -16,7 +16,7 @@ export class PositionalTagManager {
    * @remarks
    * This function does not perform any checking if the added tag is valid.
    */
-  public addTag<T extends PositionalTagType = never>(tag: Parameters<typeof loadPositionalTag<T>>[0]): void {
+  public addTag<T extends PositionalTagType>(tag: toSerializedPosTag<T>): void {
     this.tags.push(loadPositionalTag(tag));
   }
 

src/data/positional-tags/positional-tag.ts

@@ -20,7 +20,7 @@ import i18next from "i18next";
  * and should refrain from adding extra serializable fields not contained in said interface.
  * This ensures that all tags truly "become" their respective interfaces when converted to and from JSON.
  */
-export interface PositionalTagBaseArgs {
+interface PositionalTagBaseArgs {
   /**
    * The number of turns remaining until this tag's activation. \
    * Decremented by 1 at the end of each turn until reaching 0, at which point it will
@@ -30,16 +30,16 @@ export interface PositionalTagBaseArgs {
   /**
    * The {@linkcode BattlerIndex} targeted by this effect.
    */
-  targetIndex: BattlerIndex;
+  readonly targetIndex: BattlerIndex;
 }
 
 /**
  * A {@linkcode PositionalTag} is a variant of an {@linkcode ArenaTag} that targets a single *slot* of the battlefield.
- * Each tag can last one or more turns, triggering various effects on removal.
+ * Each tag can last one or more turns, triggering various effects on removal. \
  * Multiple tags of the same kind can stack with one another, provided they are affecting different targets.
  */
 export abstract class PositionalTag implements PositionalTagBaseArgs {
-  /** This tag's {@linkcode PositionalTagType | type} */
+  /** This tag's {@linkcode PositionalTagType | type}. */
   public abstract readonly tagType: PositionalTagType;
   // These arguments have to be public to implement the interface, but are functionally private
   // outside this and the tag manager.
@@ -76,9 +76,9 @@ interface DelayedAttackArgs extends PositionalTagBaseArgs {
   /**
    * The {@linkcode Pokemon.id | PID} of the {@linkcode Pokemon} having created this effect.
    */
-  sourceId: number;
+  readonly sourceId: number;
   /** The {@linkcode MoveId} that created this attack. */
-  sourceMove: MoveId;
+  readonly sourceMove: MoveId;
 }
 
 /**
@@ -88,6 +88,7 @@ interface DelayedAttackArgs extends PositionalTagBaseArgs {
  */
 export class DelayedAttackTag extends PositionalTag implements DelayedAttackArgs {
   public override readonly tagType = PositionalTagType.DELAYED_ATTACK;
+
   public readonly sourceMove: MoveId;
   public readonly sourceId: number;
 
@@ -135,9 +136,9 @@ export class DelayedAttackTag extends PositionalTag implements DelayedAttackArgs
 /** Interface containing arguments used to construct a {@linkcode WishTag}. */
 interface WishArgs extends PositionalTagBaseArgs {
   /** The amount of {@linkcode Stat.HP | HP} to heal; set to 50% of the user's max HP during move usage. */
-  healHp: number;
+  readonly healHp: number;
   /** The name of the {@linkcode Pokemon} having created the tag. */
-  pokemonName: string;
+  readonly pokemonName: string;
 }
 
 /**

test/moves/delayed-attack.test.ts

@@ -67,17 +67,6 @@ describe("Moves - Delayed Attacks", () => {
     }
   }
 
-  /**
-   * Expect that future sight is active with the specified number of attacks.
-   * @param numAttacks - The number of delayed attacks that should be queued; default `1`
-   */
-  function expectFutureSightActive(numAttacks = 1) {
-    const delayedAttacks = game.scene.arena.positionalTagManager["tags"].filter(
-      t => t.tagType === PositionalTagType.DELAYED_ATTACK,
-    );
-    expect(delayedAttacks).toHaveLength(numAttacks);
-  }
-
   it.each<{ name: string; move: MoveId }>([
     { name: "Future Sight", move: MoveId.FUTURE_SIGHT },
     { name: "Doom Desire", move: MoveId.DOOM_DESIRE },
@@ -88,7 +77,12 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(move);
     await game.toNextTurn();
 
-    expectFutureSightActive();
+    expect(game).toHavePositionalTag({
+      tagType: PositionalTagType.DELAYED_ATTACK,
+      sourceMove: move,
+      targetIndex: BattlerIndex.ENEMY,
+      turnCount: 2,
+    });
 
     game.doSwitchPokemon(1);
     game.forceEnemyToSwitch();
@@ -96,7 +90,7 @@ describe("Moves - Delayed Attacks", () => {
 
     await passTurns(1);
 
-    expectFutureSightActive(0);
+    expect(game).not.toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
     const enemy = game.field.getEnemyPokemon();
     expect(enemy).not.toHaveFullHp();
     expect(game).toHaveShownMessage(
@@ -113,14 +107,14 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.FUTURE_SIGHT);
     await game.toNextTurn();
 
-    expectFutureSightActive();
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
     const bronzong = game.field.getPlayerPokemon();
     expect(bronzong.getLastXMoves()[0].result).toBe(MoveResult.OTHER);
 
     game.move.use(MoveId.FUTURE_SIGHT);
     await game.toNextTurn();
 
-    expectFutureSightActive();
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
     expect(bronzong.getLastXMoves()[0].result).toBe(MoveResult.FAIL);
   });
 
@@ -131,13 +125,13 @@ describe("Moves - Delayed Attacks", () => {
     game.move.forceMetronomeMove(MoveId.FUTURE_SIGHT);
     await game.toNextTurn();
 
-    expectFutureSightActive();
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
     const enemy = game.field.getEnemyPokemon();
     expect(enemy).toHaveFullHp();
 
     await passTurns(2);
 
-    expectFutureSightActive(0);
+    expect(game).not.toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
     expect(enemy).not.toHaveFullHp();
   });
 
@@ -151,7 +145,7 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.FUTURE_SIGHT, BattlerIndex.PLAYER_2, BattlerIndex.ENEMY_2);
     await game.toEndOfTurn();
 
-    expectFutureSightActive(2);
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK, 2);
     expect(enemy1).toHaveFullHp();
     expect(enemy2).toHaveFullHp();
     expect(karp.getLastXMoves()[0].result).toBe(MoveResult.OTHER);
@@ -159,6 +153,7 @@ describe("Moves - Delayed Attacks", () => {
 
     await passTurns(2);
 
+    expect(game).not.toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
     expect(enemy1).not.toHaveFullHp();
     expect(enemy2).not.toHaveFullHp();
   });
@@ -179,7 +174,7 @@ describe("Moves - Delayed Attacks", () => {
     await game.setTurnOrder(oldOrder.map(p => p.getBattlerIndex()));
     await game.toNextTurn();
 
-    expectFutureSightActive(4);
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK, 4);
 
     // Lower speed to change turn order
     alomomola.setStatStage(Stat.SPD, 6);
@@ -191,7 +186,7 @@ describe("Moves - Delayed Attacks", () => {
     await passTurns(2, false);
 
     // All attacks have concluded at this point, unshifting new `MoveEffectPhase`s to the queue.
-    expectFutureSightActive(0);
+    expect(game).not.toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
 
     const MEPs = game.scene.phaseManager["phaseQueue"].findAll("MoveEffectPhase");
     expect(MEPs).toHaveLength(4);
@@ -208,7 +203,7 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.SPLASH, BattlerIndex.PLAYER_2);
     await game.toNextTurn();
 
-    expectFutureSightActive(1);
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK, 1);
 
     // Milotic / Feebas // Karp
     game.doSwitchPokemon(2);
@@ -246,7 +241,7 @@ describe("Moves - Delayed Attacks", () => {
     await game.toNextTurn();
 
     expect(enemy2.isFainted()).toBe(true);
-    expectFutureSightActive();
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
 
     expect(game).toHavePositionalTag({
       tagType: PositionalTagType.DELAYED_ATTACK,
@@ -273,7 +268,7 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.FUTURE_SIGHT, BattlerIndex.PLAYER, BattlerIndex.ENEMY_2);
     await game.toNextTurn();
 
-    expectFutureSightActive(1);
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK, 1);
 
     game.move.use(MoveId.SPLASH);
     await game.killPokemon(enemy2);
@@ -282,7 +277,7 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.SPLASH);
     await game.toNextTurn();
 
-    expectFutureSightActive(0);
+    expect(game).not.toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
     expect(enemy1).toHaveFullHp();
     expect(game).not.toHaveShownMessage(
       i18next.t("moveTriggers:tookMoveAttack", {
@@ -303,7 +298,7 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.FUTURE_SIGHT, BattlerIndex.PLAYER, BattlerIndex.ENEMY_2);
     await game.toNextTurn();
 
-    expectFutureSightActive(1);
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK, 1);
 
     game.move.use(MoveId.SPLASH, BattlerIndex.PLAYER);
     await game.toNextTurn();
@@ -341,7 +336,7 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.DOOM_DESIRE);
     await game.toNextTurn();
 
-    expectFutureSightActive();
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
 
     await passTurns(1);
 
@@ -371,7 +366,7 @@ describe("Moves - Delayed Attacks", () => {
     game.move.use(MoveId.FUTURE_SIGHT);
     await game.toNextTurn();
 
-    expectFutureSightActive();
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
 
     await passTurns(1);
 
@@ -401,7 +396,7 @@ describe("Moves - Delayed Attacks", () => {
     await game.move.forceEnemyMove(MoveId.FUTURE_SIGHT);
     await game.toNextTurn();
 
-    expectFutureSightActive(1);
+    expect(game).toHavePositionalTag(PositionalTagType.DELAYED_ATTACK, 1);
 
     await passTurns(1);
 
@@ -412,7 +407,7 @@ describe("Moves - Delayed Attacks", () => {
     });
     await game.toEndOfTurn();
 
-    expectFutureSightActive(0);
+    expect(game).not.toHavePositionalTag(PositionalTagType.DELAYED_ATTACK);
   });
 
   // TODO: Implement and move to a power spot's test file

test/test-utils/matchers/to-have-positional-tag.ts

@@ -2,7 +2,7 @@
 import type { GameManager } from "#test/test-utils/game-manager";
 // biome-ignore-end lint/correctness/noUnusedImports: TSDoc
 
-import type { serializedPosTagMap } from "#data/positional-tags/load-positional-tag";
+import type { toSerializedPosTag } from "#data/positional-tags/load-positional-tag";
 import type { PositionalTagType } from "#enums/positional-tag-type";
 import type { OneOther } from "#test/@types/test-helpers";
 import { getOnelineDiffStr } from "#test/test-utils/string-utils";
@@ -10,9 +10,10 @@ import { isGameManagerInstance, receivedStr } from "#test/test-utils/test-utils"
 import { toTitleCase } from "#utils/strings";
 import type { MatcherState, SyncExpectationResult } from "@vitest/expect";
 
-export type toHavePositionalTagOptions<P extends PositionalTagType> = OneOther<serializedPosTagMap[P], "tagType"> & {
+/**
-  tagType: P;
+ * Options type for {@linkcode toHavePositionalTag}.
-};
+ */
+export type toHavePositionalTagOptions<P extends PositionalTagType> = OneOther<toSerializedPosTag<P>, "tagType">;
 
 /**
  * Matcher to check if the {@linkcode Arena} has a certain number of {@linkcode PositionalTag}s active.

test/types/positional-tags.test-d.ts

@@ -1,28 +1,27 @@
-import type { SerializedPositionalTag, serializedPosTagMap } from "#data/positional-tags/load-positional-tag";
+import type { SerializedPositionalTag, toSerializedPosTag } from "#data/positional-tags/load-positional-tag";
 import type { DelayedAttackTag, WishTag } from "#data/positional-tags/positional-tag";
 import type { PositionalTagType } from "#enums/positional-tag-type";
-import type { Mutable, NonFunctionPropertiesRecursive } from "#types/type-helpers";
+import type { NonFunctionPropertiesRecursive } from "#types/type-helpers";
 import { describe, expectTypeOf, it } from "vitest";
 
-// Needed to get around properties being readonly in certain classes
+describe("toSerializedPosTag", () => {
-type NonFunctionMutable<T> = Mutable<NonFunctionPropertiesRecursive<T>>;
+  it("should map each class' tag type to their serialized forms", () => {
-
+    expectTypeOf<toSerializedPosTag<PositionalTagType.DELAYED_ATTACK>>().branded.toEqualTypeOf<
-describe("serializedPositionalTagMap", () => {
+      NonFunctionPropertiesRecursive<DelayedAttackTag>
-  it("should contain representations of each tag's serialized form", () => {
+    >();
-    expectTypeOf<serializedPosTagMap[PositionalTagType.DELAYED_ATTACK]>().branded.toEqualTypeOf<
+    expectTypeOf<toSerializedPosTag<PositionalTagType.WISH>>().branded.toEqualTypeOf<
-      NonFunctionMutable<DelayedAttackTag>
+      NonFunctionPropertiesRecursive<WishTag>
     >();
-    expectTypeOf<serializedPosTagMap[PositionalTagType.WISH]>().branded.toEqualTypeOf<NonFunctionMutable<WishTag>>();
   });
 });
 
 describe("SerializedPositionalTag", () => {
-  it("should accept a union of all serialized tag forms", () => {
+  it("should be a union of all serialized tag forms", () => {
     expectTypeOf<SerializedPositionalTag>().branded.toEqualTypeOf<
-      NonFunctionMutable<DelayedAttackTag> | NonFunctionMutable<WishTag>
+      NonFunctionPropertiesRecursive<DelayedAttackTag> | NonFunctionPropertiesRecursive<WishTag>
     >();
   });
-  it("should accept a union of all unserialized tag forms", () => {
+  it("should be extended by all unserialized tag forms", () => {
     expectTypeOf<WishTag>().toExtend<SerializedPositionalTag>();
     expectTypeOf<DelayedAttackTag>().toExtend<SerializedPositionalTag>();
   });