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

#3207 TSpikes Pecha Fix #3275

Closed
wants to merge 2 commits into from
Closed

Conversation

Pawkkie
Copy link
Collaborator

@Pawkkie Pawkkie commented Sep 3, 2023

Fixes #3207, involving status cure berries blocking an instance of hazard damage and hazard status being applied.

Changes the ItemBattleEffects function to use the same pattern as ItemEffectMoveEnd when handling HOLD_EFFECT_CURE_XYZ and berries. This cleanly fixes this issue, but I'm not super familiar with this section of code to be confident that this is an appropriate solution. Feel free to dismiss outright if not.

I updated all of the status cure berries with this change rather than just Pecha and Lum in case any downstream users might have added a TSpikes that burns etc.

Video

2nd Poochyena has a Pecha Berry, 4th Poochyena has a Lum Berry. Both behave as expected.

TSpikes.mp4

Issue(s) that this PR fixes

Fixes #3207

Discord contact info

Pawkkie

@AsparagusEduardo
Copy link
Collaborator

Note for the reviewer:
I would recommend adding tests that check that the battlers can be statused again once the corresponding status-healing items trigger. This way, we can prevent these specific regressions.

@DizzyEggg
Copy link
Collaborator

As Asparagus said, this fix should have the tests included.

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented Sep 5, 2023

Unfortunately writing these tests is beyond the scope of the time I'm willing to commit to this PR, I've not made them before and while I'm sure the syntax isn't that bad to learn I'm personally not willing to spend the time doing so and then also spend the time figuring out and testing that specific implementation.

Just fixed this in my own project and figured I'd share it. Feel free to close this PR if that's a required feature for bug fix acceptance, I'll keep a branch available on my GitHub we can direct people to if anyone wants it in the meantime :)

@AlexOn1ine
Copy link
Collaborator

This test fails with this PR.

SINGLE_BATTLE_TEST("Hold effect cures status if a pokemon enters a battle")
{
    u16 status;
    PARAMETRIZE{ status = STATUS1_BURN; }
    PARAMETRIZE{ status = STATUS1_FREEZE; }
    PARAMETRIZE{ status = STATUS1_PARALYSIS; }
    PARAMETRIZE{ status = STATUS1_POISON; }
    PARAMETRIZE{ status = STATUS1_TOXIC_POISON; }
    PARAMETRIZE{ status = STATUS1_SLEEP; }

    GIVEN {
        PLAYER(SPECIES_WOBBUFFET);
        OPPONENT(SPECIES_WOBBUFFET) { Status1(status); Item(ITEM_LUM_BERRY); }
    } WHEN {
        TURN { }
    } SCENE {
        ANIMATION(ANIM_TYPE_GENERAL, B_ANIM_HELD_ITEM_EFFECT, opponent);
    }
}

The berries are supposed to trigger when a pokemon with a status condition enters the battle.

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented Sep 7, 2023

This PR can probably be closed unless anyone has any other ideas. Alex and I did some chatting this morning, and while this fixes the interaction between status cure berries and avoiding hazard damage, it introduces an issue where if you have a pre-status'd mon with a status cure berry as the first mon that enters during a battle, its status won't be cured immediately, which it should be.

Neither of us are sure where to go from here, this bug is weird, but he'll PR his tests separately :)

@AsparagusEduardo
Copy link
Collaborator

Replaced by #3316

@Pawkkie Pawkkie deleted the tspikes-pecha-fix branch October 1, 2023 17:13
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.

Toxic Spikes don't function properly after a Pecha Berry activation.
4 participants