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

ShouldPivot type cleanup #5441

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

Pawkkie
Copy link
Collaborator

@Pawkkie Pawkkie commented Sep 29, 2024

Description

As discussed on Discord starting here, ShouldPivot being a bool32 that returns one of three values from an enum is unintuitive (or at least was to me lol). This PR follows MGriffin's cleanup suggestion, and also moves the enum definition such that it can be referenced from across various AI functions in the future.

People who collaborated with me in this PR

MGriffin and Hedara for explaining how this works and offering a solution

Discord contact info

@Pawkkie

@Bassoonian
Copy link
Collaborator

I would personally ensure the enums have a clear prefix to indicate what they belong to, as PIVOT on its own is somewhat vague and potentially confusing later down the line.

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented Oct 1, 2024

I would personally ensure the enums have a clear prefix to indicate what they belong to, as PIVOT on its own is somewhat vague and potentially confusing later down the line.

Dyou mean rename from AIPivot to AIShouldPivot, or rename the PIVOT enum to SHOULD_PIVOT, or both?

@Bassoonian
Copy link
Collaborator

The latter! We actually don't have a lot of precedent for what eg AIPivot should be, but the actual enum entries should follow the define conventions and have a clear context in their name

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented Oct 1, 2024

The latter! We actually don't have a lot of precedent for what eg AIPivot should be, but the actual enum entries should follow the define conventions and have a clear context in their name

Will push after breakfast! :)

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented Oct 1, 2024

Fixed and conflicts resolved!

@Bassoonian Bassoonian merged commit 2afc7f6 into rh-hideout:upcoming Oct 10, 2024
1 check passed
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.

2 participants