fix: Switching to manage views from popup causing crash#971
fix: Switching to manage views from popup causing crash#971OH296 merged 3 commits intoAdeptus-Dominus:mainfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughSummary by CodeRabbit
WalkthroughBy the Omnissiah’s logic-engine: numeric menu codes are replaced with named constants across UI and controller scripts. One popup path is simplified to a single toggle call. No public interfaces are altered. A minor double-assignment appears in scr_ui_manage but still resolves to MENU.Manage. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
objects/obj_event_log/Draw_0.gml (1)
6-10: Wrong constant namespace and comparator; will misroute logic (and possibly crash).
- Manage.EventLog likely does not exist; elsewhere you use MENU.EventLog.
- '=' assigns instead of compares, subverting menu flow.
- Same for 'if (bad = 0)' below.
Apply this sanctified patch:
- if (obj_controller.menu = Manage.EventLog) { + if (obj_controller.menu == MENU.EventLog) { bad = 0; } } - if (bad = 0) { + if (bad == 0) {objects/obj_popup/Step_0.gml (1)
712-720: Omnissiah’s verdict — update inline comment; scr_toggle_manage runs in obj_controller (no wrapping needed); cooldown missingVerified: scr_toggle_manage() already executes its logic inside with(obj_controller) (scripts/scr_controller_helpers/scr_controller_helpers.gml, ~lines 174–183). It does NOT set a cooldown, so wrapping the call is unnecessary — add a cooldown only if you want parity with other toggles.
Files to change:
- objects/obj_popup/Step_0.gml (lines 712–720) — update the stale comment.
- scripts/scr_controller_helpers/scr_controller_helpers.gml (lines 174–183) — confirms with(obj_controller) usage.
Suggested diffs:
Comment correction (required):
- // Nothing + // Switch to Manage UI (centralised toggle)Optional cooldown (if you want a short controller cooldown after toggling):
- scr_toggle_manage(); + scr_toggle_manage(); + // Optional: prevent rapid re-toggles + with (obj_controller) { + cooldown = 10; + }Omnissiah approves the centralised toggle; please apply the comment fix and decide whether to add the cooldown.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
objects/obj_controller/Mouse_50.gml(2 hunks)objects/obj_event_log/Draw_0.gml(1 hunks)objects/obj_popup/Step_0.gml(1 hunks)scripts/scr_controller_helpers/scr_controller_helpers.gml(1 hunks)scripts/scr_ui_advisors/scr_ui_advisors.gml(1 hunks)scripts/scr_ui_manage/scr_ui_manage.gml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.gml
⚙️ CodeRabbit Configuration File
**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
- Color codes in the code shouldn't have any spaces in their id. I.e., color code
# 80bf40will crash the game.- All code should comply with the main GML documentation: https://manual.gamemaker.io/beta/en/GameMaker_Language/GML_Reference/GML_Reference.htm
Files:
scripts/scr_ui_advisors/scr_ui_advisors.gmlscripts/scr_controller_helpers/scr_controller_helpers.gmlobjects/obj_popup/Step_0.gmlscripts/scr_ui_manage/scr_ui_manage.gmlobjects/obj_event_log/Draw_0.gmlobjects/obj_controller/Mouse_50.gml
**/*.*
⚙️ CodeRabbit Configuration File
**/*.*: - Having humanly understandable and maintainable code is always the top most priority.
- DRY (Don't repeat yourself) principle is also very important.
- Ensure that the code is compliant with the CODE_STYLE.MD: https://github.com/Adeptus-Dominus/ChapterMaster/blob/main/docs/CODE_STYLE.md
- If a TODO comment is added, ask the user if you should create a GitHub issue for this TODO.
- If a TODO comment is deleted, remind the user if there is an active GitHub issue related to that comment.
Files:
scripts/scr_ui_advisors/scr_ui_advisors.gmlscripts/scr_controller_helpers/scr_controller_helpers.gmlobjects/obj_popup/Step_0.gmlscripts/scr_ui_manage/scr_ui_manage.gmlobjects/obj_event_log/Draw_0.gmlobjects/obj_controller/Mouse_50.gml
🧠 Learnings (2)
📚 Learning: 2025-04-18T13:33:54.167Z
Learnt from: EttyKitty
PR: Adeptus-Dominus/ChapterMaster#0
File: :0-0
Timestamp: 2025-04-18T13:33:54.167Z
Learning: In obj_new_button/Step_0.gml, when using scr_click_left() for detecting clicks on menu buttons, it should be called with parameter 0 (scr_click_left(0)) to disable the default cooldown of 60 steps. Using the default cooldown can cause menu buttons to appear unresponsive since it prevents subsequent clicks from being detected.
Applied to files:
objects/obj_popup/Step_0.gmlobjects/obj_controller/Mouse_50.gml
📚 Learning: 2025-04-16T09:51:55.065Z
Learnt from: EttyKitty
PR: Adeptus-Dominus/ChapterMaster#0
File: :0-0
Timestamp: 2025-04-16T09:51:55.065Z
Learning: In obj_ingame_menu/Step_0.gml, the line "if (effect=14){instance_create(0,0,obj_fade);fading=0.1;}" must not be commented out, as it's critical for menu button transitions. When clicking the Exit button, it sets effect=14, but without this handler creating the fade effect object, no transition occurs.
Applied to files:
objects/obj_popup/Step_0.gml
🔇 Additional comments (4)
scripts/scr_controller_helpers/scr_controller_helpers.gml (1)
212-216: Good replacement of magic number with MENU enum.Using MENU.Apothecarion instead of a hard-coded literal is the correct, more readable approach. No functional regressions detected in this hunk.
objects/obj_controller/Mouse_50.gml (2)
230-233: Enum usage upgrade acknowledged.Switching the back button target to MENU.Manage is correct and aligns with the new constant set. Cooldown logic unaffected.
262-266: Enum replacement: sound and cog-toothed.The Chapter Master back arrow now returns to MENU.Manage appropriately. Minor, but consistent with the refactor.
scripts/scr_ui_manage/scr_ui_manage.gml (1)
82-83: LGTM: return-to-Manage after load is correct.Setting menu to MENU.Manage post-load (when not reloading) is consistent with expected UX and the enum migration.
1 participant
Purpose and Description
Testing done
Related things and/or additional context