-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Removes duplicate code in AI functions #5457
Conversation
Ready for review. |
src/battle_util.c
Outdated
switch(gLastUsedAbility) | ||
{ | ||
case ABILITY_MOTOR_DRIVE: | ||
statId = STAT_SPEED; | ||
break; | ||
case ABILITY_LIGHTNING_ROD: | ||
case ABILITY_STORM_DRAIN: | ||
statId = STAT_SPATK; | ||
break; | ||
case ABILITY_SAP_SIPPER: | ||
case ABILITY_WIND_RIDER: | ||
statId = STAT_ATK; | ||
break; | ||
case ABILITY_WELL_BAKED_BODY: | ||
statAmount = 2; | ||
statId = STAT_DEF; | ||
break; | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is a bit scuffed but it was worse when I checked them in the function directly. It is cleaner isolated here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enums look chef's kiss though, I'll try to make time to review tomorrow if no one beats me to it :)
I found a potential bug in singles (present before the pr) but when I tried to fix it some tests started failing. Didn't want to go to deep into it so I left a comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read through this 3 times across a couple of days cuz it's kind of a chunky refactor and I'm trying to be careful, but this looks great to me. Happy to wait for another reviewer's approval as well if you like but I think this looks good to go :)
Let's wait with merging until #5489 is merged |
Ready for re-review. I think it is fine if this one is merged first, Someone will have to take care of conflicts anyways. |
CanAbilityBlockMove
,CanPartnerAbilityBlockMove
andCanAbilityAbsorbMove
to remove redundancy inAI_CheckBadMove
andIsDamageMoveUnusable
Not that much code was removed but the important part was to unify both so we don't need to add those abilities to AI functions. We actually didn't have a check for Good As Gold, Wind Rider and we didn't check the opposing partner ability.
Closes #5454