Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash with Psych Up, Focus Energy, and Dragon Cheer #10573

Merged

Conversation

Eric-w-H
Copy link
Contributor

If a pokemon under the effect of focus energy tries to copy the dragon cheer effect, the server crashes. Removing the possible crit stage modifiers first protects against dragon cheer trying to set the property of a volatile that was never successfully added.

I didn't test this for faithfulness against the in-game behavior.

Replication instructions:
start a local server.
create two doubles teams. One must have a mon that can use both focus energy and psych up, e.g. hydreigon. The other must have a pokemon that can use dragon cheer. Challenge yourself with the two teams.
Turn 1: use dragon cheer and focus energy
Turn 2: use psych up from the mon under focus energy, targeting the opposing mon under the effect of dragon cheer

Sample crash log/replay below:

CRASH: TypeError: Cannot set properties of undefined (setting 'hasDragonType')
    at Battle.onHit (/.../pokemon-showdown/data/moves.ts:14550:65)
    at Battle.singleEvent (/.../pokemon-showdown/sim/battle.ts:585:25)
    at BattleActions.runMoveEffects (/.../pokemon-showdown/sim/battle-actions.ts:1305:31)
    at BattleActions.spreadMoveHit (/.../pokemon-showdown/sim/battle-actions.ts:1118:17)
    at BattleActions.hitStepMoveHitLoop (/.../pokemon-showdown/sim/battle-actions.ts:953:44)
    at BattleActions.trySpreadMoveHit (/.../pokemon-showdown/sim/battle-actions.ts:612:79)
    at BattleActions.useMoveInner (/.../pokemon-showdown/sim/battle-actions.ts:529:22)
    at BattleActions.useMove (/.../pokemon-showdown/sim/battle-actions.ts:386:27)
    at BattleActions.runMove (/.../pokemon-showdown/sim/battle-actions.ts:322:33)
    at Battle.runAction (/.../pokemon-showdown/sim/battle.ts:2548:17)

Additional information:
  chunk = >p2 move 1 2,move 2
  inputLog = 
>version 8ac3035eb2f1e9c09b4abf7277f02e6311032cc0
>start {"formatid":"gen9doublesou","seed":[36206,60552,37751,22375],"rated":"Rated battle"}
>player p1 {"name":"local","avatar":"266","team":"Roaring Moon|||Protosynthesis|DragonCheer,DragonDance||1,,,,,|N|,0,,,,|||,,,,,Dragon]Dragonite|||InnerFocus|DragonCheer,DragonDance||1,,,,,|F|,0,,,,|||,,,,,Dragon","rating":1072}
>player p2 {"name":"local2","avatar":"169","team":"Clefable|||CuteCharm|PsychUp,PlayRough||1,,,,,|M||||,,,,,Fairy]Cresselia|||Levitate|Safeguard,Protect||1,,,,,|F|,0,,,,|||,,,,,Psychic]Hydreigon|||Levitate|PsychUp,FocusEnergy||1,,,,,|F|,0,,,,|||,,,,,Dark","rating":1076}
>p1 team 1, 2
>p2 team 3, 2, 1
>p1 move dragoncheer -2, move dragondance
>p2 move focusenergy, move protect
>p1 move dragondance, move dragoncheer -1
>p2 move psychup +2, move protect
  log = 
|t:|1727364917
|gametype|doubles
|player|p1|local|266|1072
|player|p2|local2|169|1076
|teamsize|p1|2
|teamsize|p2|3
|gen|9
|tier|[Gen 9] Doubles OU
|rated|
|rule|Species Clause: Limit one of each Pokémon
|rule|OHKO Clause: OHKO moves are banned
|rule|Evasion Moves Clause: Evasion moves are banned
|rule|Gravity Sleep Clause: The combination of sleep-inducing moves with imperfect accuracy and Gravity or Gigantamax Orbeetle are banned
|rule|Endless Battle Clause: Forcing endless battles is banned
|rule|HP Percentage Mod: HP is shown in percentages
|clearpoke
|poke|p1|Roaring Moon|
|poke|p1|Dragonite, F|
|poke|p2|Clefable, M|
|poke|p2|Cresselia, F|
|poke|p2|Hydreigon, F|
|teampreview
|
|t:|1727364923
|start
|switch|p1a: Roaring Moon|Roaring Moon|351/351
|switch|p1b: Dragonite|Dragonite, F|323/323
|switch|p2a: Hydreigon|Hydreigon, F|325/325
|switch|p2b: Cresselia|Cresselia, F|381/381
|turn|1
|
|t:|1727364933
|move|p2b: Cresselia|Protect|p2b: Cresselia
|-singleturn|p2b: Cresselia|Protect
|move|p1a: Roaring Moon|Dragon Cheer|p1b: Dragonite
|-start|p1b: Dragonite|move: Dragon Cheer
|move|p2a: Hydreigon|Focus Energy|p2a: Hydreigon
|-start|p2a: Hydreigon|move: Focus Energy
|move|p1b: Dragonite|Dragon Dance|p1b: Dragonite
|-boost|p1b: Dragonite|atk|1
|-boost|p1b: Dragonite|spe|1
|
|upkeep
|turn|2
|
|t:|1727364945
|move|p2b: Cresselia|Protect||[still]
|-fail|p2b: Cresselia
|move|p1b: Dragonite|Dragon Cheer|p1a: Roaring Moon
|-start|p1a: Roaring Moon|move: Dragon Cheer
|move|p1a: Roaring Moon|Dragon Dance|p1a: Roaring Moon
|-boost|p1a: Roaring Moon|atk|1
|-boost|p1a: Roaring Moon|spe|1
|move|p2a: Hydreigon|Psych Up|p1b: Dragonite

@DaWoblefet
Copy link
Member

This does match cartridge behavior; a Pokemon with Focus Energy that uses Psych Up into +1 Dragon Cheer will have +1 crit ratio after the move resolves. So it's fine to clear everything first, then add the new volatiles. You will need to also do this in Costar and all the areas where Psych Up, etc. is overriden. This PR should give you a good idea of those places: #10412

I would like to see some tests as well. I can write those for you.

@DaWoblefet DaWoblefet changed the title bugfix psych up copying crit modifiers Fix crash with Psych Up, Focus Energy, and Dragon Cheer Sep 27, 2024
Comment on lines +14549 to 14550
for (const volatile of volatilesToCopy) source.removeVolatile(volatile);
for (const volatile of volatilesToCopy) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (const volatile of volatilesToCopy) source.removeVolatile(volatile);
for (const volatile of volatilesToCopy) {
for (const volatile of volatilesToCopy) {
source.removeVolatile(volatile);

wouldn't this functionally do the same thing with one less iteration?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is this a concern for Costar?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is that you need to remove the Focus Energy volatile in order to be able to properly copy the Dragon Cheer volatile and vice versa. Costar can presumably activate after having volatiles though use of Neutralising Gas or other methods.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I guess the other approach would be to avoid having codependent volatiles altogether by putting all of the logic in the Psych Up volatile which would have to work out whether it had been set by Dragon Cheer on a non-Dragon type in which case it would only add 1 to the crit ratio.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't move the removeVolatile call inside the same loop that tries to add volatiles since that would not remove focusenergy before dragoncheer is added; in this case fusing the loop changes the behavior. We could re-order the volatilesToCopy list, but I'm not sure (or tbh willing to exhaustively test) that would be robust, and it definitely wouldn't be robust against possible future additions or list re-orderings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the cartridge behavior of Costar after neutralizing gas?
e.g. roaring moon has 2 dd's and dragoncheer effect, flamigo switches in and copies all three boosts (+2 atk, +2 spe, +2 crit).
opponent switches weezing-galar (neutralizing gas) in
roaring moon switches out to hydreigon
weezing galar switches out

should flamigo after costar re-ups have no boosts? volatiles only?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the cartridge behavior of Costar after neutralizing gas? e.g. roaring moon has 2 dd's and dragoncheer effect, flamigo switches in and copies all three boosts (+2 atk, +2 spe, +2 crit). opponent switches weezing-galar (neutralizing gas) in roaring moon switches out to hydreigon weezing galar switches out

should flamigo after costar re-ups have no boosts? volatiles only?

It should have attempted to copy Hydreigon's stats, which in this case would be +0 Atk and +0 Spe. It also would copy Hydreigon's lack of crit volatile, so it should have a +0 crit rate again after Costar activates. It's identical to how Psych Up behaves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, that's the current behavior I have tested, then.

@DaWoblefet DaWoblefet merged commit 00f3bdc into smogon:master Oct 1, 2024
1 check passed
@Eric-w-H Eric-w-H deleted the bugfix_focus_energy_dragon_cheer_crash branch October 2, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants