fix(move): resolve Parting Shot failing logic and switch out bugs#7152
fix(move): resolve Parting Shot failing logic and switch out bugs#7152EduFabrizio wants to merge 1 commit intopagefaultgames:betafrom
Conversation
Madmadness65
added
Move
| override apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean { | ||
| const statDropTriggered = super.apply(user, target, move, args); | ||
|
|
||
| if (statDropTriggered) { | ||
| let willDrop = false; | ||
|
|
||
| const isBlockedByMist = !!globalScene.arena.getTagOnSide( | ||
| ArenaTagType.MIST, | ||
| target.isPlayer() ? ArenaTagSide.PLAYER : ArenaTagSide.ENEMY, | ||
| ); | ||
|
|
||
| if (!isBlockedByMist) { | ||
| const hasContrary = target.hasAbility(AbilityId.CONTRARY); | ||
| const stageMod = hasContrary ? 1 : -1; | ||
|
|
||
| // Silently simulate immunities to predict if the stat drop will be blocked | ||
| const cancelledAtk = new BooleanHolder(false); | ||
| applyAbAttrs("ProtectStatAbAttr", { | ||
| pokemon: target, | ||
| stat: Stat.ATK, | ||
| stages: -1, | ||
| cancelled: cancelledAtk, | ||
| simulated: true, | ||
| }); | ||
| const canChangeAtk = | ||
| !cancelledAtk.value | ||
| && target.getStatStage(Stat.ATK) + stageMod >= -6 | ||
| && target.getStatStage(Stat.ATK) + stageMod <= 6; | ||
|
|
||
| const cancelledSpAtk = new BooleanHolder(false); | ||
| applyAbAttrs("ProtectStatAbAttr", { | ||
| pokemon: target, | ||
| stat: Stat.SPATK, | ||
| stages: -1, | ||
| cancelled: cancelledSpAtk, | ||
| simulated: true, | ||
| }); | ||
| const canChangeSpAtk = | ||
| !cancelledSpAtk.value | ||
| && target.getStatStage(Stat.SPATK) + stageMod >= -6 | ||
| && target.getStatStage(Stat.SPATK) + stageMod <= 6; | ||
|
|
||
| willDrop = canChangeAtk || canChangeSpAtk; | ||
| } |
There was a problem hiding this comment.
@EduFabrizio
IIRC, these condition checks should be placed inside getCondition as otherwise the move will no-op without "failing" (for the handful of moves and effects that care about it).
There was a problem hiding this comment.
@Bertie690 Thank you very much for the review and suggestions! I have pushed an update that implements your requested changes:
Denested the apply method logic using early returns.
Stored ForceSwitchOutAttr privately in the class constructor rather than re-creating it every time.
Moved the detailed explanation to the class description and added the TODO regarding the duplicated stat checks.
Regarding the suggestion to move the condition checks into getCondition():
I looked into doing this, but moving these checks to .condition() causes the move to fail outright and bypasses the ShowAbilityPhase entirely. If we do that, the player won't see the ability pop-up (e.g., "Clear Body prevented stat loss"), which breaks parity with the official games (this was a blocker point raised in a previous PR attempt for this move #3471).
By keeping it inside apply() and using the simulated: true check, we allow the base StatStageChangeAttr to naturally queue the ShowAbilityPhase if an immunity is hit, while accurately preventing the switch-out. Let me know if this denested approach looks good to you!
Fixes bug where Parting Shot unconditionally fails if the user has no eligible party members. Reconstructs PartingShotAttr to extend StatStageChangeAttr, simulating stat immunity (Clear Body, etc) internally before queueing the SwitchPhase. Also fixes existing automated tests logic. Signed-off-by: Eduardo Siqueira <eduardosiqueira@tecnico.ulisboa.pt>
EduFabrizio
force-pushed
the
fix/parting-shot
branch
from
ad8b0f8 to
75b1369
Compare
|
Hi @Bertie690 . I just wanted to drop a quick ping to let you know that I've pushed the requested changes (denesting the logic, optimizing the attribute instantiation, and updating the docs/TODOs) and replied to the getCondition() thread above. Whenever you have a moment, please let me know if the updates look good to you or if there is anything else you'd like me to adjust. Thanks again for your time and the great feedback! |
3 participants
Closes #5379
What are the changes the user will see?
Parting Shotwill no longer say "But it failed!" when the user has no eligible Pokémon in their party to switch into. It will correctly drop the target's stats and end the turn.Clear Body,Mist,Good as Gold, or if the target's stats are already at-6/-6.Substitute(due to being sound-based).Why am I making these changes?
To resolve the core logic bugs associated with Parting Shot. The cause of the bug was that the move's attributes were decoupled. The switch was triggered unconditionally by
ForceSwitchOutAttr, while the move would also unconditionally fail if the party was empty due to the same attribute's default conditions blocking the execution from the start.What are the changes from a developer perspective?
ForceSwitchOutAttrfrom the base move definition. Created a customPartingShotAttrthat directly inherits fromStatStageChangeAttr.apply(), the move now silently predicts if the stat-drop will be blocked by usingapplyAbAttrs(..., { simulated: true })and checking stat bounds. TheSwitchPhaseis conditionally queued only if this simulation guarantees a successful drop and the user has a valid party.getUserBenefitScoreto sum both the stat-drop and switch tactical scores, preserving the enemy AI's ability to evaluate the move correctly..todoautomated tests (which suffered fromFaintPhaseGameManager desyncs and assertion typos) and added new deterministic test coverage for empty parties,Contrary, andSubstitute.Screenshots/Videos
https://youtu.be/TBsUvtFkbv0
How to test the changes?
Run the automated test suite locally:
pnpm run test:silent test/tests/moves/parting-shot.test.tsChecklist
The PR content is correctly formatted:
betaas my base branchbeta,mainor the name of another long-lived feature branchThe PR has been confirmed to work correctly:
pnpm test:silentto test locally)pnpm test:create) or updated existing tests related to the PR's changes if necessaryAre there any localization additions or changes? If so:
Does this require any additions or changes to in-game assets? If so: