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

Minor switch AI refactor #4849

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

Pawkkie
Copy link
Collaborator

@Pawkkie Pawkkie commented Jun 20, 2024

Description

Alex left a comment on #4794 that sounded really sweet, and after noodling with it a bit with he and Egg we've unfortunately determined it's just not a good solution in this specific context in spite of how cool an idea it is, because the requirement for two different array index orders for the different switching behaviours mean we awkwardly need multiple enums and the nonzero array initialization to PARTY_SIZE makes it awkward to replace the big series of ID defines at the top of the GetBestMonIntegrated function. Unfortunate.

That aside, one of the primary goals was to try to make the conditionals section a bit more readable, and Egg had this sweet suggestion for structuring them in the way I've done here. Also taken this opportunity to fix a typo of "per say" to "per se" which is definitely not either my or Egg's fault, adjust a few comments that referenced functions that no longer exist in the conditional block, and swap over the file's remaining gItemsInfo uses to the new ItemID_ ones.

This is a very minor refactor, but given that there are 3 tiny changes that would be nice to have I figured it was worth submitting now before we all forgot about it between now and whenever we add another feature in this section of the codebase.

People who collaborated with me on this PR

Egg, Alex and Duke

Discord contact info

@Pawkkie

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented Jun 20, 2024

I am pretty confident the build failing is the CI's fault, this is a really minor set of changes that compiles and passes tests fine on my end

@DizzyEggg
Copy link
Collaborator

Why are you marked as a first-time contributor here? Wtf

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.

This looks good, though are we okay with this PR making its way to upcoming and not master? It's fine with me anyway.

@AlexOn1ine
Copy link
Collaborator

This looks good, though are we okay with this PR making its way to upcoming and not master? It's fine with me anyway.
Technically it does not fix any bugs so I think upcoming is fine

@Pawkkie
Copy link
Collaborator Author

Pawkkie commented Jun 21, 2024

Why are you marked as a first-time contributor here? Wtf

Uh I have no idea I didn't do anything usual when submitting lol

This looks good, though are we okay with this PR making its way to upcoming and not master? It's fine with me anyway.
Technically it does not fix any bugs so I think upcoming is fine

This is why I put it to upcoming, I'm happy to retarget it if we think that makes more sense though :)

@DizzyEggg DizzyEggg merged commit 2640bcd into rh-hideout:upcoming Jun 22, 2024
1 check passed
@Pawkkie Pawkkie deleted the minor-switchAI-refactor branch October 1, 2024 15:34
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.

3 participants