From e8b9672fc77316d8b4ee19256a055024bb554e6c Mon Sep 17 00:00:00 2001 From: Bertie690 Date: Mon, 19 May 2025 20:50:18 -0400 Subject: [PATCH] Applied some review suggestions --- src/data/abilities/ability.ts | 27 +++++++++------- src/data/battler-tags.ts | 55 ++++++++++++++++----------------- src/data/moves/invalid-moves.ts | 11 +++++++ src/data/moves/move.ts | 35 ++++++++++++--------- src/field/arena.ts | 3 ++ src/phases/move-effect-phase.ts | 4 +-- src/phases/move-phase.ts | 12 ++++--- 7 files changed, 87 insertions(+), 60 deletions(-) diff --git a/src/data/abilities/ability.ts b/src/data/abilities/ability.ts index 5424ad956a4..efc20d07590 100644 --- a/src/data/abilities/ability.ts +++ b/src/data/abilities/ability.ts @@ -1,8 +1,8 @@ -import { HitResult, MoveResult, PlayerPokemon, type TurnMove } from "#app/field/pokemon"; +import { HitResult, MoveResult, PlayerPokemon } from "#app/field/pokemon"; import { BooleanHolder, NumberHolder, toDmgValue, isNullOrUndefined, randSeedItem, randSeedInt, type Constructor } from "#app/utils/common"; import { getPokemonNameWithAffix } from "#app/messages"; import { BattlerTagLapseType, GroundedTag } from "#app/data/battler-tags"; -import { getNonVolatileStatusEffects, getStatusEffectDescriptor, getStatusEffectHealText, type Status } from "#app/data/status-effect"; +import { getNonVolatileStatusEffects, getStatusEffectDescriptor, getStatusEffectHealText } from "#app/data/status-effect"; import { Gender } from "#app/data/gender"; import { AttackMove, @@ -65,10 +65,8 @@ import { CommonAnim } from "../battle-anims"; import { getBerryEffectFunc } from "../berry"; import { BerryUsedEvent } from "#app/events/battle-scene"; - // Type imports -import { EnemyPokemon, PokemonMove } from "#app/field/pokemon"; -import Pokemon from "#app/field/pokemon"; +import Pokemon, { EnemyPokemon, PokemonMove } from "#app/field/pokemon"; import type { Weather } from "#app/data/weather"; import type { BattlerTag } from "#app/data/battler-tags"; import type { AbAttrCondition, PokemonDefendCondition, PokemonStatStageChangeCondition, PokemonAttackCondition, AbAttrApplyFunc, AbAttrSuccessFunc } from "#app/@types/ability-types"; @@ -4447,11 +4445,10 @@ export class PostDancingMoveAbAttr extends PostMoveUsedAbAttr { move: PokemonMove, source: Pokemon, targets: BattlerIndex[], - simulated: boolean, + simulated: boolean, args: any[]): void { if (!simulated) { // If the move is an AttackMove or a StatusMove the Dancer must replicate the move on the source of the Dance - // TODO: fix in main dancer PR (currently keeping this purely semantic rather than actually fixing bug) if (move.getMove() instanceof AttackMove || move.getMove() instanceof StatusMove) { const target = this.getTarget(dancer, source, targets); globalScene.unshiftPhase(new MovePhase(dancer, target, move, MoveUseType.INDIRECT)); @@ -5547,8 +5544,8 @@ class ForceSwitchOutHelper { * * @param pokemon The {@linkcode Pokemon} attempting to switch out. * @returns `true` if the switch is successful - * TODO: Make this actually cancel pending move phases on the switched out target - */ + */ + // TODO: Make this cancel pending move phases on the switched out target public switchOutLogic(pokemon: Pokemon): boolean { const switchOutTarget = pokemon; /** @@ -7177,7 +7174,13 @@ export function initAbilities() { .attr(PostFaintHPDamageAbAttr) .bypassFaint(), new Ability(Abilities.DANCER, 7) - .attr(PostDancingMoveAbAttr), + .attr(PostDancingMoveAbAttr) + .edgeCase(), + /* Incorrect interations with: + * Petal Dance (should not lock in or count down timer; currently does both) + * Flinches (due to tag being removed earlier) + * Failed/protected moves (should not trigger if original move is protected against) + */ new Ability(Abilities.BATTERY, 7) .attr(AllyMoveCategoryPowerBoostAbAttr, [ MoveCategory.SPECIAL ], 1.3), new Ability(Abilities.FLUFFY, 7) @@ -7320,7 +7323,9 @@ export function initAbilities() { .bypassFaint() .edgeCase(), // interacts incorrectly with rock head. It's meant to switch abilities before recoil would apply so that a pokemon with rock head would lose rock head first and still take the recoil new Ability(Abilities.GORILLA_TACTICS, 8) - .attr(GorillaTacticsAbAttr), + .attr(GorillaTacticsAbAttr) + .edgeCase(), + // May or may not incorrectly increase struggle's power by 50% (needs confirmation) new Ability(Abilities.NEUTRALIZING_GAS, 8) .attr(PostSummonAddArenaTagAbAttr, true, ArenaTagType.NEUTRALIZING_GAS, 0) .attr(PreLeaveFieldRemoveSuppressAbilitiesSourceAbAttr) diff --git a/src/data/battler-tags.ts b/src/data/battler-tags.ts index a3714422474..45f04f88d87 100644 --- a/src/data/battler-tags.ts +++ b/src/data/battler-tags.ts @@ -45,6 +45,7 @@ import { StatusEffect } from "#enums/status-effect"; import { WeatherType } from "#enums/weather-type"; import { isNullOrUndefined } from "#app/utils/common"; import { MoveUseType } from "#enums/move-use-type"; +import { invalidEncoreMoves } from "./moves/invalid-moves"; export enum BattlerTagLapseType { FAINT, @@ -379,12 +380,11 @@ export class GorillaTacticsTag extends MoveRestrictionBattlerTag { /** * Ensures that move history exists on {@linkcode Pokemon} and has a valid move to lock into. - * @param pokemon - the {@linkcode Pokemon} to add the tag to + * @param pokemon - The {@linkcode Pokemon} to add the tag to * @returns `true` if the tag can be added */ override canAdd(pokemon: Pokemon): boolean { - // Choice items ignore struggle - // TODO: Check if struggle also gets the 50% power boost + // Choice items ignore struggle, so Gorilla Tactics should too const lastSelectedMove = pokemon.getLastNonVirtualMove(); return ( !isNullOrUndefined(lastSelectedMove) && @@ -398,8 +398,13 @@ export class GorillaTacticsTag extends MoveRestrictionBattlerTag { * @param pokemon - The {@linkcode Pokemon} to add the tag to */ override onAdd(pokemon: Pokemon): void { + const lastMove = pokemon.getLastNonVirtualMove(); + if (!lastMove) { + return; + } + super.onAdd(pokemon); - this.moveId = pokemon.getLastNonVirtualMove()!.move; // `canAdd` returns false if no move + this.moveId = lastMove.move; pokemon.setStat(Stat.ATK, pokemon.getStat(Stat.ATK, false) * 1.5, false); } @@ -1122,7 +1127,6 @@ export class FrenzyTag extends BattlerTag { * Applies the effects of {@linkcode Moves.ENCORE} onto the target Pokemon. * Encore forces the target Pokemon to use its most-recent move for 3 turns. */ -// TODO: Refactor and fix the bugs involving struggle and lock ons export class EncoreTag extends MoveRestrictionBattlerTag { public moveId: Moves; @@ -1147,17 +1151,7 @@ export class EncoreTag extends MoveRestrictionBattlerTag { return false; } - const unEncoreableMoves = new Set([ - Moves.MIMIC, - Moves.MIRROR_MOVE, - Moves.TRANSFORM, - Moves.STRUGGLE, - Moves.SKETCH, - Moves.SLEEP_TALK, - Moves.ENCORE, - ]); - - if (unEncoreableMoves.has(lastMove.move)) { + if (invalidEncoreMoves.has(lastMove.move)) { return false; } @@ -1167,6 +1161,7 @@ export class EncoreTag extends MoveRestrictionBattlerTag { } onAdd(pokemon: Pokemon): void { + // TODO: shouldn't this be `onAdd`? super.onRemove(pokemon); globalScene.queueMessage( @@ -1179,7 +1174,6 @@ export class EncoreTag extends MoveRestrictionBattlerTag { if (movePhase) { const movesetMove = pokemon.getMoveset().find(m => m.moveId === this.moveId); if (movesetMove) { - // TODO: Check encore + calling move interactions and change to `pokemon.getLastNonVirtualMove()` if needed const lastMove = pokemon.getLastXMoves(1)[0]; globalScene.tryReplacePhase( m => m instanceof MovePhase && m.pokemon === pokemon, @@ -1908,20 +1902,23 @@ export class TruantTag extends AbilityBattlerTag { const lastMove = pokemon.getLastXMoves()[0]; - if (lastMove && lastMove.move !== Moves.NONE) { - // ignore if just slacked off OR first turn of battle - const passive = pokemon.getAbility().id !== Abilities.TRUANT; - (globalScene.getCurrentPhase() as MovePhase).cancel(); - // TODO: Ability displays should be handled by the ability - globalScene.queueAbilityDisplay(pokemon, passive, true); - globalScene.queueMessage( - i18next.t("battlerTags:truantLapse", { - pokemonNameWithAffix: getPokemonNameWithAffix(pokemon), - }), - ); - globalScene.queueAbilityDisplay(pokemon, passive, false); + if (!lastMove) { + // Don't interrupt move if last move was `Moves.NONE` OR no prior move was found + return true; } + // Interrupt move usage in favor of slacking off + const passive = pokemon.getAbility().id !== Abilities.TRUANT; + (globalScene.getCurrentPhase() as MovePhase).cancel(); + // TODO: Ability displays should be handled by the ability + globalScene.queueAbilityDisplay(pokemon, passive, true); + globalScene.queueMessage( + i18next.t("battlerTags:truantLapse", { + pokemonNameWithAffix: getPokemonNameWithAffix(pokemon), + }), + ); + globalScene.queueAbilityDisplay(pokemon, passive, false); + return true; } } diff --git a/src/data/moves/invalid-moves.ts b/src/data/moves/invalid-moves.ts index bee7b3e8ab0..7d1d30ab3de 100644 --- a/src/data/moves/invalid-moves.ts +++ b/src/data/moves/invalid-moves.ts @@ -269,3 +269,14 @@ export const invalidSketchMoves: ReadonlySet = new Set([ Moves.BREAKNECK_BLITZ__PHYSICAL, Moves.BREAKNECK_BLITZ__SPECIAL, ]); + +/** Set of all moves that cannot be locked into by {@linkcode Moves.ENCORE}. */ +export const invalidEncoreMoves = new Set([ + Moves.MIMIC, + Moves.MIRROR_MOVE, + Moves.TRANSFORM, + Moves.STRUGGLE, + Moves.SKETCH, + Moves.SLEEP_TALK, + Moves.ENCORE, +]); diff --git a/src/data/moves/move.ts b/src/data/moves/move.ts index 58db1450b91..fdaf32a74cc 100644 --- a/src/data/moves/move.ts +++ b/src/data/moves/move.ts @@ -5421,12 +5421,14 @@ export class FrenzyAttr extends MoveEffectAttr { // If frenzy is not active, add a tag and push 1-2 extra turns of attacks to the user's move queue. // Otherwise, tick down the existing tag. if (!user.getTag(BattlerTagType.FRENZY) && user.getMoveQueue().length === 0) { - const turnCount = user.randBattleSeedIntRange(1, 2); - new Array(turnCount).fill(null).map(() => user.getMoveQueue().push({ move: move.id, targets: [ target.getBattlerIndex() ], useType: MoveUseType.IGNORE_PP })); + const turnCount = user.randBattleSeedIntRange(1, 2); // excludes initial use + for (let i = 0; i < turnCount; i++) { + user.pushMoveQueue({ move: move.id, targets: [ target.getBattlerIndex() ], useType: MoveUseType.IGNORE_PP }); + } user.addTag(BattlerTagType.FRENZY, turnCount, move.id, user.id); } else { applyMoveAttrs(AddBattlerTagAttr, user, target, move, args); - user.lapseTag(BattlerTagType.FRENZY); // if FRENZY is already in effect (moveQueue.length > 0), lapse the tag + user.lapseTag(BattlerTagType.FRENZY); } return true; @@ -7001,14 +7003,14 @@ export class CopyMoveAttr extends CallMoveAttr { apply(user: Pokemon, target: Pokemon, _move: Move, args: any[]): boolean { this.hasTarget = this.mirrorMove; - // TODO: Confirm whether Mirror Move and co. can copy struggle + // bang is correct as condition func returns `false` and fails move if no last move exists const lastMove = this.mirrorMove ? target.getLastNonVirtualMove(false, false)!.move : globalScene.currentBattle.lastMove; return super.apply(user, target, allMoves[lastMove], args); } getCondition(): MoveConditionFunc { return (_user, target, _move) => { - const lastMove = this.mirrorMove ? target.getLastNonVirtualMove(false, true)?.move : globalScene.currentBattle.lastMove; + const lastMove = this.mirrorMove ? target.getLastNonVirtualMove(false, false)?.move : globalScene.currentBattle.lastMove; return !isNullOrUndefined(lastMove) && !this.invalidMoves.has(lastMove); }; } @@ -7140,12 +7142,13 @@ export class RepeatMoveAttr extends MoveEffectAttr { Moves.TRANSFORM, Moves.MIMIC, Moves.STRUGGLE, - // TODO: Add Max/G-Move blockage if or when they are implemented + // TODO: Add Max/G-Max/Z-Move blockage if or when they are implemented ]; if (!lastMove?.move // no move to instruct || !movesetMove // called move not in target's moveset (forgetting the move, etc.) || movesetMove.ppUsed === movesetMove.getMovePp() // move out of pp + // TODO: This next line is likely redundant as all charging moves are in the above list || allMoves[lastMove.move].isChargingMove() // called move is a charging/recharging move || uninstructableMoves.includes(lastMove.move)) { // called move is in the banlist return false; @@ -8607,8 +8610,7 @@ export function initMoves() { new SelfStatusMove(Moves.METRONOME, PokemonType.NORMAL, -1, 10, -1, 0, 1) .attr(RandomMoveAttr, invalidMetronomeMoves), new StatusMove(Moves.MIRROR_MOVE, PokemonType.FLYING, -1, 20, -1, 0, 1) - .attr(CopyMoveAttr, true, invalidMirrorMoveMoves) - .edgeCase(), // May or may not have incorrect interactions with Struggle + .attr(CopyMoveAttr, true, invalidMirrorMoveMoves), new AttackMove(Moves.SELF_DESTRUCT, PokemonType.NORMAL, MoveCategory.PHYSICAL, 200, 100, 5, -1, 0, 1) .attr(SacrificialAttr) .makesContact(false) @@ -8965,7 +8967,10 @@ export function initMoves() { .attr(AddBattlerTagAttr, BattlerTagType.ENCORE, false, true) .ignoresSubstitute() .condition((user, target, move) => new EncoreTag(user.id).canAdd(target)) - .reflectable(), + .reflectable() + .edgeCase(), + // Can lock infinitely into struggle; has incorrect interactions with Blood Moon/Gigaton Hammer + // Also may or may not incorrectly select targets for replacement move (needs verification) new AttackMove(Moves.PURSUIT, PokemonType.DARK, MoveCategory.PHYSICAL, 40, 100, 20, -1, 0, 2) .partial(), // No effect implemented new AttackMove(Moves.RAPID_SPIN, PokemonType.NORMAL, MoveCategory.PHYSICAL, 50, 100, 40, 100, 0, 2) @@ -9477,8 +9482,7 @@ export function initMoves() { .target(MoveTarget.NEAR_ENEMY) .unimplemented(), new SelfStatusMove(Moves.COPYCAT, PokemonType.NORMAL, -1, 20, -1, 0, 4) - .attr(CopyMoveAttr, false, invalidCopycatMoves) - .edgeCase(), // May or may not have incorrect interactions with Struggle + .attr(CopyMoveAttr, false, invalidCopycatMoves), new StatusMove(Moves.POWER_SWAP, PokemonType.PSYCHIC, -1, 10, 100, 0, 4) .attr(SwapStatStagesAttr, [ Stat.ATK, Stat.SPATK ]) .ignoresSubstitute(), @@ -10426,9 +10430,12 @@ export function initMoves() { .ignoresSubstitute() .attr(RepeatMoveAttr) .edgeCase(), - // incorrect interactions with Gigaton Hammer, Blood Moon & Torment due to them making moves _fail on use_, - // not merely unselectable. - // Also my or may not have incorrect interactions with Struggle (needs verification). + /* + * Incorrect interactions with Gigaton Hammer, Blood Moon & Torment due to them + presently making moves _fail on use_, not merely unselectable. + * May or may not have incorrect interactions with Struggle (needs verification). + * May or may not fail upon copying a copied move also in one's own moveset + */ new AttackMove(Moves.BEAK_BLAST, PokemonType.FLYING, MoveCategory.PHYSICAL, 100, 100, 15, -1, -3, 7) .attr(BeakBlastHeaderAttr) .ballBombMove() diff --git a/src/field/arena.ts b/src/field/arena.ts index f083180490b..ce4b4b0d270 100644 --- a/src/field/arena.ts +++ b/src/field/arena.ts @@ -749,6 +749,9 @@ export class Arena { ); } + // TODO: Add an overload similar to `Array.prototype.find` if the predicate func is of the form + // `(x): x is T` + /** * Uses {@linkcode findTagsOnSide} to filter (using the parameter function) for specific tags that apply to both sides * @param tagPredicate a function mapping {@linkcode ArenaTag}s to `boolean`s diff --git a/src/phases/move-effect-phase.ts b/src/phases/move-effect-phase.ts index 57518a0dfde..2ccb9e739d3 100644 --- a/src/phases/move-effect-phase.ts +++ b/src/phases/move-effect-phase.ts @@ -48,9 +48,9 @@ import { MoveTarget } from "#enums/MoveTarget"; import { MoveCategory } from "#enums/MoveCategory"; import { SpeciesFormChangePostMoveTrigger } from "#app/data/pokemon-forms"; import { PokemonType } from "#enums/pokemon-type"; -import { type DamageResult, PokemonMove, type TurnMove } from "#app/field/pokemon"; +import { PokemonMove, HitResult, MoveResult } from "#app/field/pokemon"; +import type { DamageResult, TurnMove } from "#app/field/pokemon"; import type Pokemon from "#app/field/pokemon"; -import { HitResult, MoveResult } from "#app/field/pokemon"; import { getPokemonNameWithAffix } from "#app/messages"; import { ContactHeldItemTransferChanceModifier, diff --git a/src/phases/move-phase.ts b/src/phases/move-phase.ts index fd156d22241..010f68e5b15 100644 --- a/src/phases/move-phase.ts +++ b/src/phases/move-phase.ts @@ -68,6 +68,7 @@ export class MovePhase extends BattlePhase { return this._pokemon; } + // TODO: Do we need public getters but only protected setters? protected set pokemon(pokemon: Pokemon) { this._pokemon = pokemon; } @@ -332,12 +333,15 @@ export class MovePhase extends BattlePhase { const isDelayedAttack = move.hasAttr(DelayedAttackAttr); if (isDelayedAttack) { // Check the player side arena if another delayed attack is active and hitting the same slot. - const delayedAttackTags = globalScene.arena.findTags(t => - [ArenaTagType.FUTURE_SIGHT, ArenaTagType.DOOM_DESIRE].includes(t.tagType), - ) as DelayedAttackTag[]; + // TODO: Make this use a `getTags` proxy once one is added to support custom predicates const currentTargetIndex = targets[0].getBattlerIndex(); + const delayedAttackTags = globalScene.arena.findTags( + (tag): tag is DelayedAttackTag => + (tag.tagType === ArenaTagType.FUTURE_SIGHT || tag.tagType === ArenaTagType.DOOM_DESIRE) && + (tag as DelayedAttackTag).targetIndex === currentTargetIndex, + ) as DelayedAttackTag[]; - if (delayedAttackTags.some(tag => tag.targetIndex === currentTargetIndex)) { + if (delayedAttackTags.length) { this.showMoveText(); this.failMove(); return;