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

AI_FLAG_SETUP_FIRST_TURN Rename and Clarifications #5310

Merged

Conversation

Pawkkie
Copy link
Collaborator

@Pawkkie Pawkkie commented Sep 1, 2024

Description

Addresses the concerns outlined in #5303 explicitly

Issue(s) that this PR fixes

Fixes #5303

People who collaborated with me in this PR

Alex, psf

Discord contact info

Pawkkie

@Pawkkie Pawkkie added type: documentation Improvements or additions to documentation category: battle-ai Pertains to Battle Engine Upgrade's AI type: refactor labels Sep 1, 2024
@AlexOn1ine AlexOn1ine self-assigned this Sep 1, 2024
Comment on lines 28 to 34
#define AI_FLAG_CHECK_BAD_MOVE (1 << 0)
#define AI_FLAG_TRY_TO_FAINT (1 << 1)
#define AI_FLAG_CHECK_VIABILITY (1 << 2)
#define AI_FLAG_SETUP_FIRST_TURN (1 << 3)
#define AI_FLAG_FORCE_SETUP_FIRST_TURN (1 << 3) // AI will prioritize using setup moves on the first turn at the expensve of all else. AI_FLAG_CHECK_VIABILITY will instead do this when the AI determines it makes sense.
#define AI_FLAG_RISKY (1 << 4)
#define AI_FLAG_PREFER_STRONGEST_MOVE (1 << 5)
#define AI_FLAG_PREFER_BATON_PASS (1 << 6)
Copy link
Collaborator

@AlexOn1ine AlexOn1ine Sep 2, 2024

Choose a reason for hiding this comment

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

would it potentially be useful to document every other flag that has no documentation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great idea, I gave these all a pass with comments, feel free to suggest alterations to any of them.

This is also probably our best opportunity to change the names of Omniscient and HP Aware if we'd like to, we certainly still can later but those two have been causing some confusion. Not sure if it's necessary right now though if we don't have good ideas for what to use instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's confusing about them, and what would be a better name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't personally find either confusing, but we've discussed before that HP Aware is really just an arbitrary list of score increase / decreases based on HP rather than making the AI "smart" or "aware", and Omniscient can make it sound like the AI knows what actions the player is taking.

I'm not saying we need to change either, just that this would be a good time to consider whether we'd like to. The primary reason I think we don't is that I at least don't have any better naming suggestions in spite of the confusing, I think the comments describe what they do really well and at some point we have to rely on users reading the details if they care lol

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the comments are good on those for now. We can always think about it in the future

@AlexOn1ine AlexOn1ine merged commit 720bc64 into rh-hideout:upcoming Sep 4, 2024
1 check passed
@Pawkkie Pawkkie deleted the ai-setup-first-turn-clarifications branch September 9, 2024 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: battle-ai Pertains to Battle Engine Upgrade's AI type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants