Fixed instruct interaction with Encore

src/data/abilities/ability.ts

@@ -7296,7 +7296,7 @@ export function initAbilities() {
       .attr(PostSummonMessageAbAttr, (pokemon: Pokemon) => i18next.t("abilityTriggers:postSummonTeravolt", { pokemonNameWithAffix: getPokemonNameWithAffix(pokemon) }))
       .attr(MoveAbilityBypassAbAttr),
     new Ability(AbilityId.AROMA_VEIL, 6)
-      .attr(UserFieldBattlerTagImmunityAbAttr, [ BattlerTagType.INFATUATED, BattlerTagType.TAUNT, BattlerTagType.DISABLED, BattlerTagType.TORMENT, BattlerTagType.HEAL_BLOCK ])
+      .attr(UserFieldBattlerTagImmunityAbAttr, [ BattlerTagType.INFATUATED, BattlerTagType.TAUNT, BattlerTagType.DISABLED, BattlerTagType.TORMENT, BattlerTagType.HEAL_BLOCK, BattlerTagType.ENCORE ])
       .ignorable(),
     new Ability(AbilityId.FLOWER_VEIL, 6)
       .attr(ConditionalUserFieldStatusEffectImmunityAbAttr, (target: Pokemon, source: Pokemon | null) => {

src/data/battler-tags.ts

@@ -1241,11 +1241,20 @@ export class FrenzyTag extends SerializableBattlerTag {
  */
 export class EncoreTag extends MoveRestrictionBattlerTag {
   public override readonly tagType = BattlerTagType.ENCORE;
-  /** The ID of the move the user is locked into using */
+  /** The {@linkcode MoveID} the tag holder is locked into */
   public moveId: MoveId;
 
   constructor(sourceId: number) {
-    super(BattlerTagType.ENCORE, BattlerTagLapseType.TURN_END, 3, MoveId.ENCORE, sourceId);
+    // Encore ends at the end of the 3rd turn it procs.
+    // If used on turn X when faster, it ends at the end of turn X+2.
+    // If used on turn X when slower, it ends at the end of turn X+3.
+    super(
+      BattlerTagType.ENCORE,
+      [BattlerTagLapseType.AFTER_MOVE, BattlerTagLapseType.TURN_END],
+      3,
+      MoveId.ENCORE,
+      sourceId,
+    );
   }
 
   public override loadTag(source: BaseBattlerTag & Pick<EncoreTag, "tagType" | "moveId">): void {
@@ -1267,6 +1276,10 @@ export class EncoreTag extends MoveRestrictionBattlerTag {
       return false;
     }
 
+    if (pokemon.getTag(BattlerTagType.SHELL_TRAP)) {
+      return false;
+    }
+
     this.moveId = lastMove.move;
 
     return true;
@@ -1314,16 +1327,22 @@ export class EncoreTag extends MoveRestrictionBattlerTag {
   }
 
   /**
-   * If the encored move has run out of PP, Encore ends early.
+   * If the encored move has run out of PP or the tag's turn count has elapsed,
-   * Otherwise, Encore's duration reduces at the end of the turn.
+   * Encore ends at the END of the turn.
-   * @returns `true` to persist | `false` to end and be removed
+   * Otherwise, Encore's duration reduces when the target attempts to use a move.
+   * @returns Whether the tag should remain active.
    */
   override lapse(pokemon: Pokemon, lapseType: BattlerTagLapseType): boolean {
+    if (lapseType === BattlerTagLapseType.AFTER_MOVE) {
+      this.turnCount--;
+      return true;
+    }
+
     const encoredMove = pokemon.getMoveset().find(m => m.moveId === this.moveId);
     if (isNullOrUndefined(encoredMove) || encoredMove.isOutOfPp()) {
       return false;
     }
-    return super.lapse(pokemon, lapseType);
+    return this.turnCount > 0;
   }
 
   /**

src/data/moves/invalid-moves.ts

@@ -280,3 +280,68 @@ export const invalidEncoreMoves: ReadonlySet<MoveId> = new Set([
   MoveId.SLEEP_TALK,
   MoveId.ENCORE,
 ]);
+
+export const invalidInstructMoves: ReadonlySet<MoveId> = new Set([
+  // Locking/Continually Executed moves
+  MoveId.OUTRAGE,
+  MoveId.RAGING_FURY,
+  MoveId.ROLLOUT,
+  MoveId.PETAL_DANCE,
+  MoveId.THRASH,
+  MoveId.ICE_BALL,
+  MoveId.UPROAR,
+  // Multi-turn Moves
+  MoveId.BIDE,
+  MoveId.SHELL_TRAP,
+  MoveId.BEAK_BLAST,
+  MoveId.FOCUS_PUNCH,
+  // "First Turn Only" moves
+  MoveId.FAKE_OUT,
+  MoveId.FIRST_IMPRESSION,
+  MoveId.MAT_BLOCK,
+  // Moves with a recharge turn
+  MoveId.HYPER_BEAM,
+  MoveId.ETERNABEAM,
+  MoveId.FRENZY_PLANT,
+  MoveId.BLAST_BURN,
+  MoveId.HYDRO_CANNON,
+  MoveId.GIGA_IMPACT,
+  MoveId.PRISMATIC_LASER,
+  MoveId.ROAR_OF_TIME,
+  MoveId.ROCK_WRECKER,
+  MoveId.METEOR_ASSAULT,
+  // Charging & 2-turn moves
+  MoveId.DIG,
+  MoveId.FLY,
+  MoveId.BOUNCE,
+  MoveId.SHADOW_FORCE,
+  MoveId.PHANTOM_FORCE,
+  MoveId.DIVE,
+  MoveId.ELECTRO_SHOT,
+  MoveId.ICE_BURN,
+  MoveId.GEOMANCY,
+  MoveId.FREEZE_SHOCK,
+  MoveId.SKY_DROP,
+  MoveId.SKY_ATTACK,
+  MoveId.SKULL_BASH,
+  MoveId.SOLAR_BEAM,
+  MoveId.SOLAR_BLADE,
+  MoveId.METEOR_BEAM,
+  // Copying/Move-Calling moves
+  MoveId.ASSIST,
+  MoveId.COPYCAT,
+  MoveId.ME_FIRST,
+  MoveId.METRONOME,
+  MoveId.MIRROR_MOVE,
+  MoveId.NATURE_POWER,
+  MoveId.SLEEP_TALK,
+  MoveId.SNATCH,
+  MoveId.INSTRUCT,
+  // Misc moves
+  MoveId.KINGS_SHIELD,
+  MoveId.SKETCH,
+  MoveId.TRANSFORM,
+  MoveId.MIMIC,
+  MoveId.STRUGGLE,
+  // TODO: Add Max/G-Max/Z-Move blockage if or when they are implemented
+]);

src/data/moves/move.ts

@@ -79,7 +79,7 @@ import {
   PreserveBerryModifier,
 } from "#modifiers/modifier";
 import { applyMoveAttrs } from "#moves/apply-attrs";
-import { invalidAssistMoves, invalidCopycatMoves, invalidMetronomeMoves, invalidMirrorMoveMoves, invalidSketchMoves, invalidSleepTalkMoves } from "#moves/invalid-moves";
+import { invalidAssistMoves, invalidCopycatMoves, invalidInstructMoves, invalidMetronomeMoves, invalidMirrorMoveMoves, invalidSketchMoves, invalidSleepTalkMoves } from "#moves/invalid-moves";
 import { frenzyMissFunc, getMoveTargets } from "#moves/move-utils";
 import { PokemonMove } from "#moves/pokemon-move";
 import { MoveEndPhase } from "#phases/move-end-phase";
@@ -7178,7 +7178,6 @@ export class RepeatMoveAttr extends MoveEffectAttr {
     // bangs are justified as Instruct fails if no prior move or moveset move exists
     // TODO: How does instruct work when copying a move called via Copycat that the user itself knows?
     const lastMove = target.getLastNonVirtualMove()!;
-    const movesetMove = target.getMoveset().find(m => m.moveId === lastMove?.move)!
 
     // If the last move used can hit more than one target or has variable targets,
     // re-compute the targets for the attack (mainly for alternating double/single battles)
@@ -7202,12 +7201,18 @@ export class RepeatMoveAttr extends MoveEffectAttr {
       }
     }
 
+    // If the target is currently affected by Encore, increase its duration by 1 (to offset decrease during move use)
+    const targetEncore = target.getTag(BattlerTagType.ENCORE) as EncoreTag | undefined;
+    if (targetEncore) {
+      targetEncore.turnCount++
+    }
+
     globalScene.phaseManager.queueMessage(i18next.t("moveTriggers:instructingMove", {
       userPokemonName: getPokemonNameWithAffix(user),
       targetPokemonName: getPokemonNameWithAffix(target)
     }));
     target.turnData.extraTurns++;
-    globalScene.phaseManager.appendNewToPhase("MoveEndPhase", "MovePhase", target, moveTargets, movesetMove, MoveUseMode.NORMAL);
+    globalScene.phaseManager.appendNewToPhase("MoveEndPhase", "MovePhase", target, moveTargets, this.movesetMove, MoveUseMode.NORMAL);
     return true;
   }
 
@@ -7216,77 +7221,13 @@ export class RepeatMoveAttr extends MoveEffectAttr {
       // TODO: Check instruct behavior with struggle - ignore, fail or success
       const lastMove = target.getLastNonVirtualMove();
       const movesetMove = target.getMoveset().find(m => m.moveId === lastMove?.move);
-      const uninstructableMoves = [
-        // Locking/Continually Executed moves
-        MoveId.OUTRAGE,
-        MoveId.RAGING_FURY,
-        MoveId.ROLLOUT,
-        MoveId.PETAL_DANCE,
-        MoveId.THRASH,
-        MoveId.ICE_BALL,
-        MoveId.UPROAR,
-        // Multi-turn Moves
-        MoveId.BIDE,
-        MoveId.SHELL_TRAP,
-        MoveId.BEAK_BLAST,
-        MoveId.FOCUS_PUNCH,
-        // "First Turn Only" moves
-        MoveId.FAKE_OUT,
-        MoveId.FIRST_IMPRESSION,
-        MoveId.MAT_BLOCK,
-        // Moves with a recharge turn
-        MoveId.HYPER_BEAM,
-        MoveId.ETERNABEAM,
-        MoveId.FRENZY_PLANT,
-        MoveId.BLAST_BURN,
-        MoveId.HYDRO_CANNON,
-        MoveId.GIGA_IMPACT,
-        MoveId.PRISMATIC_LASER,
-        MoveId.ROAR_OF_TIME,
-        MoveId.ROCK_WRECKER,
-        MoveId.METEOR_ASSAULT,
-        // Charging & 2-turn moves
-        MoveId.DIG,
-        MoveId.FLY,
-        MoveId.BOUNCE,
-        MoveId.SHADOW_FORCE,
-        MoveId.PHANTOM_FORCE,
-        MoveId.DIVE,
-        MoveId.ELECTRO_SHOT,
-        MoveId.ICE_BURN,
-        MoveId.GEOMANCY,
-        MoveId.FREEZE_SHOCK,
-        MoveId.SKY_DROP,
-        MoveId.SKY_ATTACK,
-        MoveId.SKULL_BASH,
-        MoveId.SOLAR_BEAM,
-        MoveId.SOLAR_BLADE,
-        MoveId.METEOR_BEAM,
-        // Copying/Move-Calling moves
-        MoveId.ASSIST,
-        MoveId.COPYCAT,
-        MoveId.ME_FIRST,
-        MoveId.METRONOME,
-        MoveId.MIRROR_MOVE,
-        MoveId.NATURE_POWER,
-        MoveId.SLEEP_TALK,
-        MoveId.SNATCH,
-        MoveId.INSTRUCT,
-        // Misc moves
-        MoveId.KINGS_SHIELD,
-        MoveId.SKETCH,
-        MoveId.TRANSFORM,
-        MoveId.MIMIC,
-        MoveId.STRUGGLE,
-        // TODO: Add Max/G-Max/Z-Move blockage if or when they are implemented
-      ];
 
-      if (!lastMove?.move // no move to instruct
+      if (
+        !lastMove?.move // no move to instruct
         || !movesetMove // called move not in target's moveset (forgetting the move, etc.)
-        || movesetMove.ppUsed === movesetMove.getMovePp() // move out of pp
+        || movesetMove.isOutOfPp() // move out of pp
-        // TODO: This next line is likely redundant as all charging moves are in the above list
+        || invalidInstructMoves.has(lastMove.move) // called move is in the banlist
-        || allMoves[lastMove.move].isChargingMove() // called move is a charging/recharging move
+      ) {
-        || uninstructableMoves.includes(lastMove.move)) { // called move is in the banlist
         return false;
       }
       this.movesetMove = movesetMove;
@@ -9207,11 +9148,11 @@ export function initMoves() {
       .hidesUser(),
     new StatusMove(MoveId.ENCORE, PokemonType.NORMAL, 100, 5, -1, 0, 2)
       .attr(AddBattlerTagAttr, BattlerTagType.ENCORE, false, true)
-      .ignoresSubstitute()
       .condition((user, target, move) => new EncoreTag(user.id).canAdd(target))
+      .ignoresSubstitute()
       .reflectable()
       // has incorrect interactions with Blood Moon/Gigaton Hammer
-      // TODO: How does Encore interact when locking
+      // TODO: Verify if Encore's duration decreases during status based move failures
       .edgeCase(),
     new AttackMove(MoveId.PURSUIT, PokemonType.DARK, MoveCategory.PHYSICAL, 40, 100, 20, -1, 0, 2)
       .partial(), // No effect implemented
@@ -9384,9 +9325,7 @@ export function initMoves() {
     new SelfStatusMove(MoveId.MAGIC_COAT, PokemonType.PSYCHIC, -1, 15, -1, 4, 3)
       .attr(AddBattlerTagAttr, BattlerTagType.MAGIC_COAT, true, true, 0)
       .condition(failIfLastCondition)
-      // Interactions with stomping tantrum, instruct, and other moves that
+      // Will not reflect roar / whirlwind if the target has ForceSwitchOutImmunityAbAttr
-      // rely on move history
-      // Also will not reflect roar / whirlwind if the target has ForceSwitchOutImmunityAbAttr
       .edgeCase(),
     new SelfStatusMove(MoveId.RECYCLE, PokemonType.NORMAL, -1, 10, -1, 0, 3)
       .unimplemented(),
@@ -9397,9 +9336,9 @@ export function initMoves() {
     new StatusMove(MoveId.YAWN, PokemonType.NORMAL, -1, 10, -1, 0, 3)
       .attr(YawnAttr)
       .reflectable()
-      .edgeCase(), // Should not be blocked by safeguard on turn of use
+      .edgeCase(), // Should not be blocked by safeguard once tag is applied
     new AttackMove(MoveId.KNOCK_OFF, PokemonType.DARK, MoveCategory.PHYSICAL, 65, 100, 20, -1, 0, 3)
-      .attr(MovePowerMultiplierAttr, (user, target, move) => target.getHeldItems().filter(i => i.isTransferable).length > 0 ? 1.5 : 1)
+      .attr(MovePowerMultiplierAttr, (_user, target, _move) => target.getHeldItems().some(i => i.isTransferable) ? 1.5 : 1)
       .attr(RemoveHeldItemAttr, false)
       .edgeCase(),
       // Should not be able to remove held item if user faints due to Rough Skin, Iron Barbs, etc.
@@ -10677,11 +10616,10 @@ export function initMoves() {
     new AttackMove(MoveId.TROP_KICK, PokemonType.GRASS, MoveCategory.PHYSICAL, 70, 100, 15, 100, 0, 7)
       .attr(StatStageChangeAttr, [ Stat.ATK ], -1),
     new StatusMove(MoveId.INSTRUCT, PokemonType.PSYCHIC, -1, 15, -1, 0, 7)
-      .ignoresSubstitute()
       .attr(RepeatMoveAttr)
+      .ignoresSubstitute()
       /*
        * Incorrect interactions with Gigaton Hammer, Blood Moon & Torment due to them _failing on use_, not merely being unselectable.
-       * Incorrectly ticks down Encore's fail counter
        * TODO: Verify whether Instruct can repeat Struggle
        * TODO: Verify whether Instruct can fail when using a copied move also in one's own moveset
        */

test/moves/delayed-attack.test.ts

@@ -385,5 +385,5 @@ describe("Moves - Delayed Attacks", () => {
   });
 
   // TODO: Implement and move to a power spot's test file
-  it.todo("Should activate ally's power spot when switched in during single battles");
+  it.todo("should activate ally's power spot when switched in during single battles");
 });

test/moves/encore.test.ts

@@ -1,3 +1,4 @@
+import { getPokemonNameWithAffix } from "#app/messages";
 import { AbilityId } from "#enums/ability-id";
 import { BattlerIndex } from "#enums/battler-index";
 import { BattlerTagType } from "#enums/battler-tag-type";
@@ -7,6 +8,7 @@ import { MoveUseMode } from "#enums/move-use-mode";
 import { SpeciesId } from "#enums/species-id";
 import { invalidEncoreMoves } from "#moves/invalid-moves";
 import { GameManager } from "#test/test-utils/game-manager";
+import i18next from "i18next";
 import Phaser from "phaser";
 import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest";
 
@@ -40,16 +42,52 @@ describe("Moves - Encore", () => {
   it("should prevent the target from using any move except the last used move", async () => {
     await game.classicMode.startBattle([SpeciesId.SNORLAX]);
 
-    const enemyPokemon = game.field.getEnemyPokemon();
+    const enemy = game.field.getEnemyPokemon();
 
     game.move.use(MoveId.ENCORE);
     await game.move.forceEnemyMove(MoveId.SPLASH);
     await game.setTurnOrder([BattlerIndex.ENEMY, BattlerIndex.PLAYER]);
     await game.toNextTurn();
 
-    expect(enemyPokemon).toHaveBattlerTag(BattlerTagType.ENCORE);
+    expect(enemy).toHaveBattlerTag(BattlerTagType.ENCORE);
-    expect(enemyPokemon.isMoveRestricted(MoveId.TACKLE)).toBe(true);
+    expect(enemy.isMoveRestricted(MoveId.TACKLE)).toBe(true);
-    expect(enemyPokemon.isMoveRestricted(MoveId.SPLASH)).toBe(false);
+    expect(enemy.isMoveRestricted(MoveId.SPLASH)).toBe(false);
+  });
+
+  it("should be removed on turn end after triggering thrice, ignoring Instruct", async () => {
+    await game.classicMode.startBattle([SpeciesId.SNORLAX]);
+
+    const enemy = game.field.getEnemyPokemon();
+    enemy.pushMoveHistory({ move: MoveId.SPLASH, targets: [BattlerIndex.PLAYER], useMode: MoveUseMode.NORMAL });
+
+    game.move.use(MoveId.ENCORE);
+    await game.move.forceEnemyMove(MoveId.SPLASH);
+    await game.setTurnOrder([BattlerIndex.PLAYER, BattlerIndex.ENEMY]);
+    await game.toNextTurn();
+
+    // Should have ticked down once
+    expect(enemy).toHaveBattlerTag(BattlerTagType.ENCORE);
+    expect(enemy.getTag(BattlerTagType.ENCORE)!.turnCount).toBe(2);
+
+    game.move.use(MoveId.INSTRUCT);
+    await game.toNextTurn();
+
+    expect(enemy.getTag(BattlerTagType.ENCORE)!.turnCount).toBe(1);
+
+    game.move.use(MoveId.INSTRUCT);
+    await game.toEndOfTurn(false);
+
+    // Tag should still be present until the `TurnEndPhase` ticks it down
+    expect(enemy).toHaveBattlerTag(BattlerTagType.ENCORE);
+
+    await game.toEndOfTurn();
+
+    expect(enemy).not.toHaveBattlerTag(BattlerTagType.ENCORE);
+    expect(game.textInterceptor.logs).toContain(
+      i18next.t("battlerTags:encoreOnRemove", {
+        pokemonNameWithAffix: getPokemonNameWithAffix(enemy),
+      }),
+    );
   });
 
   it("should override any upcoming moves with the Encored move, while still consuming PP", async () => {
@@ -72,7 +110,7 @@ describe("Moves - Encore", () => {
   // TODO: Make test using `changeMoveset`
   it.todo("should end at turn end if the user forgets the Encored move");
 
-  it("should end immediately if the move runs out of PP", async () => {
+  it("should be removed at turn end if the Encored move runs out of PP", async () => {
     await game.classicMode.startBattle([SpeciesId.SNORLAX]);
 
     // Fake enemy having used tackle the turn prior

test/test-utils/game-manager.ts

@@ -371,9 +371,13 @@ export class GameManager {
     console.log("==================[New Turn]==================");
   }
 
-  /** Transition to the {@linkcode TurnEndPhase | end of the current turn}. */
+  /**
-  async toEndOfTurn() {
+   * Transition to the {@linkcode TurnEndPhase | end of the current turn}.
-    await this.phaseInterceptor.to("TurnEndPhase");
+   * @param runTarget - Whether or not to run the {@linkcode TurnEndPhase}; default `true`
+   * @returns A Promise that resolves once the turn has ended.
+   */
+  async toEndOfTurn(runTarget = true): Promise<void> {
+    await this.phaseInterceptor.to("TurnEndPhase", runTarget);
     console.log("==================[End of Turn]==================");
   }