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 interaction between Tera Blast and Unaware #10030

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vingkan
Copy link
Contributor

@vingkan vingkan commented Dec 27, 2023

Summary

Fixes issue #9381.

  • Conditions:
    • The attacking Pokemon uses Tera Blast.
    • The attacking Pokemon is terastallized.
    • The attacking Pokemon has boosted their attack stat to be higher than their special attack stat.
    • The defending Pokemon has the ability Unaware.
  • Expected Behavior: Tera Blast should be a physical attack.
  • Actual Behavior: Tera Blast is a special attack.

Also applied similar logic fixes to:

  • Tera Blast
  • Light That Burns the Sky
  • Photon Geyser
  • Shell Side Arm

Solution

Adds a new parameter to Pokemon.getStat() called ignoreBoostModifiers which, if true, skips runEvent('ModifyBoost') to avoid effects that modify stat stage boosts at damage calculation time.

By adding a new parameter, callers can opt-in to this behavior and we only opt-in for the four moves in the standard format that need this fix.

Open Questions

  1. For Shell Side Arm, should all stats be checked without considering boost modifiers? Is Shell Side Arm meant to be "unaware of Unaware?" Should some stats be checked with Unaware included in the calculation, like the target's defense and special defense?
  2. Should the conditions created by Foresight and Miracle Eye apply their boost modifiers when checking the stats of these four moves or are they only necessary at damage calculation time? These are the only standard format moves that apply the ModifyBoost handlers.
  3. Should we avoid breaking the behavior of a Pokemon with Simple using one of these moves? Or is it not necessary to maintain strict backwards compatibility for the other formats? If we want to maintain the behavior how might we support that? Should we create a new event type?

Future Work

These tasks could be saved for future pull requests:

  • Apply this fix to the moves and rules in the non-standard formats.
  • Add a unit test to verify that Unaware was used in the damage calculation even though it did not affect the move category. I was not sure how to test this meaningfully since I could not find a clear output property. We could test the damage level in cases where that value is stable.

Appendix

This new parameter skips event handlers that derive from the ModifyBoost event. As far as I can tell, that means it potentially affects interactions with these moves and abilities. Standard format entities are bolded.

  • Unaware (Ability)
  • Foresight (Move)
  • Miracle Eye (Move)
  • Foresight (Move, Gen 2)
  • Simple (Ability, Gen 4)
  • Unaware (Ability, Full Potential)
  • Carefree (Ability, Super Staff Bros)
  • Old Manpa (Ability, Super Staff Bros)
  • Ghost Spores (Ability, Super Staff Bros)
  • Plausible Deniability (Ability, Super Staff Bros)
  • Hacked Corrosion (Ability, Super Staff Bros)
  • True Grit (Ability, Super Staff Bros)
  • Unaware (Ability, Super Staff Bros)
  • Billo/Unaware (Condition, Super Staff Bros)

As far as I can tell, these are the moves and abilities that check a stat with boosts applied but not stat modifiers. These would be candidates for using the new parameter. Standard format entities are bolded.

  • Light That Burns the Sky (Move)
  • Photon Geyser (Move)
  • Shell Side Arm (Move)
  • Tera Blast (Move)
  • Strength Sap (Move) - Only checks the target's attack stat, which would not be altered by Unaware.
  • Belly Drum (Move, Gen 2)
  • Swagger (Move, Gen 2)
  • Belly Drum (Move, Gen 2 Stadium 2)
  • Tera Blast (Move, [Gen 9] Tera Donation Format)
  • Light That Burns the Sky (Move, Category Swap Mod Ruleset)
  • Photon Geyser (Move, Category Swap Mod Ruleset)
  • Shell Side Arm (Move, Category Swap Mod Ruleset)
  • Tera Blast (Move, VaporeMons)
  • BURN IT DOWN! (Ability, Super Staff Bros)
  • Bane Terrain (Move, Super Staff Bros)
  • Soul Siphon (Move, Super Staff Bros)
  • Integer Overflow (Move, Super Staff Bros)
  • Homunculus's Vanity (Move, Super Staff Bros)

@pyuk-bot
Copy link
Contributor

There are 3 other moves that change their category based on the user’s stats using getStat() like this: Photon Geyser, Light That Burns the Sky, and Shell Side Arm. All of them need this Unaware interaction fixed, and with that in mind, this is a pretty messy solution (that doesn’t even work if the Unaware user is holding an Ability Shield).

Change getStat(). I considered adding a parameter to ignore the defender's abilities in the function that gets stats. However, that would require passing that flag into the runEvent() system, so I dug deeper.

You could just change getStat() to not run the 'ModifyBoost' event at all when calculating unmodified stats. The only other vanilla effect that uses that event is Gen 4 Simple which is mutually exclusive with all moves and effects that use getStat() in that way outside of custom banlist scenarios that are understood to potentially have unspecified behaviors. I guess it would make a small problem for Gen 4 Full Potential if anyone ever tried to play that specific combination of formats, but even then it would basically never come up. I think that’s an acceptable loss to fix all of these bugs at once with one line of code.

@vingkan
Copy link
Contributor Author

vingkan commented Dec 27, 2023

You could just change getStat() to not run the 'ModifyBoost' event at all when calculating unmodified stats.

That makes sense, I'll look into that.

I posted this question on the issue, but why are there separate functions for calculateStat() and getStat()? Is it okay for them to continue to diverge?

data/moves.ts Outdated Show resolved Hide resolved
@KrisXV KrisXV changed the title Fix interaction between Tera Blast and Unaware. Fix interaction between Tera Blast and Unaware Dec 27, 2023
@vingkan vingkan force-pushed the vk_terablast_ignore_ability_get_stat branch from 55515d5 to 081ffba Compare December 27, 2023 20:11
@vingkan
Copy link
Contributor Author

vingkan commented Dec 27, 2023

Apologies, I rebased poorly and the branch got cluttered with other changes. GitHub automatically requested reviews from codeowners of those other files and now I can't remove the review request.

@vingkan
Copy link
Contributor Author

vingkan commented Dec 28, 2023

Hi @pyuk-bot and @KrisXV, I implemented the approach that Pyuk suggested, could you review the new version and let me know what you think?

I called out three open questions in the description:

  1. For Shell Side Arm, should all stats be checked without considering boost modifiers? Is Shell Side Arm meant to be "unaware of Unaware?" Should some stats be checked with Unaware included in the calculation, like the target's defense and special defense?
  2. Should the conditions created by Foresight and Miracle Eye apply their boost modifiers when checking the stats of these four moves or are they only necessary at damage calculation time? These are the only standard format moves that apply the ModifyBoost handlers.
  3. Should we avoid breaking the behavior of a Pokemon with Simple using one of these moves? Or is it not necessary to maintain strict backwards compatibility for the other formats? If we want to maintain the behavior how might we support that? Should we create a new event type?

@DaWoblefet DaWoblefet self-requested a review January 3, 2024 16:51
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.

3 participants