Merge pull request #2 from Bertie690/ally-target-review

PR Review

src/phases/move-phase.ts

@@ -87,15 +87,16 @@ export class MovePhase extends BattlePhase {
   }
 
   /**
-   * Checks if the pokemon is active, if the move is usable, and that the move is targeting something.
+   * Check if the current Move is usable and targeting at least 1 active pokemon.
    * @param ignoreDisableTags `true` to not check if the move is disabled
    * @returns `true` if all the checks pass
    */
   public canMove(ignoreDisableTags = false): boolean {
+    const targets = this.getActiveTargetPokemon();
     return (
-      this.pokemon.isActive(true) &&
       this.move.isUsable(this.pokemon, isIgnorePP(this.useMode), ignoreDisableTags) &&
-      this.targets.length > 0
+      // TODO: Don't hardcode a check for traps here
+      (targets.length > 0 || this.move.getMove().hasAttr("AddArenaTrapTagAttr"))
     );
   }
 
@@ -122,9 +123,9 @@ export class MovePhase extends BattlePhase {
 
     console.log(MoveId[this.move.moveId], enumValueToKey(MoveUseMode, this.useMode));
 
+    // If the target isn't on field (such as due to leaving the field from Whirlwind/etc), do nothing.
     if (!this.pokemon.isActive(true)) {
-      this.cancel();
+      super.end();
-      this.end();
       return;
     }
 
@@ -172,30 +173,18 @@ export class MovePhase extends BattlePhase {
     this.end();
   }
 
-  /** Check for cancellation edge cases - no targets remaining, or {@linkcode MoveId.NONE} is in the queue */
+  /** Check for cancellation edge cases - no targets remaining, out of PP, or {@linkcode MoveId.NONE} is in the queue */
   protected resolveFinalPreMoveCancellationChecks(): void {
-    const targets = this.getActiveTargetPokemon();
     const moveQueue = this.pokemon.getMoveQueue();
 
-    // Check if move is unusable (e.g. running out of PP due to a mid-turn Spite
-    // or the user no longer being on field)
-
-    if (!this.canMove(true)) {
-      if (this.pokemon.isActive(true)) {
-        this.fail();
-        this.showMoveText();
-        this.showFailedText();
-      }
-      return;
-    }
-
     if (
-      (targets.length === 0 && !this.move.getMove().hasAttr("AddArenaTrapTagAttr")) ||
+      // skip disable checks since we already triggered them inside `lapsePreMoveAndMoveTags`
+      !this.canMove(true) ||
       (moveQueue.length > 0 && moveQueue[0].move === MoveId.NONE)
     ) {
       this.showMoveText();
       this.showFailedText();
-      this.cancel();
+      this.fail();
     }
   }
 

test/data/status-effect.test.ts

@@ -395,28 +395,23 @@ describe("Status Effects", () => {
       await game.classicMode.startBattle([SpeciesId.FEEBAS]);
 
       const player = game.field.getPlayerPokemon();
-      player.status = new Status(StatusEffect.SLEEP, 0, 4);
+      // Set sleep turns to 2 for brevity
-
+      player.status = new Status(StatusEffect.SLEEP, 0, 2);
-      game.move.select(MoveId.DRAGON_CHEER);
+      game.move.changeMoveset(player, MoveId.DRAGON_CHEER);
-      await game.toNextTurn();
-
-      expect(player.status.effect).toBe(StatusEffect.SLEEP);
-
-      game.move.select(MoveId.DRAGON_CHEER);
-      await game.toNextTurn();
-
-      expect(player.status.effect).toBe(StatusEffect.SLEEP);
 
       game.move.select(MoveId.DRAGON_CHEER);
       await game.toNextTurn();
 
       expect(player.status.effect).toBe(StatusEffect.SLEEP);
+      expect(player.getMoveset()[0].ppUsed).toBe(0);
       expect(player.getLastXMoves(1)[0].result).toBe(MoveResult.FAIL);
 
       game.move.select(MoveId.DRAGON_CHEER);
       await game.toNextTurn();
 
+      // Sleep was cured, move failed as normal and consumed PP
       expect(player.status).toBeFalsy();
+      expect(player.getMoveset()[0].ppUsed).toBe(1);
       expect(player.getLastXMoves(1)[0].result).toBe(MoveResult.FAIL);
     });
   });