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

[Bug] fixes: fixes Fell Stinger giving Atk. boost when enemy dies to another source after being hit #4723

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

Conversation

bjparker1226
Copy link
Contributor

@bjparker1226 bjparker1226 commented Oct 25, 2024

Fixes #1695

What are the changes the user will see?

Previously, if a player used and hit an enemy with Fell Stinger and the target survived, but died to another source later in the same turn, Fell Stinger would grant the user a 3 stage attack boost. Other sources include: recoil moves, damaging weather, status effects, etc. The stat boost will now only be granted if the enemy dies from the move's damage specifically.

Why am I making these changes?

Bug bad.

What are the changes from a developer perspective?

Previously, when a Pokemon fainted, the last attack to hit it was checked in faint-phase.ts. If the last move to hit it was Fell Stinger when it fainted, the user of the move was granted the stat boost. lastDmgSrc has now been added to Pokemon.turnData and is updated to reflect the last source that damaged it. It can hold WeatherType or StatusEffect enum values, or a Pokemon object. lastDmgSrc is now what is checked to determine if the Pokemon was actually KO'd by the move or something else.

Screenshots/Videos

How to test the changes?

Use src/overrides.ts to set up encounters with Endure Pokemon so they can get dropped to 1HP by Fell Stinger and faint in the same turn due to something else (weather, status effect, or recoil).
Automated tests have been introduced for the move for several scenarios:

  • The enemy survives Fell Stinger but is KO'd by recoil on the same turn, no stat boost
  • The enemy survives Fell Stinger but is KO'd by a status effect on the same turn, no stat boost
  • The enemy survives Fell Stinger but is KO'd by damaging weather on the same turn, no stat boost
  • The enemy survives Fell Stinger but is KO'd by the combination of Dry Skin and harsh sunlight on the same turn, no stat boost
  • The enemy drops to 0HP from Fell Stinger but is brought back to 50% HP by Reviver Seed, no stat boost
  • The enemy is KO'd by Fell Stinger damage directly, stat boost granted!

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 considered writing automated tests for the issue?
  • If I have text, did I make it translatable and add a key in the English locale file(s)?
  • 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?

@bjparker1226 bjparker1226 requested a review from a team as a code owner October 25, 2024 09:09
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.

This isn't a complete fix. For example, there are multiple battler tags that deal damage at the end of each turn:

  • Salt Cure
  • Leech Seed
  • Curse (when used by a Ghost-type)
  • Binding effects (e.g. Fire Spin, Sand Tomb)

None of these are set up to change lastDmgSource, so if a Pokemon attacks another Pokemon with Fell Stinger, then damage from any of these tags finishes off the target, the user would still receive the boost.

I'm worried about the scalability of this solution. You basically have to have lastDmgSrc accommodate all forms of indirect damage, and those forms/types can add up.

My suggestion:

  • Add a boolean field to FaintPhase that signals whether or not the faint was caused by an attack.
  • Check that along with the existing condition to determine if PostVictoryAbAttrs should be applied.

FaintPhase also already has a source?: Pokemon field that's only filled when a Pokemon faints from an attack... maybe you can look into using that instead?

@Madmadness65 Madmadness65 added Move Affects a move P2 Bug Minor. Non crashing Incorrect move/ability/interaction labels Oct 26, 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.

Can you update the comment in faint-phase.ts, L36-38 since source isn't exclusively used for Destiny Bond anymore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Move Affects a move P2 Bug Minor. Non crashing Incorrect move/ability/interaction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Fell Stinger activating after opponent kills itself
3 participants