pagefaultgames / pokerogue

A browser based Pokémon fangame heavily inspired by the roguelite genre.
https://pokerogue.net
GNU Affero General Public License v3.0
4.49k stars 1.8k forks source link

[Bug] No Indication for Sleep Move Failure on Electric Terrain #3722

Open Snailman11 opened 2 months ago

Snailman11 commented 2 months ago

Describe the bug

"xxxxx was protected by electric terrain"

https://replay.pokemonshowdown.com/gen9customgame-2186597941

https://discord.com/channels/1125469663833370665/1125894949020381285/1276752620421120093 -Video where it occurs

-this could also apply to Bulldoze/Earthquake and Grassy Terrain, and Priority with Psychic

Session export file

No response

User data export file

No response

Expected behavior

There should be a message to indicate that Electric terrain has nullified the move

Screenshots / Videos

https://github.com/user-attachments/assets/f1a7685e-071a-4e64-9dd7-e7f329bdac11

What OS did you observe the bug on?

Other

If other please specify

gameplay

Which browser do you use?

Other

If other please specify

gameplay

Additional context

No response

jessemchung commented 2 months ago

Hello, I am looking for an easy first issue to work on. Could I work on this?

Snailman11 commented 2 months ago

Hello, I am looking for an easy first issue to work on. Could I work on this?

Sure

jessemchung commented 2 months ago

@Snailman11 Wait, are you a maintainer? I read online I should ask them before working

@innerthunder Are you a maintainer? Can I just start working on it?

innerthunder commented 2 months ago

Go for it! I know there are quite a few cases of move-cancelling effects missing "activation" messages, so it'd be great to have people work towards sorting those messages out.

If you have any other questions about the developer workflow or how to submit pull requests, feel free to ask in the Discord's dev channel.

jessemchung commented 2 months ago

@innerthunder Gotcha, yeah, so I am looking into the fix and I suspect adding a new trigger file might be helpful (similar to the ability-trigger.ts file) but can also add a new entry to the battle.ts file for specific terrains. Not sure so will ask in the dev channel.

Will also need to make adjustments to this case if (!typeMultiplier.value) { this.scene.queueMessage(i18next.t("battle:hitResultNoEffect", { pokemonName: getPokemonNameWithAffix(this) })); }

I also probably need to make changes to export class DrowsyTag extends BattlerTag {

Mainly adding a new queue Message

Also need to watch here

    case StatusEffect.SLEEP:
      if (this.isGrounded() && this.scene.arena.terrain?.terrainType === TerrainType.ELECTRIC) {
        // add statement here.  But not sure
        return false;
      }

Will need to try to figure out the override feature

jessemchung commented 2 months ago

Nevermind, I think I got it working with much less work, I also added overrides values for terrain type I think. Will confirm tomorrow

jessemchung commented 2 months ago

Might need a review as this is a pretty simple fix that I used but might want some infrastructure changes as this creates a couple redundancies. Namely, the issue is that canSetStatus does all the checking but if I put the message there, it is out of order with the move message being used after the move failure message. I had to put similar logic that was in that function in the trySetStatus function which might be needed but is a bit messy.

Anyways, this is what it looks like https://github.com/user-attachments/assets/66aa865b-1daa-4c5b-8e99-a415ea9fb3d8

edit: here is the pull request https://github.com/pagefaultgames/pokerogue/pull/3775