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

Remove cmd_recordability #5472

Open
wants to merge 1 commit into
base: upcoming
Choose a base branch
from

Conversation

ghoulslash
Copy link
Collaborator

Removes cmd_recordability and calls directly in CreateAbilityPopup. This also catches abilityPopupOverwrite abilities

…lityPopup to catch popupOverwite abilities
@PhallenTree
Copy link

PhallenTree commented Oct 5, 2024

All current uses of gBattleScripting.abilityPopupOverwrite don't require the overwriting ability to be recorded. abilityPopupOverwrite is used in the following cases:

  1. Ability is changed (Doodle, Role Play, Wandering Spirit, Tera Shift).
    In this case gBattleScripting.abilityPopupOverwrite is the ability before it gets changed to the target's ability (the first two), the target's old ability and the attacker's old ability (Wandering Spirit) or the user's old ability before changing forms (Tera Shift -> Tera Shell).

  2. Ability is changed to match the vanilla games' behavior of displaying each of the "As One" abilities (Unnerve and Chilling Neigh / Grim Neigh).

In this PR's current implementation, the ability recorded would either be the old ability (case 1) or part of the ability (case 2), which could cause AI issues (although in case 1 it's immediately overwritten to the correct ability again since it displays the user's new ability immediately after). *It doesn't currently cause AI issues because there isn't code checking for those abilities specifically from Recorded Abilities but it would still be ideal to record correct abilities.

@AlexOn1ine
Copy link
Collaborator

All current uses of gBattleScripting.abilityPopupOverwrite don't require the overwriting ability to be recorded.

Why is that?

@PhallenTree
Copy link

All current uses of gBattleScripting.abilityPopupOverwrite don't require the overwriting ability to be recorded.

Why is that?

Upon reviewing more AI code, I've changed my opinion, should be fine to do so to keep AI_PARTY's ability tracker updated.

@PhallenTree
Copy link

Suggested change:
In battle_ai_util.c, RecordAbilityBattle:
AI_PARTY->mons[side][partyIndex].ability should only be changed if AI_PARTY->mons[side][partyIndex].ability == ABILITY_NONE

Fixes the following scenario:
A mon has Protean, uses Skill Swap, and obtains Drought, AI_PARTY->mons[side][partyIndex]->ability for that mon becomes Drought instead of keeping it as Protean. Upon switching out and switching back in again, the AI thinks this mon has Drought instead of Protean.

Along with the changes made in this PR, most of these cases should be fixed.

@AlexOn1ine
Copy link
Collaborator

@ghoulslash could you address the comment from Phallen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants