[Move] Update documentation for AddSubstituteAttr; fix Shed Tail incorrect error message (#6873)

* [Move] Update documentation for attribute; fix Shed Tail incorrect error message

* Add another test

---------

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

docs/comments.md

@@ -38,28 +38,37 @@ For an example of how TSDoc comments work, here are some TSDoc comments taken fr
  * Attribute to put in a {@link https://bulbapedia.bulbagarden.net/wiki/Substitute_(doll) | Substitute Doll} for the user.
  */
 export class AddSubstituteAttr extends MoveEffectAttr {
-  /** The ratio of the user's max HP that is required to apply this effect */
-  private hpCost: number;
-  /** Whether the damage taken should be rounded up (Shed Tail rounds up) */
-  private roundUp: boolean;
+  /** The percentage of the user's maximum HP that is required to apply this effect. */
+  private readonly hpCost: number;
+  /** Whether the damage taken should be rounded up (Shed Tail rounds up). */
+  private readonly roundUp: boolean;
 
   constructor(hpCost: number, roundUp: boolean) {
     // code removed
   }
 
   /**
-   * Removes 1/4 of the user's maximum HP (rounded down) to create a substitute for the user
-   * @param user - The {@linkcode Pokemon} that used the move.
-   * @param target - n/a
-   * @param move - The {@linkcode Move} with this attribute.
-   * @param args - n/a
-   * @returns `true` if the attribute successfully applies, `false` otherwise
+   * Helper function to compute the amount of HP required to create a substitute.
+   * @param user - The {@linkcode Pokemon} using the move
+   * @returns The amount of HP that required to create a substitute.
    */
-  apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean {
+  private getHpCost(user: Pokemon): number {
     // code removed
   }
 
-  getUserBenefitScore(user: Pokemon, target: Pokemon, move: Move): number {
+  /**
+   * Remove a fraction of the user's maximum HP to create a 25% HP substitute doll.
+   * @param user - The {@linkcode Pokemon} using the move
+   * @param target - n/a
+   * @param move - The {@linkcode Move} being used
+   * @param args - n/a
+   * @returns Whether the attribute successfully applied.
+   */
+  public override apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean {
+    // code removed
+  }
+
+  public override getUserBenefitScore(user: Pokemon, _target: Pokemon, _move: Move): number {
     // code removed
   }
 
@@ -67,12 +76,7 @@ export class AddSubstituteAttr extends MoveEffectAttr {
     // code removed
   }
 
-  /**
-   * Get the substitute-specific failure message if one should be displayed.
-   * @param user - The pokemon using the move.
-   * @returns The substitute-specific failure message if the conditions apply, otherwise `undefined`
-   */
-  getFailedText(user: Pokemon, _target: Pokemon, _move: Move): string | undefined {
+  public override getFailedText(user: Pokemon): string | undefined {
     // code removed
   }
 }

src/data/moves/move.ts

@@ -2227,11 +2227,13 @@ export class HalfSacrificialAttr extends MoveEffectAttr {
 
 /**
  * Attribute to put in a {@link https://bulbapedia.bulbagarden.net/wiki/Substitute_(doll) | Substitute Doll} for the user.
+ *
+ * Used for {@linkcode MoveId.SUBSTITUTE} and {@linkcode MoveId.SHED_TAIL}.
  */
 export class AddSubstituteAttr extends MoveEffectAttr {
-  /** The ratio of the user's max HP that is required to apply this effect */
+  /** The percentage of the user's maximum HP that is required to apply this effect. */
   private readonly hpCost: number;
-  /** Whether the damage taken should be rounded up (Shed Tail rounds up) */
+  /** Whether the damage taken should be rounded up (Shed Tail rounds up). */
   private readonly roundUp: boolean;
 
   constructor(hpCost: number, roundUp: boolean) {
@@ -2242,50 +2244,49 @@ export class AddSubstituteAttr extends MoveEffectAttr {
   }
 
   /**
-   * Removes 1/4 of the user's maximum HP (rounded down) to create a substitute for the user
-   * @param user - The {@linkcode Pokemon} that used the move.
-   * @param target - n/a
-   * @param move - The {@linkcode Move} with this attribute.
-   * @param args - n/a
-   * @returns `true` if the attribute successfully applies, `false` otherwise
+   * Helper function to compute the amount of HP required to create a substitute.
+   * @param user - The {@linkcode Pokemon} using the move
+   * @returns The amount of HP that is required to create a substitute.
    */
-  apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean {
+  private getHpCost(user: Pokemon): number {
+    return (this.roundUp ? Math.ceil : toDmgValue)(user.getMaxHp() * this.hpCost);
+  }
+
+  /**
+   * Remove a fraction of the user's maximum HP to create a 25% HP substitute doll.
+   * @param user - The {@linkcode Pokemon} using the move
+   * @param target - n/a
+   * @param move - The {@linkcode Move} being used
+   * @param args - n/a
+   * @returns Whether the attribute successfully applied
+   */
+  public override apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean {
     if (!super.apply(user, target, move, args)) {
       return false;
     }
 
-    const damageTaken = this.roundUp
-      ? Math.ceil(user.getMaxHp() * this.hpCost)
-      : Math.floor(user.getMaxHp() * this.hpCost);
-    user.damageAndUpdate(damageTaken, { result: HitResult.INDIRECT, ignoreSegments: true, ignoreFaintPhase: true });
+    const dmgTaken = this.getHpCost(user);
+    user.damageAndUpdate(dmgTaken, { result: HitResult.INDIRECT, ignoreSegments: true, ignoreFaintPhase: true });
     user.addTag(BattlerTagType.SUBSTITUTE, 0, move.id, user.id);
     return true;
   }
 
-  getUserBenefitScore(user: Pokemon, _target: Pokemon, _move: Move): number {
+  public override getUserBenefitScore(user: Pokemon, _target: Pokemon, _move: Move): number {
     if (user.isBoss()) {
       return -10;
     }
     return 5;
   }
 
-  getCondition(): MoveConditionFunc {
-    return (user, _target, _move) =>
-      !user.getTag(SubstituteTag)
-      && user.hp > (this.roundUp ? Math.ceil(user.getMaxHp() * this.hpCost) : Math.floor(user.getMaxHp() * this.hpCost))
-      && user.getMaxHp() > 1;
+  public override getCondition(): MoveConditionFunc {
+    return user => !user.getTag(SubstituteTag) && user.hp > this.getHpCost(user);
   }
 
-  /**
-   * Get the substitute-specific failure message if one should be displayed.
-   * @param user - The pokemon using the move.
-   * @returns The substitute-specific failure message if the conditions apply, otherwise `undefined`
-   */
-  getFailedText(user: Pokemon, _target: Pokemon, _move: Move): string | undefined {
+  public override getFailedText(user: Pokemon): string | undefined {
     if (user.getTag(SubstituteTag)) {
       return i18next.t("moveTriggers:substituteOnOverlap", { pokemonName: getPokemonNameWithAffix(user) });
     }
-    if (user.hp <= Math.floor(user.getMaxHp() / 4) || user.getMaxHp() === 1) {
+    if (user.hp <= this.getHpCost(user)) {
       return i18next.t("moveTriggers:substituteNotEnoughHp");
     }
   }

test/moves/shed-tail.test.ts

@@ -4,6 +4,7 @@ import { MoveId } from "#enums/move-id";
 import { MoveResult } from "#enums/move-result";
 import { SpeciesId } from "#enums/species-id";
 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";
 
@@ -65,4 +66,17 @@ describe("Moves - Shed Tail", () => {
     expect(magikarp.isOnField()).toBeTruthy();
     expect(magikarp.getLastXMoves()[0].result).toBe(MoveResult.FAIL);
   });
+
+  it("should show the correct failure message between 26-50% HP", async () => {
+    await game.classicMode.startBattle([SpeciesId.FEEBAS, SpeciesId.ABRA]);
+
+    const feebas = game.field.getPlayerPokemon();
+    feebas.hp *= 0.4;
+
+    game.move.use(MoveId.SHED_TAIL);
+    await game.toEndOfTurn();
+
+    expect(feebas).toHaveUsedMove({ move: MoveId.SHED_TAIL, result: MoveResult.FAIL });
+    expect(game).toHaveShownMessage(i18next.t("moveTriggers:substituteNotEnoughHp"));
+  });
 });

test/moves/substitute.test.ts

@@ -509,4 +509,18 @@ describe("Moves - Substitute", () => {
 
     expect(playerPokemon.getTag(BattlerTagType.SEEDED)).toBeUndefined();
   });
+
+  it("should fail if the user has 1 max HP", async () => {
+    await game.classicMode.startBattle([SpeciesId.SHEDINJA]);
+
+    const player = game.field.getPlayerPokemon();
+
+    game.move.use(MoveId.SUBSTITUTE);
+    await game.toEndOfTurn();
+
+    expect(player).toHaveUsedMove({ move: MoveId.SUBSTITUTE, result: MoveResult.FAIL });
+    expect(player).not.toHaveBattlerTag(BattlerTagType.SUBSTITUTE);
+    expect(player).toHaveFullHp();
+    expect(player).toHaveHp(1);
+  });
 });