From 587360c8dae409e6a305a897358cdfba328711a2 Mon Sep 17 00:00:00 2001 From: PigeonBar <56974298+PigeonBar@users.noreply.github.com> Date: Mon, 2 Sep 2024 22:18:18 -0400 Subject: [PATCH] [Bug] Fix eggs having exploitable RNG (#3913) * [Bug] Fix eggs having exploitable RNG * Fix Wind Rider test having random chance to fail * Revert egg's ID back to its own unseeded generation * Remove change from wind rider test --------- Co-authored-by: NightKev <34855794+DayKev@users.noreply.github.com> Co-authored-by: Mumble <171087428+frutescens@users.noreply.github.com> --- src/battle-scene.ts | 1 + src/data/egg.ts | 140 +++++++++++++++++++--------------- src/test/battle-scene.test.ts | 10 ++- src/test/eggs/egg.test.ts | 75 +++++++++++++++++- 4 files changed, 161 insertions(+), 65 deletions(-) diff --git a/src/battle-scene.ts b/src/battle-scene.ts index 70f214f57c6..c9f7362728a 100644 --- a/src/battle-scene.ts +++ b/src/battle-scene.ts @@ -974,6 +974,7 @@ export default class BattleScene extends SceneBase { this.setSeed(Overrides.SEED_OVERRIDE || Utils.randomString(24)); console.log("Seed:", this.seed); + this.resetSeed(); // Properly resets RNG after saving and quitting a session this.disableMenu = false; diff --git a/src/data/egg.ts b/src/data/egg.ts index 3e872d364f3..9beb944de69 100644 --- a/src/data/egg.ts +++ b/src/data/egg.ts @@ -139,46 +139,57 @@ export class Egg { //// constructor(eggOptions?: IEggOptions) { - //if (eggOptions.tier && eggOptions.species) throw Error("Error egg can't have species and tier as option. only choose one of them.") + const generateEggProperties = (eggOptions?: IEggOptions) => { + //if (eggOptions.tier && eggOptions.species) throw Error("Error egg can't have species and tier as option. only choose one of them.") - this._sourceType = eggOptions?.sourceType!; // TODO: is this bang correct? - // Ensure _sourceType is defined before invoking rollEggTier(), as it is referenced - this._tier = eggOptions?.tier ?? (Overrides.EGG_TIER_OVERRIDE ?? this.rollEggTier()); - // If egg was pulled, check if egg pity needs to override the egg tier - if (eggOptions?.pulled) { - // Needs this._tier and this._sourceType to work - this.checkForPityTierOverrides(eggOptions.scene!); // TODO: is this bang correct? - } + this._sourceType = eggOptions?.sourceType!; // TODO: is this bang correct? + // Ensure _sourceType is defined before invoking rollEggTier(), as it is referenced + this._tier = eggOptions?.tier ?? (Overrides.EGG_TIER_OVERRIDE ?? this.rollEggTier()); + // If egg was pulled, check if egg pity needs to override the egg tier + if (eggOptions?.pulled) { + // Needs this._tier and this._sourceType to work + this.checkForPityTierOverrides(eggOptions.scene!); // TODO: is this bang correct? + } - this._id = eggOptions?.id ?? Utils.randInt(EGG_SEED, EGG_SEED * this._tier); + this._id = eggOptions?.id ?? Utils.randInt(EGG_SEED, EGG_SEED * this._tier); - this._sourceType = eggOptions?.sourceType ?? undefined; - this._hatchWaves = eggOptions?.hatchWaves ?? this.getEggTierDefaultHatchWaves(); - this._timestamp = eggOptions?.timestamp ?? new Date().getTime(); + this._sourceType = eggOptions?.sourceType ?? undefined; + this._hatchWaves = eggOptions?.hatchWaves ?? this.getEggTierDefaultHatchWaves(); + this._timestamp = eggOptions?.timestamp ?? new Date().getTime(); - // First roll shiny and variant so we can filter if species with an variant exist - this._isShiny = eggOptions?.isShiny ?? (Overrides.EGG_SHINY_OVERRIDE || this.rollShiny()); - this._variantTier = eggOptions?.variantTier ?? (Overrides.EGG_VARIANT_OVERRIDE ?? this.rollVariant()); - this._species = eggOptions?.species ?? this.rollSpecies(eggOptions!.scene!)!; // TODO: Are those bangs correct? + // First roll shiny and variant so we can filter if species with an variant exist + this._isShiny = eggOptions?.isShiny ?? (Overrides.EGG_SHINY_OVERRIDE || this.rollShiny()); + this._variantTier = eggOptions?.variantTier ?? (Overrides.EGG_VARIANT_OVERRIDE ?? this.rollVariant()); + this._species = eggOptions?.species ?? this.rollSpecies(eggOptions!.scene!)!; // TODO: Are those bangs correct? - this._overrideHiddenAbility = eggOptions?.overrideHiddenAbility ?? false; + this._overrideHiddenAbility = eggOptions?.overrideHiddenAbility ?? false; - // Override egg tier and hatchwaves if species was given - if (eggOptions?.species) { - this._tier = this.getEggTierFromSpeciesStarterValue(); - this._hatchWaves = eggOptions.hatchWaves ?? this.getEggTierDefaultHatchWaves(); - } - // If species has no variant, set variantTier to common. This needs to - // be done because species with no variants get filtered at rollSpecies but if the - // species is set via options or the legendary gacha pokemon gets choosen the check never happens - if (this._species && !getPokemonSpecies(this._species).hasVariants()) { - this._variantTier = VariantTier.COMMON; - } - // Needs this._tier so it needs to be generated afer the tier override if bought from same species - this._eggMoveIndex = eggOptions?.eggMoveIndex ?? this.rollEggMoveIndex(); - if (eggOptions?.pulled) { - this.increasePullStatistic(eggOptions.scene!); // TODO: is this bang correct? - this.addEggToGameData(eggOptions.scene!); // TODO: is this bang correct? + // Override egg tier and hatchwaves if species was given + if (eggOptions?.species) { + this._tier = this.getEggTierFromSpeciesStarterValue(); + this._hatchWaves = eggOptions.hatchWaves ?? this.getEggTierDefaultHatchWaves(); + } + // If species has no variant, set variantTier to common. This needs to + // be done because species with no variants get filtered at rollSpecies but if the + // species is set via options or the legendary gacha pokemon gets choosen the check never happens + if (this._species && !getPokemonSpecies(this._species).hasVariants()) { + this._variantTier = VariantTier.COMMON; + } + // Needs this._tier so it needs to be generated afer the tier override if bought from same species + this._eggMoveIndex = eggOptions?.eggMoveIndex ?? this.rollEggMoveIndex(); + if (eggOptions?.pulled) { + this.increasePullStatistic(eggOptions.scene!); // TODO: is this bang correct? + this.addEggToGameData(eggOptions.scene!); // TODO: is this bang correct? + } + }; + + if (eggOptions?.scene) { + const seedOverride = Utils.randomString(24); + eggOptions?.scene.executeWithSeedOffset(() => { + generateEggProperties(eggOptions); + }, 0, seedOverride); + } else { // For legacy eggs without scene + generateEggProperties(eggOptions); } } @@ -200,37 +211,46 @@ export class Egg { // Generates a PlayerPokemon from an egg public generatePlayerPokemon(scene: BattleScene): PlayerPokemon { - // Legacy egg wants to hatch. Generate missing properties - if (!this._species) { - this._isShiny = this.rollShiny(); - this._species = this.rollSpecies(scene!)!; // TODO: are these bangs correct? - } + let ret: PlayerPokemon; - let pokemonSpecies = getPokemonSpecies(this._species); - // Special condition to have Phione eggs also have a chance of generating Manaphy - if (this._species === Species.PHIONE) { - pokemonSpecies = getPokemonSpecies(Utils.randSeedInt(MANAPHY_EGG_MANAPHY_RATE) ? Species.PHIONE : Species.MANAPHY); - } + const generatePlayerPokemonHelper = (scene: BattleScene) => { + // Legacy egg wants to hatch. Generate missing properties + if (!this._species) { + this._isShiny = this.rollShiny(); + this._species = this.rollSpecies(scene!)!; // TODO: are these bangs correct? + } - // Sets the hidden ability if a hidden ability exists and - // the override is set or the egg hits the chance - let abilityIndex: number | undefined = undefined; - const sameSpeciesEggHACheck = (this._sourceType === EggSourceType.SAME_SPECIES_EGG && !Utils.randSeedInt(SAME_SPECIES_EGG_HA_RATE)); - const gachaEggHACheck = (!(this._sourceType === EggSourceType.SAME_SPECIES_EGG) && !Utils.randSeedInt(GACHA_EGG_HA_RATE)); - if (pokemonSpecies.abilityHidden && (this._overrideHiddenAbility || sameSpeciesEggHACheck || gachaEggHACheck)) { - abilityIndex = 2; - } + let pokemonSpecies = getPokemonSpecies(this._species); + // Special condition to have Phione eggs also have a chance of generating Manaphy + if (this._species === Species.PHIONE) { + pokemonSpecies = getPokemonSpecies(Utils.randSeedInt(MANAPHY_EGG_MANAPHY_RATE) ? Species.PHIONE : Species.MANAPHY); + } - // This function has way to many optional parameters - const ret: PlayerPokemon = scene.addPlayerPokemon(pokemonSpecies, 1, abilityIndex, undefined, undefined, false); - ret.shiny = this._isShiny; - ret.variant = this._variantTier; + // Sets the hidden ability if a hidden ability exists and + // the override is set or the egg hits the chance + let abilityIndex: number | undefined = undefined; + const sameSpeciesEggHACheck = (this._sourceType === EggSourceType.SAME_SPECIES_EGG && !Utils.randSeedInt(SAME_SPECIES_EGG_HA_RATE)); + const gachaEggHACheck = (!(this._sourceType === EggSourceType.SAME_SPECIES_EGG) && !Utils.randSeedInt(GACHA_EGG_HA_RATE)); + if (pokemonSpecies.abilityHidden && (this._overrideHiddenAbility || sameSpeciesEggHACheck || gachaEggHACheck)) { + abilityIndex = 2; + } - const secondaryIvs = Utils.getIvsFromId(Utils.randSeedInt(4294967295)); + // This function has way to many optional parameters + ret = scene.addPlayerPokemon(pokemonSpecies, 1, abilityIndex, undefined, undefined, false); + ret.shiny = this._isShiny; + ret.variant = this._variantTier; - for (let s = 0; s < ret.ivs.length; s++) { - ret.ivs[s] = Math.max(ret.ivs[s], secondaryIvs[s]); - } + const secondaryIvs = Utils.getIvsFromId(Utils.randSeedInt(4294967295)); + + for (let s = 0; s < ret.ivs.length; s++) { + ret.ivs[s] = Math.max(ret.ivs[s], secondaryIvs[s]); + } + }; + + ret = ret!; // Tell TS compiler it's defined now + scene.executeWithSeedOffset(() => { + generatePlayerPokemonHelper(scene); + }, this._id, EGG_SEED.toString()); return ret; } diff --git a/src/test/battle-scene.test.ts b/src/test/battle-scene.test.ts index 9e28ec99791..4da75cea197 100644 --- a/src/test/battle-scene.test.ts +++ b/src/test/battle-scene.test.ts @@ -1,5 +1,5 @@ import { LoadingScene } from "#app/loading-scene"; -import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import GameManager from "./utils/gameManager"; describe("BattleScene", () => { @@ -24,4 +24,12 @@ describe("BattleScene", () => { // `BattleScene.create()` is called during the `new GameManager()` call expect(game.scene.scene.remove).toHaveBeenCalledWith(LoadingScene.KEY); }); + + it("should also reset RNG on reset", () => { + vi.spyOn(game.scene, "resetSeed"); + + game.scene.reset(); + + expect(game.scene.resetSeed).toHaveBeenCalled(); + }); }); diff --git a/src/test/eggs/egg.test.ts b/src/test/eggs/egg.test.ts index 28f1b7f0a6c..4f00e843b47 100644 --- a/src/test/eggs/egg.test.ts +++ b/src/test/eggs/egg.test.ts @@ -12,6 +12,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vite describe("Egg Generation Tests", () => { let phaserGame: Phaser.Game; let game: GameManager; + const EGG_HATCH_COUNT: integer = 1000; beforeAll(() => { phaserGame = new Phaser.Game({ @@ -47,14 +48,21 @@ describe("Egg Generation Tests", () => { expect(result).toBe(expectedSpecies); }); - it("should hatch an Arceus. Set from legendary gacha", async () => { + it("should hatch an Arceus around half the time. Set from legendary gacha", async () => { const scene = game.scene; const timestamp = new Date(2024, 6, 10, 15, 0, 0, 0).getTime(); const expectedSpecies = Species.ARCEUS; + let gachaSpeciesCount = 0; - const result = new Egg({ scene, timestamp, sourceType: EggSourceType.GACHA_LEGENDARY, tier: EggTier.MASTER }).generatePlayerPokemon(scene).species.speciesId; + for (let i = 0; i < EGG_HATCH_COUNT; i++) { + const result = new Egg({ scene, timestamp, sourceType: EggSourceType.GACHA_LEGENDARY, tier: EggTier.MASTER }).generatePlayerPokemon(scene).species.speciesId; + if (result === expectedSpecies) { + gachaSpeciesCount++; + } + } - expect(result).toBe(expectedSpecies); + expect(gachaSpeciesCount).toBeGreaterThan(0.4 * EGG_HATCH_COUNT); + expect(gachaSpeciesCount).toBeLessThan(0.6 * EGG_HATCH_COUNT); }); it("should hatch an Arceus. Set from species", () => { const scene = game.scene; @@ -156,7 +164,7 @@ describe("Egg Generation Tests", () => { const scene = game.scene; const eggMoveIndex = new Egg({ scene }).eggMoveIndex; - const result = eggMoveIndex && eggMoveIndex >= 0 && eggMoveIndex <= 3; + const result = !Utils.isNullOrUndefined(eggMoveIndex) && eggMoveIndex >= 0 && eggMoveIndex <= 3; expect(result).toBe(true); }); @@ -309,4 +317,63 @@ describe("Egg Generation Tests", () => { expect(egg.variantTier).toBe(VariantTier.EPIC); }); + + it("should generate egg moves, species, shinyness, and ability unpredictably for the egg gacha", () => { + const scene = game.scene; + scene.setSeed("ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + scene.resetSeed(); + + const firstEgg = new Egg({scene, sourceType: EggSourceType.GACHA_SHINY, tier: EggTier.COMMON}); + const firstHatch = firstEgg.generatePlayerPokemon(scene); + let diffEggMove = false; + let diffSpecies = false; + let diffShiny = false; + let diffAbility = false; + for (let i = 0; i < EGG_HATCH_COUNT; i++) { + scene.gameData.unlockPity[EggTier.COMMON] = 0; + scene.setSeed("ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + scene.resetSeed(); // Make sure that eggs are unpredictable even if using same seed + + const newEgg = new Egg({scene, sourceType: EggSourceType.GACHA_SHINY, tier: EggTier.COMMON}); + const newHatch = newEgg.generatePlayerPokemon(scene); + diffEggMove = diffEggMove || (newEgg.eggMoveIndex !== firstEgg.eggMoveIndex); + diffSpecies = diffSpecies || (newHatch.species.speciesId !== firstHatch.species.speciesId); + diffShiny = diffShiny || (newHatch.shiny !== firstHatch.shiny); + diffAbility = diffAbility || (newHatch.abilityIndex !== firstHatch.abilityIndex); + } + + expect(diffEggMove).toBe(true); + expect(diffSpecies).toBe(true); + expect(diffShiny).toBe(true); + expect(diffAbility).toBe(true); + }); + + it("should generate egg moves, shinyness, and ability unpredictably for species eggs", () => { + const scene = game.scene; + scene.setSeed("ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + scene.resetSeed(); + + const firstEgg = new Egg({scene, species: Species.BULBASAUR}); + const firstHatch = firstEgg.generatePlayerPokemon(scene); + let diffEggMove = false; + let diffSpecies = false; + let diffShiny = false; + let diffAbility = false; + for (let i = 0; i < EGG_HATCH_COUNT; i++) { + scene.setSeed("ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + scene.resetSeed(); // Make sure that eggs are unpredictable even if using same seed + + const newEgg = new Egg({scene, species: Species.BULBASAUR}); + const newHatch = newEgg.generatePlayerPokemon(scene); + diffEggMove = diffEggMove || (newEgg.eggMoveIndex !== firstEgg.eggMoveIndex); + diffSpecies = diffSpecies || (newHatch.species.speciesId !== firstHatch.species.speciesId); + diffShiny = diffShiny || (newHatch.shiny !== firstHatch.shiny); + diffAbility = diffAbility || (newHatch.abilityIndex !== firstHatch.abilityIndex); + } + + expect(diffEggMove).toBe(true); + expect(diffSpecies).toBe(false); + expect(diffShiny).toBe(true); + expect(diffAbility).toBe(true); + }); });