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

Implement fainted forme regression #10810

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

Conversation

andrebastosdias
Copy link
Contributor

@andrebastosdias andrebastosdias commented Jan 9, 2025

@KrisXV I believe I have implemented your PR #9135 Closes #10025

I hard-coded regressions for Mega Evolutions, and Terastallized Ogerpon and Terapagos. I am not aware of any other Pokémon with this issue.

The first commit is just a simplification of the update of details, while the second commit contains the logic for the regression. It's easier to review as separate commits.

IMPORTANT: The client should not display the transform animation after the Pokémon faints. I added the [silent] flag, but I don’t think it’s necessary, as the animation occurs after the Pokémon leaves the screen (the client should be able to check this). Aside from that, the only other change is the Pokémon icon updating, which already happens.

@andrebastosdias
Copy link
Contributor Author

I removed the tests from the Revival Blessing file and kept only Terapagos's tests so that we can also verify if Terapagos-Terastal does not revert. The other tests are a bit redundant.

sim/battle.ts Outdated Show resolved Hide resolved
Co-authored-by: pyuk-bot <[email protected]>
@pyuk-bot
Copy link
Contributor

pyuk-bot commented Jan 9, 2025

I think Morpeko also needs special treatment to ensure it reverts to the correct forme after fainting while Terastallized.

@@ -1959,7 +1959,7 @@ export class BattleActions {
}
if (pokemon.species.baseSpecies === 'Morpeko') {
pokemon.baseSpecies = pokemon.species;
pokemon.details = pokemon.details.replace('Morpeko', pokemon.species.name);
pokemon.details = pokemon.getSimpleDetails();
Copy link
Member

@KrisXV KrisXV Jan 9, 2025

Choose a reason for hiding this comment

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

Suggested change
pokemon.details = pokemon.getSimpleDetails();
pokemon.details = pokemon.getSimpleDetails(pokemon.species.name);

Copy link
Contributor Author

@andrebastosdias andrebastosdias Jan 10, 2025

Choose a reason for hiding this comment

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

By default, pokemon.species.name is used. Currently, that argument is only used for Greninja-Bond.

@DaWoblefet
Copy link
Member

DaWoblefet commented Jan 10, 2025

Is this handling things like Ultra Necrozma becoming Dusk Mane/Dawn Wings, Zygarde-Complete becoming 50%/10%, Ash Greninja becoming regular Greninja etc? This looks like it's just addressing Megas and Ogerpon/Terapagos (see end of this post: https://www.smogon.com/forums/threads/pokemon-sun-moon-battle-mechanics-research.3586701/post-7729529). If they do work, it'd be good to have an extra test or two for forms that do regress and forms that don't.

It'd probably be good to have a test for the Hackmons behavior of e.g. Mega Charizard X -> Mega Evolves to Mega Charizard Y and faints -> regresses back to a fainted Mega Charizard X.

@andrebastosdias
Copy link
Contributor Author

andrebastosdias commented Jan 10, 2025

The logic handles every single one of those cases; we just need to add them to the condition. It should work with Mega-Charizard-X in Hackmons since it reverts to set.species.

I can add all those cases to the condition, or we could add a variable to Pokémon like revertIfFainted, which would serve as an argument for Pokemon.formeChange. Alternatively, I could set isPermanent to boolean | 'revertIfFainted' or something similar.

Edit: I could also change isPermanent?: boolean to notPermanent: boolean | null = true so:

  • true is not permanent
  • false is permanent
  • null is permanent until fainted

@DaWoblefet
Copy link
Member

I like the idea of modifying isPermanent to your approach of notPermanent.

@urkerab
Copy link
Contributor

urkerab commented Jan 10, 2025

Are Shaymin-Sky and Palafin the only cases where set.species would be incorrect? If so, would it help to update set.species in those two places, so that we could always revert to set.species rather than having an unwieldy hardcoded list?

@andrebastosdias
Copy link
Contributor Author

Shaymin-Sky, Palafin-Hero and Terapagos-Terastal (and maybe some OMs). We don't need a hardcoded list, we just need a boolean revertOnFaint that is set inside formeChange. I'm just finishing another PR and I'll take care of this.

@urkerab
Copy link
Contributor

urkerab commented Jan 10, 2025

I don't think you should have to change every call to formeChange just for three Pokémon...

@andrebastosdias
Copy link
Contributor Author

If you want to change set.species, we would also need to change it back when Terapagos-Terastal terastalizes.

@urkerab
Copy link
Contributor

urkerab commented Jan 10, 2025

So Terapagos-Terastal doesn't revert, but Terapagos-Stellar reverts to Terapagos? That's... weird.

@andrebastosdias
Copy link
Contributor Author

@andrebastosdias
Copy link
Contributor Author

I think it's clean. I wasn't feeling changing the set.

@urkerab
Copy link
Contributor

urkerab commented Jan 10, 2025

Thanks for the link; I like this version. The only think I'm curious about is why setting the regression forme to null doesn't work in the other places.

@andrebastosdias
Copy link
Contributor Author

Sorry, what do you mean by it doesn't work in the other places?

@@ -1949,6 +1949,7 @@ export class BattleActions {
}
if (pokemon.species.name === 'Terapagos-Terastal' && type === 'Stellar') {
pokemon.formeChange('Terapagos-Stellar', null, true);
pokemon.regressionForme = {species: this.dex.species.get('Terapagos'), ability: toID('Tera Shift')};
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just wondering whether this can be null or failing that {species: pokemon.baseSpecies, ability: pokemon.baseAbility}.

Copy link
Contributor Author

@andrebastosdias andrebastosdias Jan 10, 2025

Choose a reason for hiding this comment

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

Because when Terapagos activates Tera Shift, it sets baseSpecies to Terapagos-Terastal and regressionForme = null. Terapagos-Stellar is the only case where we need to revert to a form that was not the current baseSpecies.

For Morpeko, I just added a condition to avoid adding unnecessary logs if it was in its base form when it terastalized.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I got the Terapagos formes confused there.

neither updates pokemon.details
pokemon.clearVolatile may be called in OMs and perform some shenanigans. Set the base variables before the call so they are transposed to the main variables, and change the details afterwards.
Neither Zacian-C, Zamazenta-C, nor Tera Morpeko announce their transformations. They are all handled on the client side. It doesn't happen in front of the players.
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.

Ogerpon-Tera and Terapagos-Stellar should revert after Revival Blessing
5 participants