[Ability] Fully implement Flower Gift and Victory Star

[SirzBenjie]

What are the changes the user will see?

Flower Gift and Victory Star will no longer be marked as partial. Flower gift will now boost the SpDef and Attack of allies during sun / harsh sun. Victory Star will now increase the accuracy of allies.

Why am I making these changes?

Cherrim, what a great Pokemon. It deserves to have its ability fully implemented. So I did! Cherrim lovers will now get to see flower gift buffing allies as it should.
Since this required adding a class to increase stats for allies, it made sense to also fully implement Victini's victory star. So that's why this is in the same PR.

What are the changes from a developer perspective?

The changes to get this working were slightly involved.

In src/data/ability.ts.

Added a new class, AllyStatMultiplierAbAttr and a corresponding applyFieldStatMultiplierAbAttrs function.
The new class extends AbAttr and mirrors the logic of the existing StatMultiplierAbAttr. The class has an ignorable field instead of the condition field, as some ally stat multiplier abilities are ignorable and some are not. (The condition field from StatMultiplierAbAttr is related to accuracy / attack for physical / other moves.

In the flower gift ability attribute list, added two conditional attributes that check if the weather is sun/harsh sun, and add the AllyStatMultiplierAbAttr attribute for the Attack/SpDef stat with a multiplier of 1.5. I also removed .partial() as Flower Gift is fully implemented with this change.

Similarly, for Victory Star, added the AllyStatMultiplierAbAttr attribute for accuracy with a multiplier of 1.1 and removed .partial().

src/data/pokemon.ts

Added two parameters for ignoring the ally / target ally's ability to getAttackDamage and getBaseDamage that default to false.
I updated the callsites for these methods to properly provide the arguments - getAttackDamage passes along the arguments to its call to getBaseDamage; getNextMove() will conditionally ignore the target's ally ability if it has been revealed, and will never ignore its own ally's ability.

src/data/move.ts

In ShellSideArmCategoryAttr, added arguments to the call to getBaseDamage corresponding to the new parameters added to the function.

Tests

For src/test/abilities/flower_gift.test.ts:
Uncommented the lines in the test for raising stats to verify the ally's stats are raised.
Added tests to ensure that damage is properly increased / reduced (or ignored) when attacking / getting hit by a special attack (respecting mold breaker and kin).

Created src/test/abilities/victory_star.test.ts with two tests: one that ensures the accuracy of the user is increased, and another to ensure the accuracy of the user's ally is increased.

Screenshots/Videos

Flower.Gift.Ally.Attack.mp4

Flower.Gift.Ally.SpDef.mp4

How to test the changes?

For the automed tests:
For flower gift : npx vitest run src/test/abilities/flower_gift.test.ts
For victory star: npx vitest run src/test/abilities/victory_star.test.ts

For the manual tests to flower gift, in in src/overrides.ts, add overrides to have the starter mon be Cherrim. Change the moveset to be tackle, sunny day, mud slap, and recover. Add an override to make it a double battle. Change the enemy moveset to be splash, and the species of both mons to be bidoof.

In the first test, have cherrim use splash, and have the ally use tackle on the left bidoof.
Next turn, have cherrim use sunny day, and have the ally use tackle on the other bidoof. Observe that the second bidoof took ~50% more damage now that flower gift has activated.

For the second test, have cherrim use mud slap on the ally, and have the ally use splash. Move your mouse to the location on the ally's healthbar to mark how much damage it took.
Next turn, use sunny day with cherrim and recover with the ally.
Next turn, have cherrim use mud slap on the ally. Observe that ally's remaining health bar is gerater than the position the mouse was at, indicating that the SpDef has increased.

A manual test for victory star is not feasible due to it modifying accuracy.

Checklist

• I'm using beta as my base branch
• 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)
  • Have I created new automated tests (npm run create-test) or updated existing tests related to the PR's changes?
• Have I provided screenshots/videos of the changes (if applicable)?
  • [ ] Have I made sure that any UI change works for both UI themes (default and legacy)?

Are there any localization additions or changes? If so:

• [ ] Has a locales PR been created on the locales repo?
  • [ ] If so, please leave a link to it here:
• [ ] Has the translation team been contacted for proofreading/translation?

Additional Comments

The reason this PR includes implementations for two abilities is that they both rely on the new AllyStatMultiplierAbAttr class being in the codebase. It didn't make sense to me to split this into 2 PRs because of that.

I considered whether a new class needed to be added for AllyStatMultiplerAbAttr. Originally, I had considered just extending StatMultiplerAbAttr. The issue with that approach would be twofold: First, abilities that only boosts the stats of the ally, and not the user itself. Second, the portions of code which look for StatMultiplierAbAttr match against the holder of the ability, (e.g. in applyAbAttrsInternal). This would make the logic a bit clunky for a small case.

The other thing I want to note is the decision to add parameters for ignoring / not ignoring the ability. This required adding two parameters to the getBaseDamage and getAttackDamage functions. I briefly considered hijacking the ignoreAbility / ignoreSourceAbility parameters for this. However, this is a bit misleading and couples ignoring the ability of the target / source with that of the ally. Explicit is better than implicit. While it is true that the only situation which currently uses these arguments is for Shell Side Arm, that implementation chose to split out ignoring ability and ignoring source ability. I followed suit here.

For these methods, there are now four arguments related to ignoring abilities. It may be worth considering a refactor that puts ability ignores into its own class, and use that instead of this pair of 4.

For the getEffectiveStat, I also had to add a parameter here for ignoring the ally ability. Just one though, since the pokemon only cares about its own ally, and not the target.
I did search for invocations of getEffectiveStat to update this, but found no uses of that argument. It's unclear why these arguments exist, but refactoring the code to remove them would be outside the scope of this PR.