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

[Ability] Implement Illusion #3273

Open
wants to merge 90 commits into
base: beta
Choose a base branch
from
Open

Conversation

PyGaVS
Copy link
Contributor

@PyGaVS PyGaVS commented Jul 31, 2024

What are the changes?

Adding illusion ability, works with fusion also

Why am I doing these changes?

It's missing

What did change?

-Added PreSummonAbAttr class as ability attribute.
-Illusion uses the appearance of the last conscious and available Pokémon in the party (ignoring itself), including :
Species, Form, Shininess, Pokeball, Gender
-If We encounter a wild Zorua/Zoroak he will use the illusion of a pokemon in the Abyss Biome. Depending on Zorua or Zoroark level the illusion will be evolved or not.
-If we encounter a boss Zoroark he will use the illusion of Entei, Suicune or Raiku like in the original game.
-The class pokemon has a new attribute illusion.
-A pokemon with illusion can use illusion one time per battle like in the original game.
-After a battle, the pokemon can use illusion again.
-The AI move choice will be affected by the illusion.
-The type will always be displayed as the Pokémon it is disguised as, even when affected by Soak etc.
-The illusion will disappear if the Pokemon has its ability changed or suppressed.
-Pokemon cannot transform into a Pokémon that is disguised by Illusion.
-Pokémon that are disguised by Illusion cannot Transform.
-Illusion will not activate if the last conscious Pokémon is a Terastallized Ogerpon or Terapagos.

Screenshots/Videos

https://discord.com/channels/1125469663833370665/1176874654015684739/1257835032647831682
https://discord.com/channels/1125469663833370665/1176874654015684739/1257864381937619045
https://discord.com/channels/1125469663833370665/1176874654015684739/1257865512801538059
https://discord.com/channels/1125469663833370665/1176874654015684739/1258418548074745958

How to test the changes?

I played Zoroark and override the enemy pokemon with Zoroark so we can see him using correct illusion
I used illusion on a fusion and with a fusion
I tried to switch with the illusion
We can also run npm run test illusion

Old conversations here :
#2217

The link for the localization pr :
pagefaultgames/pokerogue-locales#26

Checklist

  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • Are the changes visual?
    • Have I provided screenshots/videos of the changes?

@flx-sta flx-sta added Enhancement New feature or request Ability Affects an ability labels Jul 31, 2024
Copy link
Collaborator

@xsn34kzx xsn34kzx left a comment

Choose a reason for hiding this comment

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

Mostly NITs and minor adjustments. Brought up the idea of making illusion a property of summonData rather than to Pokemon directly, may be something to look into but ultimately not a deal breaker.

src/data/ability.ts Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/field/pokemon.ts Show resolved Hide resolved
src/test/abilities/illusion.test.ts Outdated Show resolved Hide resolved
src/test/abilities/illusion.test.ts Outdated Show resolved Hide resolved
src/test/abilities/illusion.test.ts Outdated Show resolved Hide resolved
src/phases.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/field/pokemon.ts Outdated Show resolved Hide resolved
src/test/abilities/illusion.test.ts Outdated Show resolved Hide resolved
src/test/abilities/illusion.test.ts Outdated Show resolved Hide resolved
src/test/abilities/illusion.test.ts Show resolved Hide resolved
src/test/abilities/illusion.test.ts Outdated Show resolved Hide resolved
src/test/abilities/illusion.test.ts Outdated Show resolved Hide resolved
@torranx
Copy link
Contributor

torranx commented Aug 3, 2024

Please add spaces before and after the video links so it embeds correctly, and add label on what the video shows.

@PyGaVS PyGaVS changed the title [Ability] Implement Ability Illusion [Ability] Implement Illusion Aug 16, 2024
Copy link
Contributor

@torranx torranx left a comment

Choose a reason for hiding this comment

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

I still don't like how it's adding useIllusion- a very specific parameter just for illusion to a lot of functions

src/data/ability.ts Show resolved Hide resolved
src/data/ability.ts Show resolved Hide resolved
src/data/ability.ts Show resolved Hide resolved
@torranx torranx requested review from innerthunder and DayKev August 19, 2024 13:25
Copy link
Contributor

@innerthunder innerthunder left a comment

Choose a reason for hiding this comment

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

Overall I feel like the scope of Illusion-specific data should be a bit more restricted, and Illusion-specific parameters should be more generalized. My main concerns are:

  • It doesn't really make sense to have illusion as a field in Pokemon when it's explicitly an in-battle effect. Usually these kinds of effects are handled within the Pokemon's summonData, battleData, battleSummonData, or turnData depending on the effect's functionality.
  • The useIllusion parameter that's now fed to a lot of methods in Pokemon needs to change to account for future implementations that may want a similar effect (e.g. the enemy AI adjusting for form changes when evaluating type matchups). It would probably be more beneficial to adjust it to the inverse (say, useBaseType) where, if true, all effects that may change the Pokemon's observed type are ignored.

src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/field/pokemon.ts Outdated Show resolved Hide resolved
flx-sta
flx-sta previously approved these changes Nov 8, 2024
Copy link
Contributor

@innerthunder innerthunder left a comment

Choose a reason for hiding this comment

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

The enemy AI currently uses damage calculations to search for KOs, and will likely make more use of damage calculations when evaluating moves in the future. This means that the Illusion class should supply different effective stats (HP, Attack, Defense, etc.) in support of bidirectional damage calculations. It might be easier to use a direct Pokemon reference in illusion and construct a new Pokemon object in cases where the Illusion isn't based on party Pokemon (i.e. Wild battles).

src/field/pokemon.ts Outdated Show resolved Hide resolved
src/field/pokemon.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@torranx torranx left a comment

Choose a reason for hiding this comment

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

this is still using the very specific param for illusion useIllusion. if other devs are ok with it, feel free to dismiss my requested changes

@PyGaVS
Copy link
Contributor Author

PyGaVS commented Nov 9, 2024

this is still using the very specific param for illusion useIllusion. if other devs are ok with it, feel free to dismiss my requested changes

@torranx i didn't found a better alt because we can't rename the params to a less specific name for 2 reasons :

  • this param will always be specific because the illusion mechanic itself is specific
  • if we rename this params other devs in the future will not understand the utility of this param

Maybe we can create a "purpose" param where purpose can either be "forDefend", "battleInfo", etc but only the illusion mechanic will use it today, so idk

@torranx
Copy link
Contributor

torranx commented Nov 10, 2024

@torranx i didn't found a better alt because we can't rename the params to a less specific name for 2 reasons :

  • this param will always be specific because the illusion mechanic itself is specific
  • if we rename this params other devs in the future will not understand the utility of this param

Maybe we can create a "purpose" param where purpose can either be "forDefend", "battleInfo", etc but only the illusion mechanic will use it today, so idk

First of all thank you for being patient with us.
Regarding the param, it doesn't need to be specific, see @innerthunder 's comment

@PyGaVS
Copy link
Contributor Author

PyGaVS commented Nov 28, 2024

@torranx i didn't found a better alt because we can't rename the params to a less specific name for 2 reasons :

  • this param will always be specific because the illusion mechanic itself is specific
  • if we rename this params other devs in the future will not understand the utility of this param

Maybe we can create a "purpose" param where purpose can either be "forDefend", "battleInfo", etc but only the illusion mechanic will use it today, so idk

First of all thank you for being patient with us. Regarding the param, it doesn't need to be specific, see @innerthunder 's comment

I will do it later when i have time
In my opinion it's not necessary to rename these parameters as it will confuse other devs + it can be updated any time with another PR whenever we need to.

@PyGaVS
Copy link
Contributor Author

PyGaVS commented Jan 22, 2025

The enemy AI currently uses damage calculations to search for KOs, and will likely make more use of damage calculations when evaluating moves in the future. This means that the Illusion class should supply different effective stats (HP, Attack, Defense, etc.) in support of bidirectional damage calculations. It might be easier to use a direct Pokemon reference in illusion and construct a new Pokemon object in cases where the Illusion isn't based on party Pokemon (i.e. Wild battles).

So i used a direct Pokemon reference for base Pokemon in the interface Illusion this is now way better, but not in illusion since wild Zorua will not create an illusion based on an existing pokemon. That's not a problem.
But for the specific params illusion i tried to remove it by adding other func for illusion but when i do that i'm forced to add if statement with illusion everywhere in the ui files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ability Affects an ability Enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

9 participants