From 57a3efd9e2e5106c5ebee015921982535cf08446 Mon Sep 17 00:00:00 2001 From: NightKev <34855794+DayKev@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:51:05 -0700 Subject: [PATCH] [Bug] Fix off-by-one errors in some random number calls (#3665) * Fix off-by-one error in some random number calls * Fix mock RNG function used by tests Also remove unnecessary extra RNG mock from Glaive Rush test * Just some github UI manipulation don't mind me * Update Glaive Rush test * Remove unnecessary `Math.floor()` * Remove resolved comment * Add tsdocs to various functions * Remove `src/rng.md` file * Update tsdoc --- src/battle-scene.ts | 17 ++++- src/battle.ts | 6 ++ src/data/ability.ts | 2 +- src/data/battler-tags.ts | 2 +- src/data/move.ts | 2 +- src/field/pokemon.ts | 22 ++++++- src/phases/move-effect-phase.ts | 6 +- src/test/moves/glaive_rush.test.ts | 102 ++++++++++++++++------------- src/test/utils/gameManager.ts | 2 +- src/utils.ts | 8 ++- 10 files changed, 110 insertions(+), 59 deletions(-) diff --git a/src/battle-scene.ts b/src/battle-scene.ts index d4c33663c14..9123a213f4c 100644 --- a/src/battle-scene.ts +++ b/src/battle-scene.ts @@ -855,7 +855,7 @@ export default class BattleScene extends SceneBase { overrideModifiers(this, false); overrideHeldItems(this, pokemon, false); if (boss && !dataSource) { - const secondaryIvs = Utils.getIvsFromId(Utils.randSeedInt(4294967295)); + const secondaryIvs = Utils.getIvsFromId(Utils.randSeedInt(4294967296)); for (let s = 0; s < pokemon.ivs.length; s++) { pokemon.ivs[s] = Math.round(Phaser.Math.Linear(Math.min(pokemon.ivs[s], secondaryIvs[s]), Math.max(pokemon.ivs[s], secondaryIvs[s]), 0.75)); @@ -961,6 +961,16 @@ export default class BattleScene extends SceneBase { this.offsetGym = this.gameMode.isClassic && this.getGeneratedOffsetGym(); } + /** + * Generates a random number using the current battle's seed + * + * This calls {@linkcode Battle.randSeedInt}(`scene`, {@linkcode range}, {@linkcode min}) in `src/battle.ts` + * which calls {@linkcode Utils.randSeedInt randSeedInt}({@linkcode range}, {@linkcode min}) in `src/utils.ts` + * + * @param range How large of a range of random numbers to choose from. If {@linkcode range} <= 1, returns {@linkcode min} + * @param min The minimum integer to pick, default `0` + * @returns A random integer between {@linkcode min} and ({@linkcode min} + {@linkcode range} - 1) + */ randBattleSeedInt(range: integer, min: integer = 0): integer { return this.currentBattle?.randSeedInt(this, range, min); } @@ -1112,7 +1122,8 @@ export default class BattleScene extends SceneBase { doubleTrainer = false; } } - newTrainer = trainerData !== undefined ? trainerData.toTrainer(this) : new Trainer(this, trainerType, doubleTrainer ? TrainerVariant.DOUBLE : Utils.randSeedInt(2) ? TrainerVariant.FEMALE : TrainerVariant.DEFAULT); + const variant = doubleTrainer ? TrainerVariant.DOUBLE : (Utils.randSeedInt(2) ? TrainerVariant.FEMALE : TrainerVariant.DEFAULT); + newTrainer = trainerData !== undefined ? trainerData.toTrainer(this) : new Trainer(this, trainerType, variant); this.field.add(newTrainer); } } @@ -2620,7 +2631,7 @@ export default class BattleScene extends SceneBase { if (mods.length < 1) { return mods; } - const rand = Math.floor(Utils.randSeedInt(mods.length)); + const rand = Utils.randSeedInt(mods.length); return [mods[rand], ...shuffleModifiers(mods.filter((_, i) => i !== rand))]; }; modifiers = shuffleModifiers(modifiers); diff --git a/src/battle.ts b/src/battle.ts index 0f1245a4397..b80caa9e679 100644 --- a/src/battle.ts +++ b/src/battle.ts @@ -354,6 +354,12 @@ export default class Battle { return null; } + /** + * Generates a random number using the current battle's seed. Calls {@linkcode Utils.randSeedInt} + * @param range How large of a range of random numbers to choose from. If {@linkcode range} <= 1, returns {@linkcode min} + * @param min The minimum integer to pick, default `0` + * @returns A random integer between {@linkcode min} and ({@linkcode min} + {@linkcode range} - 1) + */ randSeedInt(scene: BattleScene, range: number, min: number = 0): number { if (range <= 1) { return min; diff --git a/src/data/ability.ts b/src/data/ability.ts index fde39ebb152..2407460b87d 100755 --- a/src/data/ability.ts +++ b/src/data/ability.ts @@ -2642,7 +2642,7 @@ export class ConfusionOnStatusEffectAbAttr extends PostAttackAbAttr { if (simulated) { return defender.canAddTag(BattlerTagType.CONFUSED); } else { - return defender.addTag(BattlerTagType.CONFUSED, pokemon.randSeedInt(3, 2), move.id, defender.id); + return defender.addTag(BattlerTagType.CONFUSED, pokemon.randSeedIntRange(2, 5), move.id, defender.id); } } return false; diff --git a/src/data/battler-tags.ts b/src/data/battler-tags.ts index ef91dda7b63..ddb85600c18 100644 --- a/src/data/battler-tags.ts +++ b/src/data/battler-tags.ts @@ -486,7 +486,7 @@ export class ConfusedTag extends BattlerTag { if (pokemon.randSeedInt(3) === 0) { const atk = pokemon.getEffectiveStat(Stat.ATK); const def = pokemon.getEffectiveStat(Stat.DEF); - const damage = Utils.toDmgValue(((((2 * pokemon.level / 5 + 2) * 40 * atk / def) / 50) + 2) * (pokemon.randSeedInt(15, 85) / 100)); + const damage = Utils.toDmgValue(((((2 * pokemon.level / 5 + 2) * 40 * atk / def) / 50) + 2) * (pokemon.randSeedIntRange(85, 100) / 100)); pokemon.scene.queueMessage(i18next.t("battlerTags:confusedLapseHurtItself")); pokemon.damageAndUpdate(damage); pokemon.battleData.hitCount++; diff --git a/src/data/move.ts b/src/data/move.ts index 252c474864c..96b780a8330 100644 --- a/src/data/move.ts +++ b/src/data/move.ts @@ -4400,7 +4400,7 @@ export class AddBattlerTagAttr extends MoveEffectAttr { const moveChance = this.getMoveChance(user, target, move, this.selfTarget, true); if (moveChance < 0 || moveChance === 100 || user.randSeedInt(100) < moveChance) { - return (this.selfTarget ? user : target).addTag(this.tagType, user.randSeedInt(this.turnCountMax - this.turnCountMin, this.turnCountMin), move.id, user.id); + return (this.selfTarget ? user : target).addTag(this.tagType, user.randSeedIntRange(this.turnCountMin, this.turnCountMax), move.id, user.id); } return false; diff --git a/src/field/pokemon.ts b/src/field/pokemon.ts index 269d0b1dba5..f522d50f357 100644 --- a/src/field/pokemon.ts +++ b/src/field/pokemon.ts @@ -1720,7 +1720,7 @@ export default abstract class Pokemon extends Phaser.GameObjects.Container { }; this.fusionSpecies = this.scene.randomSpecies(this.scene.currentBattle?.waveIndex || 0, this.level, false, filter, true); - this.fusionAbilityIndex = (this.fusionSpecies.abilityHidden && hasHiddenAbility ? this.fusionSpecies.ability2 ? 2 : 1 : this.fusionSpecies.ability2 ? randAbilityIndex : 0); + this.fusionAbilityIndex = (this.fusionSpecies.abilityHidden && hasHiddenAbility ? 2 : this.fusionSpecies.ability2 !== this.fusionSpecies.ability1 ? randAbilityIndex : 0); this.fusionShiny = this.shiny; this.fusionVariant = this.variant; @@ -2278,7 +2278,7 @@ export default abstract class Pokemon extends Phaser.GameObjects.Container { if (!isTypeImmune) { const levelMultiplier = (2 * source.level / 5 + 2); - const randomMultiplier = ((this.scene.randBattleSeedInt(16) + 85) / 100); + const randomMultiplier = (this.randSeedIntRange(85, 100) / 100); damage.value = Utils.toDmgValue((((levelMultiplier * power * sourceAtk.value / targetDef.value) / 50) + 2) * stabMultiplier.value * typeMultiplier @@ -3448,12 +3448,30 @@ export default abstract class Pokemon extends Phaser.GameObjects.Container { fusionCanvas.remove(); } + /** + * Generates a random number using the current battle's seed, or the global seed if `this.scene.currentBattle` is falsy + * + * This calls either {@linkcode BattleScene.randBattleSeedInt}({@linkcode range}, {@linkcode min}) in `src/battle-scene.ts` + * which calls {@linkcode Battle.randSeedInt}(`scene`, {@linkcode range}, {@linkcode min}) in `src/battle.ts` + * which calls {@linkcode Utils.randSeedInt randSeedInt}({@linkcode range}, {@linkcode min}) in `src/utils.ts`, + * or it directly calls {@linkcode Utils.randSeedInt randSeedInt}({@linkcode range}, {@linkcode min}) in `src/utils.ts` if there is no current battle + * + * @param range How large of a range of random numbers to choose from. If {@linkcode range} <= 1, returns {@linkcode min} + * @param min The minimum integer to pick, default `0` + * @returns A random integer between {@linkcode min} and ({@linkcode min} + {@linkcode range} - 1) + */ randSeedInt(range: integer, min: integer = 0): integer { return this.scene.currentBattle ? this.scene.randBattleSeedInt(range, min) : Utils.randSeedInt(range, min); } + /** + * Generates a random number using the current battle's seed, or the global seed if `this.scene.currentBattle` is falsy + * @param min The minimum integer to generate + * @param max The maximum integer to generate + * @returns a random integer between {@linkcode min} and {@linkcode max} inclusive + */ randSeedIntRange(min: integer, max: integer): integer { return this.randSeedInt((max - min) + 1, min); } diff --git a/src/phases/move-effect-phase.ts b/src/phases/move-effect-phase.ts index f100a763219..9b22c520e19 100644 --- a/src/phases/move-effect-phase.ts +++ b/src/phases/move-effect-phase.ts @@ -377,16 +377,16 @@ export class MoveEffectPhase extends PokemonPhase { return false; } - const moveAccuracy = this.move.getMove().calculateBattleAccuracy(user!, target); // TODO: is the bang correct here? + const moveAccuracy = this.move.getMove().calculateBattleAccuracy(user, target); if (moveAccuracy === -1) { return true; } const accuracyMultiplier = user.getAccuracyMultiplier(target, this.move.getMove()); - const rand = user.randSeedInt(100, 1); + const rand = user.randSeedInt(100); - return rand <= moveAccuracy * (accuracyMultiplier!); // TODO: is this bang correct? + return rand < (moveAccuracy * accuracyMultiplier); } /** Returns the {@linkcode Pokemon} using this phase's invoked move */ diff --git a/src/test/moves/glaive_rush.test.ts b/src/test/moves/glaive_rush.test.ts index 1eac3c32bb4..5867ef751b8 100644 --- a/src/test/moves/glaive_rush.test.ts +++ b/src/test/moves/glaive_rush.test.ts @@ -1,13 +1,12 @@ import { allMoves } from "#app/data/move"; import { Abilities } from "#app/enums/abilities"; -import { DamagePhase } from "#app/phases/damage-phase"; -import { TurnEndPhase } from "#app/phases/turn-end-phase"; -import { Moves } from "#enums/moves"; -import { Species } from "#enums/species"; +import { Moves } from "#app/enums/moves"; +import { Species } from "#app/enums/species"; import GameManager from "#test/utils/gameManager"; import Phaser from "phaser"; -import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; +const TIMEOUT = 20 * 1000; describe("Moves - Glaive Rush", () => { let phaserGame: Phaser.Game; @@ -25,131 +24,142 @@ describe("Moves - Glaive Rush", () => { beforeEach(() => { game = new GameManager(phaserGame); - game.override.battleType("single"); - game.override.disableCrits(); - game.override.enemySpecies(Species.MAGIKARP); - game.override.enemyAbility(Abilities.BALL_FETCH); - game.override.enemyMoveset(Array(4).fill(Moves.GLAIVE_RUSH)); - game.override.starterSpecies(Species.KLINK); - game.override.ability(Abilities.UNNERVE); - game.override.passiveAbility(Abilities.FUR_COAT); - game.override.moveset([Moves.SHADOW_SNEAK, Moves.AVALANCHE, Moves.SPLASH, Moves.GLAIVE_RUSH]); + game.override + .battleType("single") + .disableCrits() + .enemySpecies(Species.MAGIKARP) + .enemyAbility(Abilities.BALL_FETCH) + .enemyMoveset(Array(4).fill(Moves.GLAIVE_RUSH)) + .starterSpecies(Species.KLINK) + .ability(Abilities.BALL_FETCH) + .moveset([Moves.SHADOW_SNEAK, Moves.AVALANCHE, Moves.SPLASH, Moves.GLAIVE_RUSH]); }); it("takes double damage from attacks", async () => { - await game.startBattle(); + await game.classicMode.startBattle(); + const enemy = game.scene.getEnemyPokemon()!; enemy.hp = 1000; - vi.spyOn(game.scene, "randBattleSeedInt").mockReturnValue(0); game.move.select(Moves.SHADOW_SNEAK); - await game.phaseInterceptor.to(DamagePhase); + await game.phaseInterceptor.to("DamagePhase"); const damageDealt = 1000 - enemy.hp; - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); game.move.select(Moves.SHADOW_SNEAK); - await game.phaseInterceptor.to(DamagePhase); + await game.phaseInterceptor.to("DamagePhase"); expect(enemy.hp).toBeLessThanOrEqual(1001 - (damageDealt * 3)); - }, 5000); // TODO: revert back to 20s + }, TIMEOUT); it("always gets hit by attacks", async () => { - await game.startBattle(); + await game.classicMode.startBattle(); + const enemy = game.scene.getEnemyPokemon()!; enemy.hp = 1000; allMoves[Moves.AVALANCHE].accuracy = 0; game.move.select(Moves.AVALANCHE); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); expect(enemy.hp).toBeLessThan(1000); - }, 20000); + }, TIMEOUT); it("interacts properly with multi-lens", async () => { - game.override.startingHeldItems([{ name: "MULTI_LENS", count: 2 }]); - game.override.enemyMoveset(Array(4).fill(Moves.AVALANCHE)); - await game.startBattle(); + game.override + .startingHeldItems([{ name: "MULTI_LENS", count: 2 }]) + .enemyMoveset(Array(4).fill(Moves.AVALANCHE)); + await game.classicMode.startBattle(); + const player = game.scene.getPlayerPokemon()!; const enemy = game.scene.getEnemyPokemon()!; + enemy.hp = 1000; player.hp = 1000; allMoves[Moves.AVALANCHE].accuracy = 0; game.move.select(Moves.GLAIVE_RUSH); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); expect(player.hp).toBeLessThan(1000); player.hp = 1000; game.move.select(Moves.SPLASH); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); expect(player.hp).toBe(1000); - }, 20000); + }, TIMEOUT); it("secondary effects only last until next move", async () => { game.override.enemyMoveset(Array(4).fill(Moves.SHADOW_SNEAK)); - await game.startBattle(); + await game.classicMode.startBattle(); + const player = game.scene.getPlayerPokemon()!; const enemy = game.scene.getEnemyPokemon()!; + enemy.hp = 1000; player.hp = 1000; allMoves[Moves.SHADOW_SNEAK].accuracy = 0; game.move.select(Moves.GLAIVE_RUSH); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); expect(player.hp).toBe(1000); game.move.select(Moves.SPLASH); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); const damagedHp = player.hp; expect(player.hp).toBeLessThan(1000); game.move.select(Moves.SPLASH); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); expect(player.hp).toBe(damagedHp); - }, 20000); + }, TIMEOUT); it("secondary effects are removed upon switching", async () => { - game.override.enemyMoveset(Array(4).fill(Moves.SHADOW_SNEAK)); - game.override.starterSpecies(0); - await game.startBattle([Species.KLINK, Species.FEEBAS]); + game.override + .enemyMoveset(Array(4).fill(Moves.SHADOW_SNEAK)) + .starterSpecies(0); + await game.classicMode.startBattle([Species.KLINK, Species.FEEBAS]); + const player = game.scene.getPlayerPokemon()!; const enemy = game.scene.getEnemyPokemon()!; + enemy.hp = 1000; allMoves[Moves.SHADOW_SNEAK].accuracy = 0; game.move.select(Moves.GLAIVE_RUSH); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); expect(player.hp).toBe(player.getMaxHp()); game.doSwitchPokemon(1); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); game.doSwitchPokemon(1); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); expect(player.hp).toBe(player.getMaxHp()); - }, 20000); + }, TIMEOUT); it("secondary effects don't activate if move fails", async () => { game.override.moveset([Moves.SHADOW_SNEAK, Moves.PROTECT, Moves.SPLASH, Moves.GLAIVE_RUSH]); - await game.startBattle(); + await game.classicMode.startBattle(); + const player = game.scene.getPlayerPokemon()!; const enemy = game.scene.getEnemyPokemon()!; + enemy.hp = 1000; player.hp = 1000; game.move.select(Moves.PROTECT); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); game.move.select(Moves.SHADOW_SNEAK); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); game.override.enemyMoveset(Array(4).fill(Moves.SPLASH)); const damagedHP1 = 1000 - enemy.hp; enemy.hp = 1000; game.move.select(Moves.SHADOW_SNEAK); - await game.phaseInterceptor.to(TurnEndPhase); + await game.phaseInterceptor.to("TurnEndPhase"); const damagedHP2 = 1000 - enemy.hp; expect(damagedHP2).toBeGreaterThanOrEqual((damagedHP1 * 2) - 1); - }, 20000); + }, TIMEOUT); }); diff --git a/src/test/utils/gameManager.ts b/src/test/utils/gameManager.ts index f367fc70936..ade33aa1148 100644 --- a/src/test/utils/gameManager.ts +++ b/src/test/utils/gameManager.ts @@ -76,7 +76,7 @@ export default class GameManager { constructor(phaserGame: Phaser.Game, bypassLogin: boolean = true) { localStorage.clear(); ErrorInterceptor.getInstance().clear(); - BattleScene.prototype.randBattleSeedInt = (arg) => arg-1; + BattleScene.prototype.randBattleSeedInt = (range, min: number = 0) => min + range - 1; // This simulates a max roll this.gameWrapper = new GameWrapper(phaserGame, bypassLogin); this.scene = new BattleScene(); this.phaseInterceptor = new PhaseInterceptor(this.scene); diff --git a/src/utils.ts b/src/utils.ts index 173ea25b17c..fd5430d7276 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,5 +1,5 @@ -import i18next from "i18next"; import { MoneyFormat } from "#enums/money-format"; +import i18next from "i18next"; export const MissingTextureKey = "__MISSING"; @@ -82,6 +82,12 @@ export function randInt(range: integer, min: integer = 0): integer { return Math.floor(Math.random() * range) + min; } +/** + * Generates a random number using the global seed, or the current battle's seed if called via `Battle.randSeedInt` + * @param range How large of a range of random numbers to choose from. If {@linkcode range} <= 1, returns {@linkcode min} + * @param min The minimum integer to pick, default `0` + * @returns A random integer between {@linkcode min} and ({@linkcode min} + {@linkcode range} - 1) + */ export function randSeedInt(range: integer, min: integer = 0): integer { if (range <= 1) { return min;