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

Damage roll selection in AI_CalcDamage #4615

Merged
merged 7 commits into from
May 28, 2024

Conversation

Pawkkie
Copy link
Collaborator

@Pawkkie Pawkkie commented May 23, 2024

Description

As is, AI_CalcDamage always assumes the lowest possible damage roll when calculating move damage. This PR adds a parameter to AI_CalcDamage that can specify using an alternate approach based on context, and adds an enum that includes highest, lowest, and average options to start with.

This PR also adds a very simple AI_FLAG_CONSERVATIVE to act as an inverse of AI_FLAG_RISKY that can be used to have trainers always use the lowest damage rolls when scoring their moves.

For all cases where this function is called, I've specified the rolls as follows. Happy to change any of these if we think it's a better idea:

  • HasBadOdds: Assume player gets max roll (favour conservative switching decisions)
  • Smart mon choices: Assume player gets max roll (favour conservative switching decisions)
  • Best mon damage (a default case for vanilla switching): Assume AI gets an average roll
  • Should use Z move: Assume AI gets an average roll
  • Simulated damage: Use average rather than lowest by default for pre-calculating AI damage. Risky AI will instead use highest rolls, and conservative AI will instead use lowest rolls.

I should also note that the HighestRollDmg function currently accepts dmg and as input and immediately returns it as is. This is really just for future proofing, or very easily letting users change how they want bounds of the AI's damage rolls to behave without needing to change any of the other functions.

People who collaborated with me in this PR

Just me writing it, but @iriv24 gave me the idea!

Discord contact info

@Pawkkie

Copy link
Collaborator

@DizzyEggg DizzyEggg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo it's a good idea, and I'm all for this change.

Why is the PR failing though?

include/battle_ai_util.h Outdated Show resolved Hide resolved
src/battle_ai_util.c Outdated Show resolved Hide resolved
@AlexOn1ine
Copy link
Collaborator

Imo it's a good idea, and I'm all for this change.

Why is the PR failing though?

probably failing because a lot of tests assume lowest roll for their damage calcs.

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented May 24, 2024

It's failing on exactly one test, which is one I wrote for the switch AI overhaul last fall, "AI_FLAG_SMART_MON_CHOICES: Mid-battle switches prioritize defensive options". In this test the AI should switch out their lead Ponyta to a defensive Aron that can tank the player's attacks. With this PR, the AI doesn't switch at all, and leaves Ponyta in to get KO'd.

I genuinely do not know why this test is failing. My project has deviated from expansion a bit and I'm pulling new stuff individually so it's hard to directly compare, but this is directly copied from the implementation I'm using which passes everything on my end. I was very surprised to see this fail after being submitted.

I'll try to take some time to investigate further, but I'm very open to suggestions, especially regarding major AI stuff that's changed the last few months that might have introduced whatever AI logic changes have caused this deviation and that I might have missed so aren't in my project to cause the same problem.

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented May 24, 2024

Okay this should be fixed and finished, the test in question relies on an Aron not being 3HKO'd by a Swellow, and it turns out Boomburst's high roll is literally 34% of its HP with zero IVs and EVs. I have inflated poor Aron's SpDefense in the test as though it had 31 IVs in the stat, which allows it to tank the Boomburst and the test behaves as intended.

Figuring out why it doesn't 3HKO in my project will be a separate problem lol

@Pawkkie Pawkkie requested a review from DizzyEggg May 25, 2024 21:59
@Bassoonian
Copy link
Collaborator

Conflicts

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented May 26, 2024

Conflicts

Updated :)

src/battle_ai_util.c Outdated Show resolved Hide resolved
@Pawkkie
Copy link
Collaborator Author

Pawkkie commented May 27, 2024

Spacing fixed :)

@Bassoonian Bassoonian dismissed DizzyEggg’s stale review May 28, 2024 08:21

raised issues resolved

@Bassoonian Bassoonian merged commit be25174 into rh-hideout:upcoming May 28, 2024
1 check passed
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.

4 participants