feat: Main menu refactor; many things to GUI layer; UI controller #333
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request refactors the main menu, moves several components to the GUI layer, and introduces a UI controller. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
bot
added
PR: Major Coding
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @EttyKitty - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -119,19 +120,24 @@ word_from_duke2="blank"; | |||
| mess_alpha=0; | |||
| out_of_date=0; | |||
|
|
|||
There was a problem hiding this comment.
suggestion: Consider using an enum for menu stages
Using an enum for the menu stages would make the code more maintainable and self-documenting than using magic numbers.
Suggested implementation:
enum MenuStage {
INITIAL,
NO_LEGAL,
HAS_LEGAL
}
randomize();
// fade=80;
away=0;
hi=0;
info=0;
mess_alpha=0;
out_of_date=0;
stage = MenuStage.INITIAL;
if (last_legal == 0) then stage = MenuStage.NO_LEGAL;
if (last_legal != 0) {
alarm[2] = 1;
stage = MenuStage.HAS_LEGAL;
tim1 = -50;
tim2 = 424;
}
tim1=0;
You'll need to:
- Update any other code that checks the
stagevariable to use the MenuStage enum values instead of numbers - Make sure any code that sets the stage variable uses the enum values
| @@ -0,0 +1,69 @@ | |||
| if (button_id = 1) { | |||
| wid = 142 * scaling; | |||
There was a problem hiding this comment.
suggestion (performance): Cache scaled dimensions in Create event
The scaled width and height calculations could be moved to the Create event since scaling doesn't change during runtime.
Suggested implementation:
if (button_id = 1) {
wid = button_widths[0];
hei = button_heights[0];
} else if (button_id = 2) {
wid = button_widths[1];
hei = button_heights[1];
} else if (button_id = 3) {
wid = button_widths[2];
hei = button_heights[2];
} else if (button_id = 4) {
wid = button_widths[3];
hei = button_heights[3];
}
You'll need to add this code to the Create event (Create_0.gml):
// Cache scaled button dimensions
button_widths[0] = 142 * scaling;
button_heights[0] = 43 * scaling;
button_widths[1] = 142 * scaling;
button_heights[1] = 43 * scaling;
button_widths[2] = 115 * scaling;
button_heights[2] = 43 * scaling;
button_widths[3] = 108 * scaling;
button_heights[3] = 42 * scaling;| draw_rectangle(x2 + 1317, y2 + 113, x2 + 1461, y2 + 146, 0); | ||
| draw_set_alpha(1); | ||
|
|
||
| if (mouse_left >= 1) and(!instance_exists(obj_popup)) and(cooldown <= 0) { // Load |
There was a problem hiding this comment.
suggestion: Consider extracting repeated button interaction logic into a reusable function
The same pattern of checking mouse_left, obj_popup existence and cooldown appears multiple times. This could be encapsulated in a helper function like check_button_interaction() to reduce duplication and improve maintainability.
Suggested implementation:
// Helper function to check button interaction and handle hover effect
function check_button_interaction(x1, y1, x2, y2) {
var is_hovering = scr_hit(x1, y1, x2, y2);
var can_interact = (mouse_left >= 1) && (!instance_exists(obj_popup)) && (cooldown <= 0);
if (is_hovering) {
draw_set_alpha(0.1);
draw_set_color(c_white);
draw_rectangle(x1, y1, x2, y2, 0);
draw_set_alpha(1);
}
return is_hovering && can_interact;
}
if (check_button_interaction(x2 + 1317, y2 + 113, x2 + 1461, y2 + 146)) {
You'll need to:
- Update all other similar button interaction checks in the file to use the new
check_button_interaction()function - Make sure the
cooldownvariable is accessible where the function is defined - Consider moving this helper function to a separate script file if it could be useful in other parts of the game
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Refactor the main menu and move view-related calculations to the GUI layer using a UI controller.
New Features:
Enhancements: