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

Move RestoreBattlerData out of Ai_CalcDmg and add SaveBattlerData Protections #4156

Merged
merged 6 commits into from
May 15, 2024

Conversation

ghoulslash
Copy link
Collaborator

@ghoulslash ghoulslash commented Feb 8, 2024

Having this restore function implicit to AI_CalcDamage seems like poor coding practice. Also, AI_CalcDamage and AI_CalcDamageSaveBattlers were called within ai routines without saving or restoring the battlers afterwards, respectively, which could be problematic

Also adds a "saved" field to struct AI_SavedBattleMon to prevent calling SaveBattlerData twice on the same battler until the data is restored to prevent overwriting values

@ghoulslash ghoulslash changed the title Move RestoreBattlerData out of Ai_CalcDmg Move RestoreBattlerData out of Ai_CalcDmg and add SaveBattlerData Protections Feb 9, 2024
@AlexOn1ine AlexOn1ine requested a review from DizzyEggg February 10, 2024 08:10
include/battle.h Show resolved Hide resolved
src/battle_ai_util.c Show resolved Hide resolved
@AlexOn1ine AlexOn1ine merged commit 9c6dfb7 into rh-hideout:upcoming May 15, 2024
1 check passed
@ghoulslash
Copy link
Collaborator Author

Just realized SetBatrlerData checks if (!BattlerHasAi(battlerId) && !AI_THINKING_STRUCT->saved[battlerId].saved) which is not set in AiCalcPartyDamage.

I will add a follow-up PR for this tomorrow if nobody else gets to it first

AlexOn1ine added a commit that referenced this pull request Jun 28, 2024
Since #4156 SetBattlerData and RestoreBattlerData were moved out of `AI_CalcDamage`. I assume this was a bad merge somewhere
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.

2 participants