[Move] Use BattlerTag for move-disabling effects

[zacharied]

What are the changes?

This PR implements a new BattlerTag for move-disabling effects such as those done by Disable, Taunt, Throat Chop, etc.. It also replaces the implementation of the move Disable and the ability Cursed Body with a new implementation using this tag.

Why am I doing these changes?

The current approach

Disabling is a somewhat common effect of moves, but the current implementation of disabling has a few issues:

• Only one move can be disabled per Pokemon at a time, and very few disable effects actually work this way in-game
• No way to introspect on source of the disable
• Introduces more state management to the Pokemon object
• We have to modify phases.ts to implement new disabling effects

#751 is an example of a PR that attempts to solve this problem by extending the current approach, using Throat Chop as a baseline. It carries with it the same issues as the current implementation of Disable, namely in that it adds more state for the Pokemon object to deal with. Incorrect behavior can be observed in #751 if a pokemon were to learn a sound-based move after getting hit by Throat Chop; the sound-based move would execute successfully because the disabledMoves array does not reflect the actual disabled moves according to the rules. This would be incorrect, as all sound-based moves should be disabled regardless of if they were originally in the moveset.

Related work-in-progress

Other PRs implement new Tags, which feels safer to me. #1121 implements Imprison correctly, but my concern is over tacking more conditions onto PokemonMove.isUsable. Every condition added to that function also means more code spread elsewhere to account for displaying the move-specific messages and other such effects. I am concerned that by the time we have all the moves implemented, the function will become incredibly long while simultaneously requiring its callers to take on the work of presenting the correct messages. With that said, a lot of work has gone into verifying the correctness of Imprison in #1121, so I don't want to overstep any boundaries of what seems to be the more mature PR.

Likewise, there is #1153, which is an overall agreeable solution, but I'm unsure if a PR with that many features will get merged anytime soon. An intermediate PR that lays the groundwork for those moves, like this PR, might be necessary to get that through.

To avoid invalidating the work of these existing commits, my idea was to introduce some system-level changes that will make move-disabling effects more consistent and easier to implement. I haven't checked the exact merges myself, but I think things like #1121 and #1153 could coexist with and benefit from these changes they were to adapt this tag system. But since those commits are further along the pipeline, I'd also be happy to wait for them to go mainline and then rebase my changes onto those implementations. I'm open to all discussion!

This PR

In this PR, to implement new disable types, you only need to extend DisablingBattlerTag and implement its isMoveDisabled to match the filter applied by the move. Then, any Pokemon that has that tag will handle disabling the matching moves -- no need to edit PokemonMove.isUsable or MovePhase.canMove or anything like that.

What did change?

Add abstract class DisablingBattlerTag that serves as the base for all tags that disable moves. The tag lapses in PRE_MOVE to cancel any disable move getting used. It also lapses at TURN_END to remove the tag. Also updated menus and AI command selection to prevent players and enemies from selecting disabled moves.

Add a battler tag DisabledTag that represents Disabling as applied by the move Disable and ability Cursed Body. Also remove DisableMoveAttr (the old implementation of the Disable move) and replace Disable's effect with adding a DisabledTag.

Add battler tag type DISABLED which maps to DisabledTag. This tag has been given the same targetBenefit as was previously bestowed by DisableMoveAttr.

disabledMove and disabledTurns fields removed from PokemonSummonData. Any time those fields would be set, instead an instance of tag DisabledTag is applied. Because this uses the tag system, all the code that had to manually manipulate/check disabledTurns has been removed as well.

Add new functions Pokemon.getDisablingTag(Moves) and Pokemon.isMoveDisabled(Moves) that find the tag that disables a given move (if it exists), and checks whether the Pokemon has a tag that disables the given move, respectively.

Screenshots/Videos

How to test the changes?

I used the following overrides for testing beyond the unit tests.

/**
 * OVERALL OVERRIDES
 */

// a specific seed (default: a random string of 24 characters)
export const SEED_OVERRIDE: string = "";
export const WEATHER_OVERRIDE: WeatherType = WeatherType.NONE;
export const DOUBLE_BATTLE_OVERRIDE: boolean = false;
export const SINGLE_BATTLE_OVERRIDE: boolean = true;
export const STARTING_WAVE_OVERRIDE: integer = 1;
export const STARTING_BIOME_OVERRIDE: Biome = Biome.TOWN;
export const ARENA_TINT_OVERRIDE: TimeOfDay = null;
// Multiplies XP gained by this value including 0. Set to null to ignore the override
export const XP_MULTIPLIER_OVERRIDE: number = null;
export const IMMEDIATE_HATCH_EGGS_OVERRIDE: boolean = false;
// default 1000
export const STARTING_MONEY_OVERRIDE: integer = 0;
export const POKEBALL_OVERRIDE: { active: boolean, pokeballs: PokeballCounts } = {
  active: false,
  pokeballs: {
    [PokeballType.POKEBALL]: 5,
    [PokeballType.GREAT_BALL]: 0,
    [PokeballType.ULTRA_BALL]: 0,
    [PokeballType.ROGUE_BALL]: 0,
    [PokeballType.MASTER_BALL]: 0,
  }
};

/**
 * PLAYER OVERRIDES
 */

export const STARTING_LEVEL_OVERRIDE: integer = 10;

export const STARTER_SPECIES_OVERRIDE: Species | integer = Species.BIDOOF;
export const ABILITY_OVERRIDE: Abilities = Abilities.NONE;
export const PASSIVE_ABILITY_OVERRIDE: Abilities = Abilities.NONE;
export const STATUS_OVERRIDE: StatusEffect = StatusEffect.NONE;
export const GENDER_OVERRIDE: Gender = null;
export const MOVESET_OVERRIDE: Array<Moves> = [Moves.DISABLE, Moves.SPLASH, Moves.GROWL, Moves.TAIL_WHIP];

export const OPP_SPECIES_OVERRIDE: Species | integer = Species.BIDOOF;
export const OPP_LEVEL_OVERRIDE: number = 1;
export const OPP_ABILITY_OVERRIDE: Abilities = Abilities.NONE;
export const OPP_PASSIVE_ABILITY_OVERRIDE = Abilities.NONE;
export const OPP_STATUS_OVERRIDE: StatusEffect = StatusEffect.NONE;
export const OPP_GENDER_OVERRIDE: Gender = null;
export const OPP_MOVESET_OVERRIDE: Array<Moves> = [Moves.DISABLE, Moves.SPLASH, Moves.NONE, Moves.NONE];
export const OPP_SHINY_OVERRIDE: boolean = false;
export const OPP_VARIANT_OVERRIDE: Variant = 0;

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?

[zacharied]

I've added some moves as a proof of concept to another branch. For example, here's Throat Chop and Taunt: 94cfb53

[zacharied]

Rewrote OP to focus more on integrating the variety of disparate Disable effect PRs.

[zacharied]

Converted to draft due to significant overlap with other PRs.

[DayKev]

Imo it'd be best if this PR was merged and then the other PRs were updated to use the new, non-spaghetti code.

[audriddax]

I'm just going through this, but as the developer of #1121, I have no objection to cleaning this up. I kinda touched on it in some of my back and forth in that one, I wasn't thrilled with what it required to implement. Not only did I talk about how I was hesitant to update code on something that basically affected every move, but on my end I also was seeing exactly what you are trying to address based on your summary here.

I'm all for a change that creates a better framework for my code, I'll gladly update it rather than add extra code to an "unrelated" section just for my move. I'm actually glad I had to step away for a few weeks and haven't had my code committed if it means we're getting a better framework!

[audriddax]

Imo it'd be best if this PR was merged and then the other PRs were updated to use the new, non-spaghetti code.

I agree with this, don't try to update my move, I'll do that gladly!

[zacharied]

WTF did i do to make those tests fail 😭

[DayKev]

One of the overrides changed how it worked (instead of SINGLE_BATTLE_OVERRIDE/DOUBLE_BATTLE_OVERRIDE there's just BATTLE_TYPE_OVERRIDE: "double" | "single" | null).
Also there are helper functions to change overrides now (game.override.x()) that you can make use of.

[frutescens]

Hi, thank you for working on this. I was wondering if this was able to be extended to multiple categories / attrs of moves. I'm currently working on Heal Block right now and was hoping to try disabling moves based on attributes.

[torranx]

Are you still working on this @zacharied ?

[torranx]

couple nits, some docs are missing @param and @returns too, otherwise looks good to me

[torranx]

lgtm