Skip to content

refactor: Huge refactor of main menu and settings#1134

Merged
EttyKitty merged 3 commits intomainfrom
refactor/main-menu
Mar 3, 2026
Merged

refactor: Huge refactor of main menu and settings#1134
EttyKitty merged 3 commits intomainfrom
refactor/main-menu

Conversation

@EttyKitty
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added Size: Epic Type: Refactor Rewriting/restructuring code, while keeping general behavior labels Mar 2, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 991c1ae and f5aeb96.

📒 Files selected for processing (6)
  • objects/obj_ingame_menu/Draw_0.gml
  • objects/obj_ingame_menu/Step_0.gml
  • objects/obj_main_menu_buttons/Create_0.gml
  • objects/obj_persistent/Step_0.gml
  • scripts/SettingsManager/SettingsManager.gml
  • scripts/scr_controller_helpers/scr_controller_helpers.gml

📝 Walkthrough

Under the Hood

  • New SettingsManager script/module (scripts/SettingsManager) centralises settings: load/save/apply_video/sync_ui and exposes global.settings (master, music, sfx, fullscreen, window_rect, autosave).
  • Replaced scattered INI reads/writes and per-object setting variables with global.settings; many ini_open/ini_read/ini_close sites removed across main menu, creation, controller, saveload and others.
  • All audio volume/lookups switched to global.settings.* (master, music, sfx) across objects and scripts.
  • Settings persistence and window/fullscreen tracking moved to obj_persistent (Create_0/Step_0) and SettingsManager (instead of ad-hoc per-object INI handling).
  • Added Boot room (rooms/Boot/Boot.yy) containing obj_persistent as an initial/boot room.
  • Major main-menu refactor:
    • Simplified obj_main_menu to a fade/title_alpha-driven flow (removed multi-stage timers, particle systems, many alarms and legacy timed sequences).
    • Reworked menu UI to data-driven buttons array and loop-based draw/step logic (obj_main_menu_buttons, obj_new_button); centralised hover/fade/transition handling.
    • Consolidated in-game menu effects into new enum eIN_GAME_MENU_EFFECT and reworked effect handling (save/load/options/exit/return flows).
    • Introduced utility functions: approach(), start_room_transition(), and draw lifecycle calls add_draw_return_values()/pop_draw_return_values() in several Draw events.
  • Removed obj_lol_version resource and migrated/adjusted version/build display logic into main menu draw code.
  • Numerous small cleanups: deletion of legacy window-geometry persistence sites, commented/unused code, particle init, and alarm lines across many objects.

Player Notes

  • Settings are now centrally persistent: changes to volume, fullscreen/window, autosave and window position are saved and applied via the new SettingsManager.
  • Volume controls in the settings UI are visual progress bars for master, SFX and music, and use the new settings backend.
  • Video changes (fullscreen/window, window size) are applied and synced automatically; UI surface is kept at 1600×900 when applied.
  • Main menu startup and transitions use a simpler fade/title animation; menu buttons and navigation feel more consistent and responsive.
  • Version/build text remains copyable from the main menu corner despite obj_lol_version removal.

Walkthrough

Tech‑Priest: Centralised settings into a new SettingsManager and migrated dispersed INI/window/audio code to global.settings; added a Boot room and persistent startup flow; refactored main menu and button UI to data‑driven draw/step logic; removed obj_lol_version and cleaned legacy saveload/INI remnants.

Changes

Cohort / File(s) Summary
Settings manager & boot
scripts/SettingsManager/SettingsManager.gml, scripts/SettingsManager/SettingsManager.yy, rooms/Boot/Boot.yy, objects/obj_persistent/Create_0.gml
Added SettingsManager (load/save/apply_video/sync_ui), manifest, new Boot room and persistent init that constructs/applies settings at startup. Attention: startup ordering and apply_video side effects.
Global settings migration (audio & autosave)
objects/obj_controller/.../Alarm_7.gml, objects/obj_controller/.../Step_0.gml, objects/obj_creation/.../Create_0.gml, objects/obj_creation/.../Step_0.gml, objects/obj_formation_bar/.../Mouse_56.gml, objects/obj_ncombat/.../*.gml, objects/obj_saveload/Destroy_0.gml, scripts/scr_controller_helpers/scr_controller_helpers.gml, scripts/scr_creation/scr_creation.gml, scripts/scr_librarium/scr_librarium.gml
Replaced ad‑hoc globals/ini reads with global.settings.* lookups (master, music, sfx, autosave); updated audio_gain calculations to use new fields. Attention: confirm all audio sites now read same units and defaults.
Main menu & title flow
objects/obj_main_menu/*.gml, objects/obj_main_menu/obj_main_menu.yy
Removed legacy INI/window/particle/timed‑stage logic; introduced fade_alpha/title_alpha driven flow, simplified music handling and removed several alarms/async handlers. Attention: music handle (global.current_music) and fade timing changes.
Menu/button UI refactor
objects/obj_main_menu_buttons/*, objects/obj_ingame_menu/*, objects/obj_new_button/*, scripts/macros/macros.gml
Converted to data‑driven buttons array, unified draw/step, added in‑menu volume controls and fullscreen toggle, added enum eIN_GAME_MENU_EFFECT, and integrated add_draw_return_values()/pop_draw_return_values(). Attention: new enum values and button callback lifecycle.
Window/runtime sync
objects/obj_controller/Create_0.gml, objects/obj_persistent/Step_0.gml, ChapterMaster.yyp
Removed legacy window geometry/INI handling from controller; added window/fullscreen polling in persistent step to sync/save via SettingsManager; Boot room added to project manifest. Attention: race conditions on initial window sizing.
Removed / cleanup
objects/obj_lol_version/*, scripts/__global_object_depths/__global_object_depths.gml, objects/obj_saveload/*
Deleted obj_lol_version resource and removed references; stripped legacy restart/INI code from saveload and other minor cleanups. Attention: ensure no remaining references to removed object.
Minor autosave & room edits
scripts/scr_save/scr_save.gml, rooms/Main_Menu/Main_Menu.yy
Adjusted autosave alarm index (alarm[2] → alarm[3]); removed several instances from Main_Menu room configuration. Attention: verify autosave trigger mappings.
Draw lifecycle helpers added
objects/obj_main_menu/Draw_0.gml, objects/obj_new_button/Draw_0.gml, objects/obj_main_menu_buttons/Draw_0.gml
Added draw-return helper usage (add_draw_return_values/pop_draw_return_values) to UI draw paths. Attention: ensure paired push/pop on all paths.

Possibly related PRs

Tech‑Priest: record these alterations in the sanctified changelog and run consecrated validation.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely absent, failing to provide any context, rationale, testing information, or related references required by the repository template. Tech-Priest must supply a comprehensive description addressing: purpose of the refactor, specific changes made, testing procedures executed, and any related context or issues.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title adheres to conventional commits format and is under 50 characters (49 characters), clearly describing the main refactoring effort.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 13


ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b197f68 and bf78911.

📒 Files selected for processing (55)
  • ChapterMaster.yyp
  • objects/obj_controller/Alarm_7.gml
  • objects/obj_controller/Create_0.gml
  • objects/obj_controller/Step_0.gml
  • objects/obj_creation/Alarm_1.gml
  • objects/obj_creation/Create_0.gml
  • objects/obj_creation/Step_0.gml
  • objects/obj_formation_bar/Mouse_56.gml
  • objects/obj_ingame_menu/Create_0.gml
  • objects/obj_ingame_menu/Draw_0.gml
  • objects/obj_ingame_menu/KeyPress_27.gml
  • objects/obj_ingame_menu/Step_0.gml
  • objects/obj_lol_version/Draw_0.gml
  • objects/obj_lol_version/obj_lol_version.yy
  • objects/obj_main_menu/Alarm_0.gml
  • objects/obj_main_menu/Alarm_1.gml
  • objects/obj_main_menu/Alarm_2.gml
  • objects/obj_main_menu/Alarm_3.gml
  • objects/obj_main_menu/Alarm_4.gml
  • objects/obj_main_menu/Create_0.gml
  • objects/obj_main_menu/Draw_0.gml
  • objects/obj_main_menu/Mouse_56.gml
  • objects/obj_main_menu/Other_2.gml
  • objects/obj_main_menu/Other_62.gml
  • objects/obj_main_menu/Step_0.gml
  • objects/obj_main_menu/obj_main_menu.yy
  • objects/obj_main_menu_buttons/Create_0.gml
  • objects/obj_main_menu_buttons/Draw_0.gml
  • objects/obj_main_menu_buttons/Mouse_50.gml
  • objects/obj_main_menu_buttons/Mouse_56.gml
  • objects/obj_main_menu_buttons/Step_0.gml
  • objects/obj_ncombat/Alarm_7.gml
  • objects/obj_ncombat/Create_0.gml
  • objects/obj_new_button/Draw_0.gml
  • objects/obj_new_button/Step_0.gml
  • objects/obj_persistent/Create_0.gml
  • objects/obj_persistent/Step_0.gml
  • objects/obj_saveload/Alarm_0.gml
  • objects/obj_saveload/Alarm_2.gml
  • objects/obj_saveload/Alarm_4.gml
  • objects/obj_saveload/Destroy_0.gml
  • objects/obj_saveload/Draw_0.gml
  • objects/obj_saveload/KeyPress_1.gml
  • objects/obj_saveload/Mouse_50.gml
  • objects/obj_saveload/Mouse_56.gml
  • objects/obj_saveload/obj_saveload.yy
  • rooms/Boot/Boot.yy
  • rooms/Main_Menu/Main_Menu.yy
  • scripts/SettingsManager/SettingsManager.gml
  • scripts/SettingsManager/SettingsManager.yy
  • scripts/__global_object_depths/__global_object_depths.gml
  • scripts/scr_controller_helpers/scr_controller_helpers.gml
  • scripts/scr_creation/scr_creation.gml
  • scripts/scr_librarium/scr_librarium.gml
  • scripts/scr_save/scr_save.gml
💤 Files with no reviewable changes (20)
  • objects/obj_saveload/Alarm_0.gml
  • objects/obj_main_menu/Other_62.gml
  • objects/obj_main_menu/Other_2.gml
  • objects/obj_main_menu/Alarm_4.gml
  • objects/obj_saveload/Alarm_2.gml
  • objects/obj_main_menu/obj_main_menu.yy
  • objects/obj_main_menu/Alarm_3.gml
  • rooms/Main_Menu/Main_Menu.yy
  • objects/obj_saveload/KeyPress_1.gml
  • objects/obj_main_menu_buttons/Mouse_50.gml
  • objects/obj_main_menu/Alarm_1.gml
  • objects/obj_creation/Alarm_1.gml
  • objects/obj_ingame_menu/KeyPress_27.gml
  • objects/obj_controller/Create_0.gml
  • objects/obj_lol_version/obj_lol_version.yy
  • objects/obj_main_menu_buttons/Mouse_56.gml
  • objects/obj_lol_version/Draw_0.gml
  • objects/obj_main_menu/Alarm_0.gml
  • objects/obj_main_menu/Mouse_56.gml
  • objects/obj_main_menu/Alarm_2.gml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.gml

⚙️ CodeRabbit configuration file

  • All code should comply with the 2026 GML documentation.

Files:

  • objects/obj_persistent/Step_0.gml
  • objects/obj_ncombat/Create_0.gml
  • objects/obj_creation/Step_0.gml
  • scripts/scr_save/scr_save.gml
  • objects/obj_ncombat/Alarm_7.gml
  • objects/obj_controller/Step_0.gml
  • objects/obj_new_button/Step_0.gml
  • scripts/scr_creation/scr_creation.gml
  • scripts/SettingsManager/SettingsManager.gml
  • objects/obj_controller/Alarm_7.gml
  • objects/obj_persistent/Create_0.gml
  • scripts/scr_librarium/scr_librarium.gml
  • scripts/__global_object_depths/__global_object_depths.gml
  • scripts/scr_controller_helpers/scr_controller_helpers.gml
  • objects/obj_formation_bar/Mouse_56.gml
  • objects/obj_main_menu_buttons/Step_0.gml
  • objects/obj_ingame_menu/Step_0.gml
  • objects/obj_main_menu_buttons/Create_0.gml
  • objects/obj_ingame_menu/Draw_0.gml
  • objects/obj_main_menu/Draw_0.gml
  • objects/obj_ingame_menu/Create_0.gml
  • objects/obj_main_menu_buttons/Draw_0.gml
  • objects/obj_creation/Create_0.gml
  • objects/obj_new_button/Draw_0.gml
  • objects/obj_main_menu/Step_0.gml
  • objects/obj_saveload/Destroy_0.gml
  • objects/obj_saveload/Draw_0.gml
  • objects/obj_main_menu/Create_0.gml
**/*.*

⚙️ CodeRabbit configuration file

**/*.*: - Code Philosophy: Prioritize explicit intent and maintainability over brevity. If a
solution is "clever" but mentally taxing, request a refactor to a clearer approach.

  • Variable Naming: Use clear, descriptive names; avoid over-abbreviation.

  • Abstraction: Apply the "Rule of Three"; suggest abstraction only when similar logic is
    repeated three or more times to avoid premature complexity.

  • Subjective Choices: For naming or architecture, ask guiding questions to prompt developer
    reflection and provide at least two alternative perspectives.

  • TODOs: If a TODO comment is added, ask the user if a GitHub issue should be created. If a
    TODO comment is deleted, remind the user to check the status of that specific issue.

Files:

  • objects/obj_persistent/Step_0.gml
  • objects/obj_ncombat/Create_0.gml
  • objects/obj_creation/Step_0.gml
  • scripts/scr_save/scr_save.gml
  • objects/obj_ncombat/Alarm_7.gml
  • objects/obj_controller/Step_0.gml
  • rooms/Boot/Boot.yy
  • objects/obj_new_button/Step_0.gml
  • scripts/scr_creation/scr_creation.gml
  • scripts/SettingsManager/SettingsManager.gml
  • scripts/SettingsManager/SettingsManager.yy
  • objects/obj_controller/Alarm_7.gml
  • objects/obj_persistent/Create_0.gml
  • scripts/scr_librarium/scr_librarium.gml
  • scripts/__global_object_depths/__global_object_depths.gml
  • scripts/scr_controller_helpers/scr_controller_helpers.gml
  • objects/obj_formation_bar/Mouse_56.gml
  • objects/obj_main_menu_buttons/Step_0.gml
  • objects/obj_ingame_menu/Step_0.gml
  • objects/obj_main_menu_buttons/Create_0.gml
  • objects/obj_ingame_menu/Draw_0.gml
  • objects/obj_main_menu/Draw_0.gml
  • objects/obj_ingame_menu/Create_0.gml
  • objects/obj_main_menu_buttons/Draw_0.gml
  • objects/obj_creation/Create_0.gml
  • objects/obj_new_button/Draw_0.gml
  • objects/obj_main_menu/Step_0.gml
  • objects/obj_saveload/obj_saveload.yy
  • objects/obj_saveload/Destroy_0.gml
  • objects/obj_saveload/Draw_0.gml
  • objects/obj_main_menu/Create_0.gml
  • ChapterMaster.yyp
**/*.yy

⚙️ CodeRabbit configuration file

**/*.yy: - When any script or sprite .yy files are deleted, their paths should also be deleted from the project .yyp file, otherwise the game will crash.

  • When any script or sprite .yy files are created, their paths should be added to the project .yyp file, otherwise they'll fail.

Files:

  • rooms/Boot/Boot.yy
  • scripts/SettingsManager/SettingsManager.yy
  • objects/obj_saveload/obj_saveload.yy
🧠 Learnings (5)
📓 Common learnings
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 424
File: scripts/scr_flavor/scr_flavor.gml:34-36
Timestamp: 2025-03-09T02:33:43.867Z
Learning: EttyKitty prefers to keep PRs focused on their stated goals and scope, and may decline to implement otherwise valid suggestions if they're not directly related to the PR's primary objective.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 579
File: objects/obj_enunit/Alarm_0.gml:200-202
Timestamp: 2025-03-11T01:38:19.874Z
Learning: EttyKitty welcomes easy, committable suggestions that improve documentation of code chunks, variables with strange names, and functions. Their codebase is generally lacking documentation, but they prioritize human-readable code above documentation.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 878
File: sprites/spr_weapon_phobos_bolt_pistol/spr_weapon_phobos_bolt_pistol.yy:26-44
Timestamp: 2025-06-16T17:08:08.239Z
Learning: EttyKitty prefers automated solutions over manual cleanup for .yy file formatting and is open to automated tools for GameMaker Studio .yy file cleanup.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 938
File: scripts/scr_complex_colour_kit/scr_complex_colour_kit.gml:478-478
Timestamp: 2025-07-21T17:03:28.251Z
Learning: EttyKitty acknowledges when PRs contain scope creep and agrees that changes should be focused on the stated PR objectives, reinforcing their preference for keeping PRs narrowly scoped to their primary purpose.
📚 Learning: 2026-01-28T13:37:09.640Z
Learnt from: OH296
Repo: Adeptus-Dominus/ChapterMaster PR: 646
File: objects/obj_pnunit/Alarm_5.gml:84-91
Timestamp: 2026-01-28T13:37:09.640Z
Learning: In obj_pnunit/Alarm_5.gml, the function get_armour_data("maintenance") will always return a numeric value (at minimum 0), making null/undefined checks unnecessary.

Applied to files:

  • scripts/scr_save/scr_save.gml
  • objects/obj_ncombat/Alarm_7.gml
  • objects/obj_controller/Alarm_7.gml
📚 Learning: 2026-01-28T13:37:09.640Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 649
File: objects/obj_enunit/Alarm_0.gml:289-291
Timestamp: 2026-01-28T13:37:09.640Z
Learning: GameMaker Studio's function `action_if_variable(image_index, -500, 0)` is auto-generated code from GameMaker's visual Drag and Drop system. It checks if image_index equals -500. In ChapterMaster, this was being used as a special flag for enemy unit movement, but wasn't triggering consistently, causing enemies to move only every other turn. The refactored code replaced this with direct function calls at specific combat stages.

Applied to files:

  • objects/obj_new_button/Step_0.gml
  • objects/obj_ingame_menu/Step_0.gml
  • objects/obj_saveload/Draw_0.gml
📚 Learning: 2026-02-07T01:59:24.823Z
Learnt from: CR
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: docs/CODE_STYLE.md:0-0
Timestamp: 2026-02-07T01:59:24.823Z
Learning: Applies to docs/**/*.gml : Don't create a separate file for every single small script; instead store scripts in library-like files (`scr_string_functions.gml`, `scr_array_functions.gml`, etc.)

Applied to files:

  • scripts/scr_creation/scr_creation.gml
📚 Learning: 2025-04-05T20:58:21.881Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.

Applied to files:

  • objects/obj_ingame_menu/Step_0.gml
🔇 Additional comments (27)
objects/obj_saveload/obj_saveload.yy (1)

37-38: Geometry rites remain intact after numeric normalisation.

Tech-Priest, the conversion to integer physicsShapePoints preserves the same bounds and improves manifest clarity; no fault-signature detected.

scripts/__global_object_depths/__global_object_depths.gml (1)

72-72: Removal mapping is stable and index-safe.

Tech-Priest, this deactivation pattern preserves array topology while cleanly excluding removed assets from ID→depth binding.

Also applies to: 161-161

objects/obj_ncombat/Alarm_7.gml (1)

7-11: Volume routing to global settings is coherent.

Tech-Priest, this transition maintains prior behaviour while binding combat music gain to the new canonical settings container.

scripts/SettingsManager/SettingsManager.yy (1)

1-13: SettingsManager manifest integrity confirmed.

Tech-Priest, the script resource descriptor is structurally sound and properly anchored to the Scripts hierarchy.

rooms/Boot/Boot.yy (1)

1-57: Boot room seed configuration is sound.

Tech-Priest, the room initialises with a single persistent instance and clean baseline metadata, fitting the bootstrap objective.

ChapterMaster.yyp (1)

708-708: Project registry and room order are properly synchronised.

Tech-Priest, the new Boot room and SettingsManager script are registered in resources, and Boot is inserted into RoomOrderNodes, preventing manifest drift.

As per coding guidelines: **/*.yy: "When any script or sprite .yy files are created, their paths should be added to the project .yyp file, otherwise they'll fail."

Also applies to: 969-969, 1873-1881

scripts/scr_librarium/scr_librarium.gml (1)

53-53: Audio gain migration is consistent with the new settings canon.

Tech-Priest, this binding to global.settings keeps the librarium SFX path aligned with the refactored volume architecture.

objects/obj_main_menu/Create_0.gml (1)

6-8: The initialisation order is properly secured through the persistent object pattern.

Tech-Priest, global.settings is forged in obj_persistent's Create event (objects/obj_persistent/Create_0.gml, lines 3-5), which is the sole instance spawned within the Boot room. The object is marked persistent:true, ensuring it survives all subsequent room transitions. Thus, any instantiation of obj_main_menu in later rooms (such as Main_Menu) will encounter an already-initialised global.settings. The examined transition pathways confirm no code execution can bypass this bootstrap sequence.

The implementation is architecturally sound. No corrective action is required.

scripts/scr_save/scr_save.gml (1)

97-97: Alarm reroute is coherent.
Tech-Priest, Line 97’s shift to alarm[3] aligns with the declared screenshot/UI reactivation path in this fragment.

objects/obj_ncombat/Create_0.gml (1)

19-23: Volume-source migration is sound.
Tech-Priest, Lines 19-23 preserve gating semantics while cleanly drawing from global.settings.

objects/obj_saveload/Destroy_0.gml (1)

9-13: This transition path is stable.
Tech-Priest, Lines 9-13 correctly preserve the mute gate while shifting to global.settings.

objects/obj_creation/Step_0.gml (1)

23-25: Audio settings integration looks correct.
Tech-Priest, Lines 23-25 apply the new global.settings source cleanly with proper positive-volume gating.

objects/obj_formation_bar/Mouse_56.gml (1)

111-113: Error-sound volume migration is coherent.
Tech-Priest, these branches preserve prior control flow while correctly sourcing gain from global.settings.

Also applies to: 125-127

objects/obj_persistent/Create_0.gml (1)

17-17: This concern is unfounded.
Tech-Priest, SettingsManager does not manage language state; it loads only audio volumes, fullscreen setting, and window data. No language persistence exists to be overridden at Line 17.

Likely an incorrect or invalid review comment.

scripts/scr_controller_helpers/scr_controller_helpers.gml (2)

440-440: Centralised settings access confirmed, Tech-Priest.

The migration to global.settings.settings_autosave aligns with the sacred SettingsManager module. This machine spirit observes consistency with the broader ritual of configuration centralisation.


465-465: Audio gain calculation appropriately refactored.

The transition from effect_volume to global.settings.sfx_volume and the adoption of global.settings.master_volume maintains consistency with the new settings infrastructure. The logic remains sound.

objects/obj_ingame_menu/Create_0.gml (2)

9-17: Commendable application of the sacred DRY principle, Tech-Priest.

The _spawn_button helper function consolidates repeated button creation logic into a single invocation pattern. This pleases the machine spirit.


25-27: Global button destruction acknowledged, Tech-Priest—yet vigilance remains warranted.

The invocation with (obj_new_button) instance_destroy() doth indeed annihilate all button instances across the realm. Analysis reveals this occurs during menu transitions when entering the Main_Menu room, suggesting deliberate state reset. The pattern is replicated elsewhere (obj_saveload similarly purges buttons). However, the architecture remains fragile: should any subsystem forge buttons independent of this menu object, they shall be unmade without mercy. Consider affixing commentary to justify why such global obliteration is sanctioned, or alternatively, tracking spawned instances to destroy only those belonging to this object's dominion.

objects/obj_creation/Create_0.gml (1)

48-52: Audio configuration migration verified, Tech-Priest.

The transition to global.settings.master_volume and global.settings.music_volume aligns with the sacred SettingsManager infrastructure. The logic flow remains unchanged; only the source of truth has been unified.

objects/obj_controller/Step_0.gml (2)

69-84: Music volume configuration unified, Tech-Priest.

The invocations for snd_blood and snd_royal now draw upon the centralised global.settings.music_volume source. The machine spirit acknowledges this alignment with the SettingsManager doctrine.


436-441: Sound effect volume migration complete.

The click sound invocations correctly utilise global.settings.sfx_volume rather than the former effect_volume. Consistency across the audio subsystem is maintained.

objects/obj_saveload/Draw_0.gml (1)

1-1: Guard condition purified, Tech-Priest.

The archaic DnD-generated action_if_variable incantation has been replaced with a direct boolean check. This cleansing improves readability whilst preserving the original logic. The machine spirit approves.

objects/obj_main_menu_buttons/Create_0.gml (1)

1-66: The ritual completes without impediment, Tech-Priest.

The sacred function start_room_transition dwells within the SettingsManager scriptorium and performs its designated duty with precision. Your data-driven button apparatus maintains clarity through explicit intent and descriptive nomenclature. The separation of button structure from action assignment upholds the principles of maintainability.

The machine spirit finds no corruption within these passages of logic.

objects/obj_main_menu/Step_0.gml (1)

5-15: Fade-to-title sequencing is sound, Tech-Priest.

State transitions are clear, and the cursor update is properly guarded.

objects/obj_main_menu_buttons/Step_0.gml (1)

15-36: Interaction gating and callback dispatch are well-forged, Tech-Priest.

The sequence is explicit and maintainable.

scripts/SettingsManager/SettingsManager.gml (1)

55-57: The machine spirits of the codebase reveal no evidence of application_surface being disabled or freed. The absence of application_surface_enable() or surface_free() calls means the surface existence is already assured in these call contexts. Adding preemptive existence checks would constitute unnecessary overhead. The code requires no modification, Tech-Priest.

Likely an incorrect or invalid review comment.

objects/obj_ingame_menu/Step_0.gml (1)

16-23: The autosave lifecycle is sound, Tech-Priest.

scr_save() executes synchronously and completes entirely within the function call. The destruction of _sl at line 22 occurs after the save operation has fully concluded—no deferred alarms, no asynchronous callbacks. The instance persists long enough for all serialisation and file I/O to complete. The code is correct.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (4)
objects/obj_main_menu/Draw_0.gml (1)

28-31: ⚠️ Potential issue | 🟡 Minor

Gate clipboard copy when metadata is empty, Tech-Priest.

The click rite still copies an empty newline and plays confirmation audio when both lines are blank.

Suggested patch
-if (point_and_click([1400, 830, 1600, 900])) {
-    clipboard_set_text($"{_build_date_line}\n{_version_line}");
+if (point_and_click([1400, 830, 1600, 900]) && (_build_date_line != "" || _version_line != "")) {
+    var _clip = _build_date_line;
+    if (_version_line != "") {
+        _clip = (_clip == "") ? _version_line : (_clip + "\n" + _version_line);
+    }
+    clipboard_set_text(_clip);
     audio_play_sound(snd_click_small, 0, false);
 }
objects/obj_new_button/Draw_0.gml (1)

6-7: ⚠️ Potential issue | 🔴 Critical

Guard sprite dimension queries behind sprite existence, Tech-Priest.

_sprite_index is dereferenced for width/height before validity is checked, risking undefined dimensions and corrupted text placement.

Suggested patch
-var _base_w = sprite_get_width(_sprite_index);
-var _base_h = sprite_get_height(_sprite_index);
+var _base_w = 0;
+var _base_h = 0;
+if (sprite_exists(_sprite_index)) {
+    _base_w = sprite_get_width(_sprite_index);
+    _base_h = sprite_get_height(_sprite_index);
+}
#!/bin/bash
set -euo pipefail

echo "== button_id assignments =="
rg -nP --type=gml '\bbutton_id\s*=\s*\d+' -C1

echo "== sprite presence check for used button_id values =="
python - <<'PY'
import re, subprocess, os

rg_out = subprocess.check_output(
    ["rg", "-nP", "--type=gml", r"\bbutton_id\s*=\s*(\d+)"],
    text=True
)
ids = sorted({int(m.group(1)) for m in re.finditer(r'button_id\s*=\s*(\d+)', rg_out)})

but_dirs = subprocess.check_output(["fd", "-td", "^spr_ui_but_", "sprites"], text=True).splitlines()
hov_dirs = subprocess.check_output(["fd", "-td", "^spr_ui_hov_", "sprites"], text=True).splitlines()

but = {os.path.basename(p.rstrip("/")) for p in but_dirs}
hov = {os.path.basename(p.rstrip("/")) for p in hov_dirs}

missing = []
for i in ids:
    b = f"spr_ui_but_{i}"
    h = f"spr_ui_hov_{i}"
    if b not in but or h not in hov:
        missing.append((i, b in but, h in hov))

print("button_ids:", ids)
print("missing entries (id, has_button_sprite, has_hover_sprite):", missing)
PY

Expected result: missing entries should be empty.

scripts/SettingsManager/SettingsManager.gml (1)

18-31: ⚠️ Potential issue | 🟠 Major

Sanitise settings payload at load boundary, Tech-Priest.

Raw INI values are accepted directly; malformed data can push audio values outside safe range and inject invalid window dimensions.

Suggested patch
-        master_volume = ini_read_real("Settings", "master_volume", 1);
-        music_volume = ini_read_real("Settings", "music_volume", 1);
-        sfx_volume = ini_read_real("Settings", "effect_volume", 1);
-        fullscreen = ini_read_real("Settings", "fullscreen", 1);
-        settings_autosave = ini_read_real("Settings", "settings_autosave", true);
+        master_volume = clamp(ini_read_real("Settings", "master_volume", 1), 0, 1);
+        music_volume = clamp(ini_read_real("Settings", "music_volume", 1), 0, 1);
+        sfx_volume = clamp(ini_read_real("Settings", "effect_volume", 1), 0, 1);
+        fullscreen = (ini_read_real("Settings", "fullscreen", 1) > 0);
+        settings_autosave = (ini_read_real("Settings", "settings_autosave", 1) > 0);
@@
-            window_rect.w = real(parts[2]);
-            window_rect.h = real(parts[3]);
+            window_rect.w = max(320, real(parts[2]));
+            window_rect.h = max(180, real(parts[3]));
objects/obj_ingame_menu/Step_0.gml (1)

17-18: 🧹 Nitpick | 🔵 Trivial

Would clearer temporary identifiers reduce future misfires, Tech-Priest?

  • Perspective I: rename by role (_saveload_instance, _back_button, _master_volume_gain).
  • Perspective II: extract helper rites so short-lived locals largely vanish.

As per coding guidelines "Variable Naming: Use clear, descriptive names; avoid over-abbreviation." and "Subjective Choices: For naming or architecture, ask guiding questions to prompt developer reflection and provide at least two alternative perspectives."

Also applies to: 25-33, 40-46, 151-153


ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf78911 and 991c1ae.

📒 Files selected for processing (11)
  • objects/obj_ingame_menu/Create_0.gml
  • objects/obj_ingame_menu/Draw_0.gml
  • objects/obj_ingame_menu/Step_0.gml
  • objects/obj_main_menu/Draw_0.gml
  • objects/obj_main_menu_buttons/Create_0.gml
  • objects/obj_main_menu_buttons/Draw_0.gml
  • objects/obj_new_button/Draw_0.gml
  • objects/obj_new_button/Step_0.gml
  • objects/obj_persistent/Step_0.gml
  • scripts/SettingsManager/SettingsManager.gml
  • scripts/macros/macros.gml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.gml

⚙️ CodeRabbit configuration file

  • All code should comply with the 2026 GML documentation.

Files:

  • objects/obj_ingame_menu/Create_0.gml
  • objects/obj_new_button/Draw_0.gml
  • objects/obj_main_menu/Draw_0.gml
  • objects/obj_new_button/Step_0.gml
  • objects/obj_ingame_menu/Step_0.gml
  • scripts/SettingsManager/SettingsManager.gml
  • scripts/macros/macros.gml
  • objects/obj_ingame_menu/Draw_0.gml
  • objects/obj_main_menu_buttons/Draw_0.gml
  • objects/obj_main_menu_buttons/Create_0.gml
  • objects/obj_persistent/Step_0.gml
**/*.*

⚙️ CodeRabbit configuration file

**/*.*: - Code Philosophy: Prioritize explicit intent and maintainability over brevity. If a
solution is "clever" but mentally taxing, request a refactor to a clearer approach.

  • Variable Naming: Use clear, descriptive names; avoid over-abbreviation.

  • Abstraction: Apply the "Rule of Three"; suggest abstraction only when similar logic is
    repeated three or more times to avoid premature complexity.

  • Subjective Choices: For naming or architecture, ask guiding questions to prompt developer
    reflection and provide at least two alternative perspectives.

  • TODOs: If a TODO comment is added, ask the user if a GitHub issue should be created. If a
    TODO comment is deleted, remind the user to check the status of that specific issue.

Files:

  • objects/obj_ingame_menu/Create_0.gml
  • objects/obj_new_button/Draw_0.gml
  • objects/obj_main_menu/Draw_0.gml
  • objects/obj_new_button/Step_0.gml
  • objects/obj_ingame_menu/Step_0.gml
  • scripts/SettingsManager/SettingsManager.gml
  • scripts/macros/macros.gml
  • objects/obj_ingame_menu/Draw_0.gml
  • objects/obj_main_menu_buttons/Draw_0.gml
  • objects/obj_main_menu_buttons/Create_0.gml
  • objects/obj_persistent/Step_0.gml
🧠 Learnings (10)
📓 Common learnings
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 424
File: scripts/scr_flavor/scr_flavor.gml:34-36
Timestamp: 2025-03-09T02:33:43.867Z
Learning: EttyKitty prefers to keep PRs focused on their stated goals and scope, and may decline to implement otherwise valid suggestions if they're not directly related to the PR's primary objective.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 579
File: objects/obj_enunit/Alarm_0.gml:200-202
Timestamp: 2025-03-11T01:38:19.874Z
Learning: EttyKitty welcomes easy, committable suggestions that improve documentation of code chunks, variables with strange names, and functions. Their codebase is generally lacking documentation, but they prioritize human-readable code above documentation.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 878
File: sprites/spr_weapon_phobos_bolt_pistol/spr_weapon_phobos_bolt_pistol.yy:26-44
Timestamp: 2025-06-16T17:08:08.239Z
Learning: EttyKitty prefers automated solutions over manual cleanup for .yy file formatting and is open to automated tools for GameMaker Studio .yy file cleanup.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 938
File: scripts/scr_complex_colour_kit/scr_complex_colour_kit.gml:478-478
Timestamp: 2025-07-21T17:03:28.251Z
Learning: EttyKitty acknowledges when PRs contain scope creep and agrees that changes should be focused on the stated PR objectives, reinforcing their preference for keeping PRs narrowly scoped to their primary purpose.
📚 Learning: 2026-01-28T13:37:09.640Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 649
File: objects/obj_enunit/Alarm_0.gml:289-291
Timestamp: 2026-01-28T13:37:09.640Z
Learning: GameMaker Studio's function `action_if_variable(image_index, -500, 0)` is auto-generated code from GameMaker's visual Drag and Drop system. It checks if image_index equals -500. In ChapterMaster, this was being used as a special flag for enemy unit movement, but wasn't triggering consistently, causing enemies to move only every other turn. The refactored code replaced this with direct function calls at specific combat stages.

Applied to files:

  • objects/obj_ingame_menu/Create_0.gml
  • objects/obj_new_button/Draw_0.gml
  • objects/obj_main_menu/Draw_0.gml
  • objects/obj_ingame_menu/Step_0.gml
📚 Learning: 2025-03-31T15:41:45.611Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 649
File: objects/obj_enunit/Alarm_0.gml:289-291
Timestamp: 2025-03-31T15:41:45.611Z
Learning: GameMaker Studio's function `action_if_variable(image_index, -500, 0)` was part of an old enemy movement system in ChapterMaster. This syntax is auto-generated from GameMaker's Drag-and-Drop interface and checks if image_index is greater than or equal to -500. In the refactored code, enemy movement is triggered directly through function calls rather than relying on this conditional check.

Applied to files:

  • objects/obj_new_button/Draw_0.gml
📚 Learning: 2025-04-25T21:13:54.164Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 766
File: scripts/scr_draw_unit_image/scr_draw_unit_image.gml:48-53
Timestamp: 2025-04-25T21:13:54.164Z
Learning: For GameMaker projects, avoid suggesting surface_exists() checks before every surface function call, especially when the function is primarily used in contexts where surface existence is already confirmed. This would create unnecessary overhead.

Applied to files:

  • objects/obj_new_button/Draw_0.gml
📚 Learning: 2025-04-05T20:58:21.881Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.

Applied to files:

  • objects/obj_new_button/Step_0.gml
  • objects/obj_ingame_menu/Step_0.gml
📚 Learning: 2025-06-16T17:12:13.045Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 878
File: scripts/scr_culture_visuals/scr_culture_visuals.gml:1256-1352
Timestamp: 2025-06-16T17:12:13.045Z
Learning: In scripts/scr_culture_visuals/scr_culture_visuals.gml, the weapon visual data declarations contain known DRY violations that are acknowledged by the development team but deferred to future refactoring efforts rather than addressed in individual feature PRs.

Applied to files:

  • objects/obj_new_button/Step_0.gml
📚 Learning: 2025-03-29T10:30:25.598Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 647
File: scripts/scr_en_weapon/scr_en_weapon.gml:24-928
Timestamp: 2025-03-29T10:30:25.598Z
Learning: A data-driven approach for weapon management in `scripts/scr_en_weapon/scr_en_weapon.gml` is planned for a future PR, not within the scope of PR `#647`.

Applied to files:

  • objects/obj_new_button/Step_0.gml
📚 Learning: 2025-03-06T16:02:06.286Z
Learnt from: MCPO-Spartan-117
Repo: Adeptus-Dominus/ChapterMaster PR: 554
File: objects/obj_popup/Step_0.gml:756-767
Timestamp: 2025-03-06T16:02:06.286Z
Learning: The variable 'woopwoopwoop' in the ChapterMaster codebase is a poorly named multi-purpose variable used both for controlling ancient ruins combat flow and for passing menu state in the save/load system. This type of naming makes code maintenance difficult.

Applied to files:

  • objects/obj_ingame_menu/Step_0.gml
  • objects/obj_persistent/Step_0.gml
📚 Learning: 2026-02-07T01:59:24.823Z
Learnt from: CR
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: docs/CODE_STYLE.md:0-0
Timestamp: 2026-02-07T01:59:24.823Z
Learning: Applies to docs/**/*.gml : Local variables should have a `_` prefix (e.g., `var _player_health`) to ease readability and prevent namespace clashes with instance variables and scripts. Not required for loop indices.

Applied to files:

  • objects/obj_ingame_menu/Step_0.gml
📚 Learning: 2025-02-25T13:49:56.926Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 514
File: scripts/scr_specialist_training/scr_specialist_training.gml:321-327
Timestamp: 2025-02-25T13:49:56.926Z
Learning: In GameMaker Language (GML), there are two ways to define true constants: macros and enumerators. Macros are defined with the syntax `#macro NAME VALUE` (no equals sign or semicolon) and can hold any value type. Enumerators use the syntax `enum Name { VALUE1, VALUE2 }` and can only contain integer values. Both are immutable at runtime and serve different purposes in code organization.

Applied to files:

  • scripts/macros/macros.gml
🔇 Additional comments (7)
objects/obj_new_button/Step_0.gml (1)

140-148: Enum-gated close rite is clear and sound, Tech-Priest.

The CLOSE_SAVELOAD branch cleanly separates saveload teardown from normal menu-effect routing.

objects/obj_main_menu_buttons/Draw_0.gml (1)

3-19: Render loop and shader lifecycle are coherent, Tech-Priest.

Single-pass per-button draw with explicit shader_reset() is solid.

scripts/macros/macros.gml (1)

125-134: Menu-effect enum codifies intent cleanly, Tech-Priest.

Named effects remove numeral lore from UI flow handling.

objects/obj_ingame_menu/Create_0.gml (1)

9-17: Helper rite is clean and maintainable, Tech-Priest.

The _spawn_button function centralises repeated button initialisation well.

objects/obj_ingame_menu/Draw_0.gml (2)

1-1: Draw-state rite is properly bracketed, Tech-Priest.

State push/pop pairing is present and improves render safety.

Also applies to: 60-60


27-46: Volume rendering loop is a sound refactor, Tech-Priest.

The _vols/_vol_y arrays with a single loop reduce duplication and keep rows consistent.

objects/obj_ingame_menu/Step_0.gml (1)

116-137: Slider interaction matrix aligns cleanly, Tech-Priest.

The _vol_y + _keys loop keeps hitboxes and setting keys synchronised.

@EttyKitty EttyKitty merged commit e304d98 into main Mar 3, 2026
2 of 4 checks passed
@EttyKitty EttyKitty deleted the refactor/main-menu branch March 3, 2026 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: Epic Type: Refactor Rewriting/restructuring code, while keeping general behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant