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

Add Random to the counterpick CSS in ranked #6

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

walz0
Copy link

@walz0 walz0 commented Dec 24, 2022

Adds a CSIcon with the Question material to the CharPickerDialog. Uses HSD_Randi to select a random character and proceeds with the normal character confirm logic.

image

Copy link
Collaborator

@rapito rapito left a comment

Choose a reason for hiding this comment

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

I would also change the places where CKIND_GANONDORF is being used and replace with a new constant CKIND_LAST_INDEX or similar so you dont have to replace all the places on the code, just change the value of the constant.

As it is right now it makes it hard to scale as too many changes are needed in different places.

Components/CharPickerDialog.c Outdated Show resolved Hide resolved
@@ -30,6 +34,9 @@ static void _InputsThink(GOBJ *gobj) {

if (downInputs & HSD_BUTTON_A) {
SFX_PlayCommon(CommonSound_ACCEPT); // Play "accept" sound
if (cpd->state.char_selection_idx == 26) { // Select random
Copy link
Collaborator

Choose a reason for hiding this comment

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

@walz0 same as the previous comment but this would be a different named constant, something like CKIND_RANDOM

Copy link
Author

Choose a reason for hiding this comment

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

should be much more scalable / readable now, i added constants for random and last index and refactored every instance where i used CKIND_GANONDORF +1 or 26

@walz0 walz0 requested a review from rapito December 24, 2022 21:32
Copy link
Collaborator

@rapito rapito left a comment

Choose a reason for hiding this comment

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

LGTM

@rapito rapito requested a review from JLaferri May 31, 2023 12:25
@walz0
Copy link
Author

walz0 commented Jan 6, 2024

@JLaferri bumping this for possible merge since its been approved for a while

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.

2 participants