From f317ec9a263b30a6ea0a6f4c21318f59ad0c4500 Mon Sep 17 00:00:00 2001 From: Bertie690 <136088738+Bertie690@users.noreply.github.com> Date: Sat, 20 Dec 2025 18:55:23 -0500 Subject: [PATCH] [Bug] Avoid pre-emptively leaving the field when forcibly switching out https://github.com/pagefaultgames/pokerogue/pull/6874 * [Bug] Avoid pre-emptively leaving the field when forcibly switching out * Fix `SwitchPhase` off-by-one error * Add check for pokemon fainting in test * add test for destiny bond crash * Fix `queueDeferred` not respecting pending `FaintPhase`s TL;DR we would defer the faint phases to run after the switch sequences, which is wrong - leaving the field has to be the LAST thing that happens in a given turn (or else shit breaks big-time). * Update test/moves/u-turn.test.ts --------- Co-authored-by: NightKev <34855794+DayKev@users.noreply.github.com> Co-authored-by: Fabi <192151969+fabske0@users.noreply.github.com> --- src/data/abilities/ability.ts | 2 -- src/data/battle-anims.ts | 2 ++ src/data/moves/move.ts | 3 --- src/phase-manager.ts | 2 +- src/phase-tree.ts | 9 ------- src/phases/switch-phase.ts | 18 ++++++-------- src/phases/switch-summon-phase.ts | 12 +++++++++ test/moves/u-turn.test.ts | 41 ++++++++++++++++++++++++++++++- 8 files changed, 62 insertions(+), 27 deletions(-) diff --git a/src/data/abilities/ability.ts b/src/data/abilities/ability.ts index 348764d4659..681238f7b2b 100644 --- a/src/data/abilities/ability.ts +++ b/src/data/abilities/ability.ts @@ -6309,7 +6309,6 @@ class ForceSwitchOutHelper { } if (switchOutTarget.hp > 0) { - switchOutTarget.leaveField(this.switchType === SwitchType.SWITCH); globalScene.phaseManager.queueDeferred( "SwitchPhase", this.switchType, @@ -6328,7 +6327,6 @@ class ForceSwitchOutHelper { return false; } if (switchOutTarget.hp > 0) { - switchOutTarget.leaveField(this.switchType === SwitchType.SWITCH); const summonIndex = globalScene.currentBattle.trainer ? globalScene.currentBattle.trainer.getNextSummonIndex((switchOutTarget as EnemyPokemon).trainerSlot) : 0; diff --git a/src/data/battle-anims.ts b/src/data/battle-anims.ts index fb9082d6ef6..e6f73eb0ce6 100644 --- a/src/data/battle-anims.ts +++ b/src/data/battle-anims.ts @@ -14,6 +14,8 @@ import { getEnumKeys, getEnumValues } from "#utils/enums"; import { toKebabCase } from "#utils/strings"; import Phaser from "phaser"; +// TODO: Split up this entire file - it has way WAY too much stuff for its own good. +// (Also happens to be positively spaghetti, but that's besides the point) export class AnimConfig { public id: number; public graphic: string; diff --git a/src/data/moves/move.ts b/src/data/moves/move.ts index cc646539dba..042ff9af44c 100644 --- a/src/data/moves/move.ts +++ b/src/data/moves/move.ts @@ -6982,7 +6982,6 @@ export class ForceSwitchOutAttr extends MoveEffectAttr { if (switchOutTarget.hp > 0) { if (this.switchType === SwitchType.FORCE_SWITCH) { - switchOutTarget.leaveField(true); const slotIndex = eligibleNewIndices[user.randBattleSeedInt(eligibleNewIndices.length)]; globalScene.phaseManager.queueDeferred( "SwitchSummonPhase", @@ -7027,7 +7026,6 @@ export class ForceSwitchOutAttr extends MoveEffectAttr { if (switchOutTarget.hp > 0) { if (this.switchType === SwitchType.FORCE_SWITCH) { - switchOutTarget.leaveField(true); const slotIndex = eligibleNewIndices[user.randBattleSeedInt(eligibleNewIndices.length)]; globalScene.phaseManager.queueDeferred( "SwitchSummonPhase", @@ -7038,7 +7036,6 @@ export class ForceSwitchOutAttr extends MoveEffectAttr { false, ); } else { - switchOutTarget.leaveField(this.switchType === SwitchType.SWITCH); globalScene.phaseManager.queueDeferred( "SwitchSummonPhase", this.switchType, diff --git a/src/phase-manager.ts b/src/phase-manager.ts index 5f1637d1378..776aebef763 100644 --- a/src/phase-manager.ts +++ b/src/phase-manager.ts @@ -536,7 +536,7 @@ export class PhaseManager { phase: T, ...args: ConstructorParameters ): void { - this.phaseQueue.unshiftToCurrent(this.create(phase, ...args)); + this.phaseQueue.addPhase(this.create(phase, ...args), true); } /** diff --git a/src/phase-tree.ts b/src/phase-tree.ts index f348f89818c..02db3d5bd1d 100644 --- a/src/phase-tree.ts +++ b/src/phase-tree.ts @@ -78,15 +78,6 @@ export class PhaseTree { this.addPhase(phase); } - /** - * Unshifts a {@linkcode Phase} to the current level. - * This is effectively the same as if the phase were added immediately after the currently-running phase, before it started. - * @param phase - The {@linkcode Phase} to be added - */ - public unshiftToCurrent(phase: Phase): void { - this.levels[this.currentLevel].unshift(phase); - } - /** * Pushes a {@linkcode Phase} to the last level of the queue. It will run only after all previously queued phases have been executed. * @param phase - The {@linkcode Phase} to be added diff --git a/src/phases/switch-phase.ts b/src/phases/switch-phase.ts index 9ab06ec827c..fe856e56f4a 100644 --- a/src/phases/switch-phase.ts +++ b/src/phases/switch-phase.ts @@ -22,7 +22,8 @@ export class SwitchPhase extends BattlePhase { * @param isModal Indicates if the switch should be forced (true) or is * optional (false). * @param doReturn Indicates if the party member on the field should be - * recalled to ball or has already left the field. Passed to {@linkcode SwitchSummonPhase}. + * recalled to ball or has already left the field. Passed to {@linkcode SwitchSummonPhase}, + * and is (ostensibly) only set to `false` from `FaintPhase`. */ constructor(switchType: SwitchType, fieldIndex: number, isModal: boolean, doReturn: boolean) { super(); @@ -36,11 +37,8 @@ export class SwitchPhase extends BattlePhase { start() { super.start(); - // Skip modal switch if impossible (no remaining party members that aren't in battle) - if ( - this.isModal - && globalScene.getPlayerParty().filter(p => p.isAllowedInBattle() && !p.isActive(true)).length === 0 - ) { + // Skip modal switch if impossible (no remaining party members that aren't already in battle) + if (this.isModal && globalScene.getPokemonAllowedInBattle().every(p => p.isOnField())) { return super.end(); } @@ -51,16 +49,14 @@ export class SwitchPhase extends BattlePhase { * if the mon should have already been returned but is still alive and well * on the field. see also; battle.test.ts */ + // TODO: If a Phasing move kills its own user, when does said user appear on field? + // Is it after the user faints if (this.isModal && !this.doReturn && !globalScene.getPlayerParty()[this.fieldIndex].isFainted()) { return super.end(); } // Check if there is any space still in field - if ( - this.isModal - && globalScene.getPlayerField().filter(p => p.isAllowedInBattle() && p.isActive(true)).length - >= globalScene.currentBattle.getBattlerCount() - ) { + if (this.isModal && globalScene.getPlayerField(true).length > globalScene.currentBattle.getBattlerCount()) { return super.end(); } diff --git a/src/phases/switch-summon-phase.ts b/src/phases/switch-summon-phase.ts index 9ccc3cc573e..e9834351015 100644 --- a/src/phases/switch-summon-phase.ts +++ b/src/phases/switch-summon-phase.ts @@ -126,6 +126,18 @@ export class SwitchSummonPhase extends SummonPhase { switchedInPokemon.resetSummonData(); switchedInPokemon.loadAssets(true); + // Even more defensive programming: Some callers will or will not make their users leave the field + // before this phase starts. + // To account for this (and avoid crashes by leaving the field during move processing), + // forcibly ensure the the victim is off of the field if they have not already done so. + // TODO: This means the switch out will occur immediately from U-turn's effect if the U-Turn user faints + // (instead of happening at end of turn from an empty slot). + // That being said, this blemish becomes completely irrelevant + // once #6611 burns the entire system to the ground. + if (this.lastPokemon.isOnField()) { + this.lastPokemon.leaveField(this.switchType === SwitchType.SWITCH); + } + applyAbAttrs("PreSummonAbAttr", { pokemon: switchedInPokemon }); applyAbAttrs("PreSwitchOutAbAttr", { pokemon: this.lastPokemon }); if (!switchedInPokemon) { diff --git a/test/moves/u-turn.test.ts b/test/moves/u-turn.test.ts index 25c333e58e1..96f683d0cb4 100644 --- a/test/moves/u-turn.test.ts +++ b/test/moves/u-turn.test.ts @@ -1,4 +1,5 @@ import { AbilityId } from "#enums/ability-id"; +import { BattlerIndex } from "#enums/battler-index"; import { MoveId } from "#enums/move-id"; import { SpeciesId } from "#enums/species-id"; import { StatusEffect } from "#enums/status-effect"; @@ -24,7 +25,7 @@ describe("Moves - U-turn", () => { game = new GameManager(phaserGame); game.override .battleStyle("single") - .enemySpecies(SpeciesId.GENGAR) + .enemySpecies(SpeciesId.MAGIKARP) .startingLevel(90) .startingWave(97) .moveset([MoveId.U_TURN]) @@ -103,4 +104,42 @@ describe("Moves - U-turn", () => { expect(game.phaseInterceptor.log).toContain("SwitchSummonPhase"); expect(game.field.getPlayerPokemon().species.speciesId).toBe(SpeciesId.SHUCKLE); }); + + it("should not crash when KOing the user from a reactive effect", async () => { + game.override.enemyAbility(AbilityId.ROUGH_SKIN); + await game.classicMode.startBattle([SpeciesId.SHEDINJA, SpeciesId.FEEBAS]); + + const player1 = game.field.getPlayerPokemon(); + + game.move.use(MoveId.U_TURN); + game.doSelectPartyPokemon(1); + await game.toEndOfTurn(); + + expect(game.field.getPlayerPokemon().species.speciesId).toBe(SpeciesId.FEEBAS); + expect(player1).toHaveFainted(); + }); + + it("should not crash when KOing the user via Destiny Bond", async () => { + await game.classicMode.startBattle([SpeciesId.FEEBAS, SpeciesId.MILOTIC]); + + const feebas = game.field.getPlayerPokemon(); + const karp = game.field.getEnemyPokemon(); + karp.hp = 1; + + game.move.use(MoveId.U_TURN); + game.doSelectPartyPokemon(1); + await game.move.forceEnemyMove(MoveId.DESTINY_BOND); + await game.setTurnOrder([BattlerIndex.ENEMY, BattlerIndex.PLAYER]); + await game.toEndOfTurn(); + + expect(karp).toHaveFainted(); + expect(feebas).toHaveFainted(); + expect(feebas.isOnField()).toBe(false); + + // Make sure feebas' faint phase runs before being switched out (since that was the root cause of the crash) + const logs = game.phaseInterceptor.log; + expect(logs).toContain("SwitchSummonPhase"); + expect(logs).toContain("FaintPhase"); + expect(logs.indexOf("SwitchSummonPhase")).toBeGreaterThan(logs.indexOf("FaintPhase")); + }); });