Refactored BattleScene.newBattle to be less jank & split up function

src/battle-scene.ts

@@ -1,5 +1,4 @@
 import { applyAbAttrs } from "#abilities/apply-ab-attrs";
-import type { FixedBattleConfig } from "#app/battle";
 import { Battle } from "#app/battle";
 import {
   ANTI_VARIANCE_WEIGHT_MODIFIER,
@@ -121,6 +120,7 @@ import { vouchers } from "#system/voucher";
 import { trainerConfigs } from "#trainers/trainer-config";
 import type { HeldModifierConfig } from "#types/held-modifier-config";
 import type { Localizable } from "#types/locales";
+import type { SessionSaveData } from "#types/save-data";
 import { AbilityBar } from "#ui/ability-bar";
 import { ArenaFlyout } from "#ui/arena-flyout";
 import { CandyBar } from "#ui/candy-bar";
@@ -165,6 +165,35 @@ export interface InfoToggle {
   isActive(): boolean;
 }
 
+// todo move this to the file
+/** Interface representing the base type of a new battle config. */
+interface NewBattleBaseProps {
+  battleType: BattleType;
+  trainer?: Trainer;
+  trainerData?: TrainerData;
+  mysteryEncounterType?: MysteryEncounterType;
+  waveIndex: number;
+  double?: boolean;
+}
+
+/**
+ * Interface representing the resolved type of a new battle config.
+ * @interface
+ */
+export type NewBattleResolvedProps = Omit<NewBattleBaseProps, "trainerConfig" | "trainerData" | "mysteryEncounterType">;
+
+/**
+ * Interface representing the type of {@linkcode BattleScene.getNewBattleProps}, used for DRY
+ * @interface
+ */
+export type NewBattleProps = Omit<NewBattleBaseProps, "trainer">;
+
+/**
+ * The `BattleScene` is the primary scene for the game.
+ * Despite its name, it handles _everything_ other than initial asset loading,
+ * up to and including title menuing and settings handling.
+ * @todo Breakup into multiple scenes
+ */
 export class BattleScene extends SceneBase {
   public rexUI: UIPlugin;
   public inputController: InputsController;
@@ -1266,10 +1295,12 @@ export class BattleScene extends SceneBase {
     }
   }
 
-  getDoubleBattleChance(newWaveIndex: number, playerField: PlayerPokemon[]) {
+  // TODO: Invert the chances for this
+  private getDoubleBattleChance(newWaveIndex: number): number {
     const doubleChance = new NumberHolder(newWaveIndex % 10 === 0 ? 32 : 8);
     this.applyModifiers(DoubleBattleChanceBoosterModifier, true, doubleChance);
-    for (const p of playerField) {
+    for (const p of this.getPlayerField()) {
+      // TODO: This passes `null` to `applyAbAttrs`
       applyAbAttrs("DoubleBattleChanceAbAttr", { pokemon: p, chance: doubleChance });
     }
     return Math.max(doubleChance.value, 1);
@@ -1280,65 +1311,119 @@ export class BattleScene extends SceneBase {
     const isEndlessOrDaily = this.gameMode.hasShortBiomes || this.gameMode.isDaily;
     const isEndlessFifthWave = this.gameMode.hasShortBiomes && currentBattle.waveIndex % 5 === 0;
     const isWaveIndexMultipleOfFiftyMinusOne = currentBattle.waveIndex % 50 === 49;
-    const isNewBiome =
+    return isWaveIndexMultipleOfTen || isEndlessFifthWave || (isEndlessOrDaily && isWaveIndexMultipleOfFiftyMinusOne);
-      isWaveIndexMultipleOfTen || isEndlessFifthWave || (isEndlessOrDaily && isWaveIndexMultipleOfFiftyMinusOne);
-    return isNewBiome;
   }
 
-  newBattle(
+  /**
-    waveIndex?: number,
+   * @param fromSession - The {@linkcode SessionSaveData} being used to seed the battle.
-    battleType?: BattleType,
+   * Should be omitted if not loading a new save file.
-    trainerData?: TrainerData,
+   * @returns The newly created Battle instance
-    double?: boolean,
+   */
-    mysteryEncounterType?: MysteryEncounterType,
+  newBattle(fromSession?: SessionSaveData): Battle {
-  ): Battle {
+    const props = this.getNewBattleProps(fromSession);
-    const _startingWave = Overrides.STARTING_WAVE_OVERRIDE || startingWave;
+    const foo: Partial<NewBattleResolvedProps> = {};
-    const newWaveIndex = waveIndex || (this.currentBattle?.waveIndex || _startingWave - 1) + 1;
+    const { waveIndex } = props;
-    let newDouble: boolean | undefined;
-    let newBattleType: BattleType;
-    let newTrainer: Trainer | undefined;
 
-    let battleConfig: FixedBattleConfig | null = null;
+    this.resetSeed(waveIndex);
 
-    this.resetSeed(newWaveIndex);
+    // First, check if it's a fixed wave and do stuff accordingly
+    if (this.gameMode.isFixedBattle(waveIndex)) {
+      this.handleFixedBattle(foo, waveIndex);
+    } else if (props.trainerData) {
+      this.handleSavedBattle(foo, props);
+    } else {
+      this.handleNonFixedBattle(foo, props);
+    }
 
-    const playerField = this.getPlayerField();
+    foo.double = this.checkIsDouble(foo, props);
 
-    if (this.gameMode.isFixedBattle(newWaveIndex) && trainerData === undefined) {
+    const lastBattle = this.currentBattle;
-      battleConfig = this.gameMode.getFixedBattle(newWaveIndex);
+    const maxExpLevel = this.getMaxExpLevel();
-      newDouble = battleConfig.double;
+
-      newBattleType = battleConfig.battleType;
+    this.lastEnemyTrainer = lastBattle?.trainer ?? null;
+    this.lastMysteryEncounter = lastBattle?.mysteryEncounter;
+
+    // TODO: Is this even needed?
+    if (lastBattle?.double && !foo.double) {
+      this.phaseManager.tryRemovePhase((p: Phase) => p.is("SwitchPhase"));
+      // TODO: We already do this later in the function
+      for (const p of this.getPlayerField()) {
+        p.lapseTag(BattlerTagType.COMMANDED);
+      }
+    }
+
+    // NB: Type assertion is fine as foo should always be defined
     this.executeWithSeedOffset(
-        () => (newTrainer = battleConfig?.getTrainer()),
+      () => {
-        (battleConfig.seedOffsetWaveIndex || newWaveIndex) << 8,
+        this.currentBattle = new Battle(this.gameMode, foo as NewBattleResolvedProps);
+      },
+      waveIndex << 3, // TODO: Why use this specific index?
+      this.waveSeed,
     );
-      if (newTrainer) {
+    this.currentBattle.incrementTurn();
-        this.field.add(newTrainer);
+
+    this.currentBattle.mysteryEncounterType = props.mysteryEncounterType;
+
+    if (fromSession && lastBattle) {
+      this.doPostBattleCleanup(lastBattle, maxExpLevel);
     }
-    } else {
+    return this.currentBattle;
-      if (
-        !this.gameMode.hasTrainers
-        || Overrides.BATTLE_TYPE_OVERRIDE === BattleType.WILD
-        || (Overrides.DISABLE_STANDARD_TRAINERS_OVERRIDE && trainerData == null)
-      ) {
-        newBattleType = BattleType.WILD;
-      } else {
-        newBattleType =
-          Overrides.BATTLE_TYPE_OVERRIDE
-          ?? battleType
-          ?? (this.gameMode.isWaveTrainer(newWaveIndex, this.arena) ? BattleType.TRAINER : BattleType.WILD);
   }
 
-      if (newBattleType === BattleType.TRAINER) {
+  // TODO: Document these and stop
-        const trainerType =
+  private handleFixedBattle(foo: Partial<NewBattleResolvedProps>, waveIndex: number): Trainer {
-          Overrides.RANDOM_TRAINER_OVERRIDE?.trainerType ?? this.arena.randomTrainerType(newWaveIndex);
+    let t: Trainer;
-        let doubleTrainer = false;
+    const battleConfig = this.gameMode.getFixedBattle(waveIndex)!;
+    foo.double = battleConfig.double;
+    foo.battleType = battleConfig.battleType;
+    this.executeWithSeedOffset(
+      () => {
+        t = battleConfig.getTrainer();
+      },
+      (battleConfig.seedOffsetWaveIndex || waveIndex) << 8,
+    );
+    // Tell TS this is defined
+    this.field.add(t!);
+    return t!;
+  }
+
+  private handleSavedBattle(foo: Partial<NewBattleResolvedProps>, props: NewBattleProps): void {
+    foo.battleType = props.battleType;
+    foo.double = props.double;
+    foo.trainer = props.trainerData?.toTrainer();
+    foo.waveIndex = props.waveIndex;
+  }
+
+  private handleNonFixedBattle(foo: Partial<NewBattleResolvedProps>, { waveIndex, battleType }: NewBattleProps): void {
+    battleType =
+      !this.gameMode.hasTrainers || Overrides.DISABLE_STANDARD_TRAINERS_OVERRIDE
+        ? BattleType.WILD
+        : (Overrides.BATTLE_TYPE_OVERRIDE
+          ?? (this.gameMode.isWaveTrainer(waveIndex, this.arena) ? BattleType.TRAINER : BattleType.WILD));
+
+    // Check for mystery encounter
+    // Can only occur in place of a standard (non-boss) wild battle, waves 10-180
+    if (this.isWaveMysteryEncounter(battleType, waveIndex)) {
+      foo.battleType = BattleType.MYSTERY_ENCOUNTER;
+      // Reset to base spawn weight
+      this.mysteryEncounterSaveData.encounterSpawnChance = BASE_MYSTERY_ENCOUNTER_SPAWN_WEIGHT;
+      return;
+    }
+
+    if (battleType !== BattleType.TRAINER) {
+      return;
+    }
+
+    // Determine the trainer's attributes
+    const trainerType = Overrides.RANDOM_TRAINER_OVERRIDE?.trainerType ?? this.arena.randomTrainerType(waveIndex);
+    let doubleTrainer: boolean;
     if (trainerConfigs[trainerType].doubleOnly) {
       doubleTrainer = true;
-        } else if (trainerConfigs[trainerType].hasDouble) {
+    } else if (!trainerConfigs[trainerType].hasDouble) {
+      doubleTrainer = false;
+    } else {
       doubleTrainer =
-            Overrides.RANDOM_TRAINER_OVERRIDE?.alwaysDouble
+        Overrides.RANDOM_TRAINER_OVERRIDE?.alwaysDouble || !randSeedInt(this.getDoubleBattleChance(waveIndex));
-            || !randSeedInt(this.getDoubleBattleChance(newWaveIndex, playerField));
       // Add a check that special trainers can't be double except for tate and liza - they should use the normal double chance
       if (
         trainerConfigs[trainerType].trainerTypeDouble
@@ -1347,106 +1432,41 @@ export class BattleScene extends SceneBase {
         doubleTrainer = false;
       }
     }
+
     const variant = doubleTrainer
       ? TrainerVariant.DOUBLE
       : randSeedInt(2)
         ? TrainerVariant.FEMALE
         : TrainerVariant.DEFAULT;
-        newTrainer = trainerData !== undefined ? trainerData.toTrainer() : new Trainer(trainerType, variant);
+    const trainer = new Trainer(trainerType, variant);
-        this.field.add(newTrainer);
+    this.field.add(trainer);
+    foo.trainer = trainer;
   }
 
-      // Check for mystery encounter
+  private getNewBattleProps(fromSession?: SessionSaveData): NewBattleProps {
-      // Can only occur in place of a standard (non-boss) wild battle, waves 10-180
+    const battleType = fromSession?.battleType ?? BattleType.WILD;
-      if (
+    const mysteryEncounterType =
-        !Overrides.BATTLE_TYPE_OVERRIDE
+      fromSession?.mysteryEncounterType != null && fromSession?.mysteryEncounterType !== -1
-        && (this.isWaveMysteryEncounter(newBattleType, newWaveIndex) || newBattleType === BattleType.MYSTERY_ENCOUNTER)
+        ? fromSession?.mysteryEncounterType
-      ) {
+        : undefined;
-        newBattleType = BattleType.MYSTERY_ENCOUNTER;
+    // Don't increment wave index when computing starting wave
-        // Reset to base spawn weight
+    const newWaveIndex =
-        this.mysteryEncounterSaveData.encounterSpawnChance = BASE_MYSTERY_ENCOUNTER_SPAWN_WEIGHT;
+      (fromSession?.waveIndex ?? this.currentBattle?.waveIndex) != null
-      }
+        ? (fromSession?.waveIndex ?? this.currentBattle?.waveIndex) + 1
+        : (Overrides.STARTING_WAVE_OVERRIDE ?? startingWave);
+    const trainerData = fromSession?.trainer;
+    const fixedDouble =
+      fromSession == null
+        ? undefined
+        : battleType === BattleType.TRAINER
+          ? trainerConfigs[fromSession?.trainer.trainerType]?.doubleOnly
+            || fromSession.trainer?.variant === TrainerVariant.DOUBLE
+          : battleType !== BattleType.MYSTERY_ENCOUNTER && fromSession.enemyParty.length > 1;
+
+    return { battleType, mysteryEncounterType, waveIndex: newWaveIndex, trainerData, double: fixedDouble };
   }
 
-    if (double === undefined && newWaveIndex > 1) {
+  private doPostBattleCleanup(lastBattle: Battle, maxExpLevel: number): void {
-      if (newBattleType === BattleType.WILD && !this.gameMode.isWaveFinal(newWaveIndex)) {
-        newDouble = !randSeedInt(this.getDoubleBattleChance(newWaveIndex, playerField));
-      } else if (newBattleType === BattleType.TRAINER) {
-        newDouble = newTrainer?.variant === TrainerVariant.DOUBLE;
-      }
-    } else if (!battleConfig) {
-      newDouble = !!double;
-    }
-
-    // Disable double battles on Endless/Endless Spliced Wave 50x boss battles (Introduced 1.2.0)
-    if (this.gameMode.isEndlessBoss(newWaveIndex)) {
-      newDouble = false;
-    }
-
-    if (Overrides.BATTLE_STYLE_OVERRIDE != null) {
-      let doubleOverrideForWave: "single" | "double" | null = null;
-
-      switch (Overrides.BATTLE_STYLE_OVERRIDE) {
-        case "double":
-          doubleOverrideForWave = "double";
-          break;
-        case "single":
-          doubleOverrideForWave = "single";
-          break;
-        case "even-doubles":
-          doubleOverrideForWave = newWaveIndex % 2 ? "single" : "double";
-          break;
-        case "odd-doubles":
-          doubleOverrideForWave = newWaveIndex % 2 ? "double" : "single";
-          break;
-      }
-
-      if (doubleOverrideForWave === "double") {
-        newDouble = true;
-      }
-      /**
-       * Override battles into single only if not fighting with trainers.
-       * @see {@link https://github.com/pagefaultgames/pokerogue/issues/1948 GitHub Issue #1948}
-       */
-      if (newBattleType !== BattleType.TRAINER && doubleOverrideForWave === "single") {
-        newDouble = false;
-      }
-    }
-
-    const lastBattle = this.currentBattle;
-
-    const maxExpLevel = this.getMaxExpLevel();
-
-    this.lastEnemyTrainer = lastBattle?.trainer ?? null;
-    this.lastMysteryEncounter = lastBattle?.mysteryEncounter;
-
-    if (newBattleType === BattleType.MYSTERY_ENCOUNTER) {
-      // Disable double battle on mystery encounters (it may be re-enabled as part of encounter)
-      newDouble = false;
-    }
-
-    if (lastBattle?.double && !newDouble) {
-      this.phaseManager.tryRemovePhase((p: Phase) => p.is("SwitchPhase"));
-      for (const p of this.getPlayerField()) {
-        p.lapseTag(BattlerTagType.COMMANDED);
-      }
-    }
-
-    this.executeWithSeedOffset(
-      () => {
-        this.currentBattle = new Battle(this.gameMode, newWaveIndex, newBattleType, newTrainer, newDouble);
-      },
-      newWaveIndex << 3,
-      this.waveSeed,
-    );
-    this.currentBattle.incrementTurn();
-
-    if (newBattleType === BattleType.MYSTERY_ENCOUNTER) {
-      // Will generate the actual Mystery Encounter during NextEncounterPhase, to ensure it uses proper biome
-      this.currentBattle.mysteryEncounterType = mysteryEncounterType;
-    }
-
-    if (!waveIndex && lastBattle) {
     const isNewBiome = this.isNewBiome(lastBattle);
     /** Whether to reset and recall pokemon */
     const resetArenaState =
@@ -1458,17 +1478,14 @@ export class BattleScene extends SceneBase {
       enemyPokemon.destroy();
     }
     this.trySpreadPokerus();
-      if (!isNewBiome && newWaveIndex % 10 === 5) {
+    if (!isNewBiome && this.currentBattle.waveIndex % 10 === 5) {
       this.arena.updatePoolsForTimeOfDay();
     }
     if (resetArenaState) {
       this.arena.resetArenaEffects();
 
-        for (const pokemon of playerField) {
+      this.getPlayerField().forEach((pokemon, p) => {
         pokemon.lapseTag(BattlerTagType.COMMANDED);
-        }
-
-        playerField.forEach((pokemon, p) => {
         if (pokemon.isOnField()) {
           this.phaseManager.pushNew("ReturnPhase", p);
         }
@@ -1505,7 +1522,39 @@ export class BattleScene extends SceneBase {
     }
   }
 
-    return this.currentBattle;
+  /** Sub-method of `newBattle` that returns whether the new battle is a double battle. */
+  private checkIsDouble(
+    { trainer }: Partial<NewBattleResolvedProps>,
+    { double, battleType, waveIndex }: NewBattleProps,
+  ): boolean {
+    // Edge cases
+    if (
+      waveIndex === 1 // Wave 1 doubles cause crashes
+      || this.gameMode.isWaveFinal(waveIndex)
+      || this.gameMode.isEndlessBoss(waveIndex)
+      || battleType === BattleType.MYSTERY_ENCOUNTER
+    ) {
+      return false;
+    }
+    if (double != null) {
+      return double;
+    }
+
+    switch (Overrides.BATTLE_STYLE_OVERRIDE) {
+      case "double":
+        return true;
+      case "single":
+        return false;
+      case "even-doubles":
+        return waveIndex % 2 === 0;
+      case "odd-doubles":
+        return waveIndex % 2 === 1;
+    }
+
+    if (battleType === BattleType.WILD) {
+      return !randSeedInt(this.getDoubleBattleChance(waveIndex));
+    }
+    return trainer?.variant === TrainerVariant.DOUBLE;
   }
 
   newArena(biome: BiomeId, playerFaints = 0): Arena {

src/battle.ts

@@ -1,3 +1,4 @@
+import type { NewBattleResolvedProps } from "#app/battle-scene";
 import type { GameMode } from "#app/game-mode";
 import { globalScene } from "#app/global-scene";
 import { ArenaTagType } from "#enums/arena-tag-type";
@@ -56,7 +57,7 @@ export class Battle {
   public waveIndex: number;
   public battleType: BattleType;
   public battleSpec: BattleSpec;
-  public trainer: Trainer | null;
+  public trainer: Trainer | undefined;
   public enemyLevels: number[] | undefined;
   public enemyParty: EnemyPokemon[] = [];
   public seenEnemyPartyMemberIds: Set<number> = new Set<number>();
@@ -99,11 +100,11 @@ export class Battle {
 
   private rngCounter = 0;
 
-  constructor(gameMode: GameMode, waveIndex: number, battleType: BattleType, trainer?: Trainer, double = false) {
+  constructor(gameMode: GameMode, { waveIndex, battleType, trainer, double = false }: NewBattleResolvedProps) {
     this.gameMode = gameMode;
     this.waveIndex = waveIndex;
     this.battleType = battleType;
-    this.trainer = trainer ?? null;
+    this.trainer = trainer;
     this.initBattleSpec();
     this.enemyLevels =
       battleType !== BattleType.TRAINER
@@ -469,13 +470,12 @@ export class Battle {
 
 export class FixedBattle extends Battle {
   constructor(waveIndex: number, config: FixedBattleConfig) {
-    super(
+    super(globalScene.gameMode, {
-      globalScene.gameMode,
       waveIndex,
-      config.battleType,
+      battleType: config.battleType,
-      config.battleType === BattleType.TRAINER ? config.getTrainer() : undefined,
+      trainer: config.battleType === BattleType.TRAINER ? config.getTrainer() : undefined,
-      config.double,
+      double: config.double,
-    );
+    });
     if (config.getEnemyParty) {
       this.enemyParty = config.getEnemyParty();
     }

src/data/trainers/trainer-party-template.ts

@@ -1,5 +1,5 @@
 import { globalScene } from "#app/global-scene";
-import { startingWave } from "#app/starting-wave";
+import { startingWave } from "#balance/misc";
 import { ClassicFixedBossWaves } from "#enums/fixed-boss-waves";
 import { GameModes } from "#enums/game-modes";
 import { PartyMemberStrength } from "#enums/party-member-strength";

src/game-mode.ts

@@ -265,7 +265,7 @@ export class GameMode implements GameModeConfig {
         return waveIndex === 200;
       case GameModes.ENDLESS:
       case GameModes.SPLICED_ENDLESS:
-        return !(waveIndex % 250);
+        return waveIndex % 250 === 0;
       case GameModes.DAILY:
         return waveIndex === 50;
     }

src/overrides.ts

@@ -290,7 +290,10 @@ class DefaultOverrides {
    */
   readonly ITEM_REWARD_OVERRIDE: ModifierOverride[] = [];
 
-  /** If `true`, disable all non-scripted opponent trainer encounters. */
+  /**
+   * If `true`, disable all non-scripted opponent trainer encounters.
+   * @todo Meld into `BATTLE_TYPE_OVERRIDE`
+  */
   readonly DISABLE_STANDARD_TRAINERS_OVERRIDE: boolean = false;
 
   /**

src/system/game-data.ts

@@ -57,7 +57,6 @@ import {
   applySystemVersionMigration,
 } from "#system/version-migration/version-converter";
 import { VoucherType, vouchers } from "#system/voucher";
-import { trainerConfigs } from "#trainers/trainer-config";
 import type { DexData, DexEntry } from "#types/dex-data";
 import type {
   AchvUnlocks,
@@ -984,19 +983,8 @@ export class GameData {
 
         globalScene.newArena(fromSession.arena.biome, fromSession.playerFaints);
 
-        const battleType = fromSession.battleType || 0;
+        const battleType = fromSession.battleType ?? BattleType.WILD;
-        const trainerConfig = fromSession.trainer ? trainerConfigs[fromSession.trainer.trainerType] : null;
+        const battle = globalScene.newBattle(fromSession);
-        const mysteryEncounterType =
-          fromSession.mysteryEncounterType !== -1 ? fromSession.mysteryEncounterType : undefined;
-        const battle = globalScene.newBattle(
-          fromSession.waveIndex,
-          battleType,
-          fromSession.trainer,
-          battleType === BattleType.TRAINER
-            ? trainerConfig?.doubleOnly || fromSession.trainer?.variant === TrainerVariant.DOUBLE
-            : fromSession.enemyParty.length > 1,
-          mysteryEncounterType,
-        );
         battle.enemyLevels = fromSession.enemyParty.map(p => p.level);
 
         globalScene.arena.init();

test/test-utils/game-manager-utils.ts

@@ -111,5 +111,10 @@ export function initSceneWithoutEncounterPhase(scene: BattleScene, species?: Spe
     scene.getPlayerParty().push(starterPokemon);
   });
 
-  scene.currentBattle = new Battle(getGameMode(GameModes.CLASSIC), 5, BattleType.WILD, undefined, false);
+  scene.currentBattle = new Battle(getGameMode(GameModes.CLASSIC), {
+    waveIndex: 5,
+    battleType: BattleType.WILD,
+    trainer: undefined,
+    double: false,
+  });
 }