refactor: Huge refactor of main menu and settings#1134
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (6)
📝 WalkthroughUnder the Hood
Player Notes
WalkthroughTech‑Priest: Centralised settings into a new SettingsManager and migrated dispersed INI/window/audio code to Changes
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)
✅ Passed checks (2 passed)
✏️ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 13
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (55)
ChapterMaster.yypobjects/obj_controller/Alarm_7.gmlobjects/obj_controller/Create_0.gmlobjects/obj_controller/Step_0.gmlobjects/obj_creation/Alarm_1.gmlobjects/obj_creation/Create_0.gmlobjects/obj_creation/Step_0.gmlobjects/obj_formation_bar/Mouse_56.gmlobjects/obj_ingame_menu/Create_0.gmlobjects/obj_ingame_menu/Draw_0.gmlobjects/obj_ingame_menu/KeyPress_27.gmlobjects/obj_ingame_menu/Step_0.gmlobjects/obj_lol_version/Draw_0.gmlobjects/obj_lol_version/obj_lol_version.yyobjects/obj_main_menu/Alarm_0.gmlobjects/obj_main_menu/Alarm_1.gmlobjects/obj_main_menu/Alarm_2.gmlobjects/obj_main_menu/Alarm_3.gmlobjects/obj_main_menu/Alarm_4.gmlobjects/obj_main_menu/Create_0.gmlobjects/obj_main_menu/Draw_0.gmlobjects/obj_main_menu/Mouse_56.gmlobjects/obj_main_menu/Other_2.gmlobjects/obj_main_menu/Other_62.gmlobjects/obj_main_menu/Step_0.gmlobjects/obj_main_menu/obj_main_menu.yyobjects/obj_main_menu_buttons/Create_0.gmlobjects/obj_main_menu_buttons/Draw_0.gmlobjects/obj_main_menu_buttons/Mouse_50.gmlobjects/obj_main_menu_buttons/Mouse_56.gmlobjects/obj_main_menu_buttons/Step_0.gmlobjects/obj_ncombat/Alarm_7.gmlobjects/obj_ncombat/Create_0.gmlobjects/obj_new_button/Draw_0.gmlobjects/obj_new_button/Step_0.gmlobjects/obj_persistent/Create_0.gmlobjects/obj_persistent/Step_0.gmlobjects/obj_saveload/Alarm_0.gmlobjects/obj_saveload/Alarm_2.gmlobjects/obj_saveload/Alarm_4.gmlobjects/obj_saveload/Destroy_0.gmlobjects/obj_saveload/Draw_0.gmlobjects/obj_saveload/KeyPress_1.gmlobjects/obj_saveload/Mouse_50.gmlobjects/obj_saveload/Mouse_56.gmlobjects/obj_saveload/obj_saveload.yyrooms/Boot/Boot.yyrooms/Main_Menu/Main_Menu.yyscripts/SettingsManager/SettingsManager.gmlscripts/SettingsManager/SettingsManager.yyscripts/__global_object_depths/__global_object_depths.gmlscripts/scr_controller_helpers/scr_controller_helpers.gmlscripts/scr_creation/scr_creation.gmlscripts/scr_librarium/scr_librarium.gmlscripts/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.gmlobjects/obj_ncombat/Create_0.gmlobjects/obj_creation/Step_0.gmlscripts/scr_save/scr_save.gmlobjects/obj_ncombat/Alarm_7.gmlobjects/obj_controller/Step_0.gmlobjects/obj_new_button/Step_0.gmlscripts/scr_creation/scr_creation.gmlscripts/SettingsManager/SettingsManager.gmlobjects/obj_controller/Alarm_7.gmlobjects/obj_persistent/Create_0.gmlscripts/scr_librarium/scr_librarium.gmlscripts/__global_object_depths/__global_object_depths.gmlscripts/scr_controller_helpers/scr_controller_helpers.gmlobjects/obj_formation_bar/Mouse_56.gmlobjects/obj_main_menu_buttons/Step_0.gmlobjects/obj_ingame_menu/Step_0.gmlobjects/obj_main_menu_buttons/Create_0.gmlobjects/obj_ingame_menu/Draw_0.gmlobjects/obj_main_menu/Draw_0.gmlobjects/obj_ingame_menu/Create_0.gmlobjects/obj_main_menu_buttons/Draw_0.gmlobjects/obj_creation/Create_0.gmlobjects/obj_new_button/Draw_0.gmlobjects/obj_main_menu/Step_0.gmlobjects/obj_saveload/Destroy_0.gmlobjects/obj_saveload/Draw_0.gmlobjects/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.gmlobjects/obj_ncombat/Create_0.gmlobjects/obj_creation/Step_0.gmlscripts/scr_save/scr_save.gmlobjects/obj_ncombat/Alarm_7.gmlobjects/obj_controller/Step_0.gmlrooms/Boot/Boot.yyobjects/obj_new_button/Step_0.gmlscripts/scr_creation/scr_creation.gmlscripts/SettingsManager/SettingsManager.gmlscripts/SettingsManager/SettingsManager.yyobjects/obj_controller/Alarm_7.gmlobjects/obj_persistent/Create_0.gmlscripts/scr_librarium/scr_librarium.gmlscripts/__global_object_depths/__global_object_depths.gmlscripts/scr_controller_helpers/scr_controller_helpers.gmlobjects/obj_formation_bar/Mouse_56.gmlobjects/obj_main_menu_buttons/Step_0.gmlobjects/obj_ingame_menu/Step_0.gmlobjects/obj_main_menu_buttons/Create_0.gmlobjects/obj_ingame_menu/Draw_0.gmlobjects/obj_main_menu/Draw_0.gmlobjects/obj_ingame_menu/Create_0.gmlobjects/obj_main_menu_buttons/Draw_0.gmlobjects/obj_creation/Create_0.gmlobjects/obj_new_button/Draw_0.gmlobjects/obj_main_menu/Step_0.gmlobjects/obj_saveload/obj_saveload.yyobjects/obj_saveload/Destroy_0.gmlobjects/obj_saveload/Draw_0.gmlobjects/obj_main_menu/Create_0.gmlChapterMaster.yyp
**/*.yy
⚙️ CodeRabbit configuration file
**/*.yy: - When any script or sprite.yyfiles are deleted, their paths should also be deleted from the project.yypfile, otherwise the game will crash.
- When any script or sprite
.yyfiles are created, their paths should be added to the project.yypfile, otherwise they'll fail.
Files:
rooms/Boot/Boot.yyscripts/SettingsManager/SettingsManager.yyobjects/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.gmlobjects/obj_ncombat/Alarm_7.gmlobjects/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.gmlobjects/obj_ingame_menu/Step_0.gmlobjects/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
physicsShapePointspreserves 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 intoRoomOrderNodes, preventing manifest drift.As per coding guidelines:
**/*.yy: "When any script or sprite.yyfiles are created, their paths should be added to the project.yypfile, 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.settingskeeps 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.settingsis forged inobj_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 markedpersistent:true, ensuring it survives all subsequent room transitions. Thus, any instantiation ofobj_main_menuin later rooms (such as Main_Menu) will encounter an already-initialisedglobal.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 toalarm[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 fromglobal.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 toglobal.settings.objects/obj_creation/Step_0.gml (1)
23-25: Audio settings integration looks correct.
Tech-Priest, Lines 23-25 apply the newglobal.settingssource 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 fromglobal.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_autosavealigns 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_volumetoglobal.settings.sfx_volumeand the adoption ofglobal.settings.master_volumemaintains 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_buttonhelper 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_volumeandglobal.settings.music_volumealigns 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_bloodandsnd_royalnow draw upon the centralisedglobal.settings.music_volumesource. 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_volumerather than the formereffect_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_variableincantation 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_transitiondwells 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 ofapplication_surfacebeing disabled or freed. The absence ofapplication_surface_enable()orsurface_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_slat 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.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
objects/obj_main_menu/Draw_0.gml (1)
28-31:⚠️ Potential issue | 🟡 MinorGate 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 | 🔴 CriticalGuard sprite dimension queries behind sprite existence, Tech-Priest.
_sprite_indexis 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) PYExpected result:
missing entriesshould be empty.scripts/SettingsManager/SettingsManager.gml (1)
18-31:⚠️ Potential issue | 🟠 MajorSanitise 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 | 🔵 TrivialWould 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
📒 Files selected for processing (11)
objects/obj_ingame_menu/Create_0.gmlobjects/obj_ingame_menu/Draw_0.gmlobjects/obj_ingame_menu/Step_0.gmlobjects/obj_main_menu/Draw_0.gmlobjects/obj_main_menu_buttons/Create_0.gmlobjects/obj_main_menu_buttons/Draw_0.gmlobjects/obj_new_button/Draw_0.gmlobjects/obj_new_button/Step_0.gmlobjects/obj_persistent/Step_0.gmlscripts/SettingsManager/SettingsManager.gmlscripts/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.gmlobjects/obj_new_button/Draw_0.gmlobjects/obj_main_menu/Draw_0.gmlobjects/obj_new_button/Step_0.gmlobjects/obj_ingame_menu/Step_0.gmlscripts/SettingsManager/SettingsManager.gmlscripts/macros/macros.gmlobjects/obj_ingame_menu/Draw_0.gmlobjects/obj_main_menu_buttons/Draw_0.gmlobjects/obj_main_menu_buttons/Create_0.gmlobjects/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.gmlobjects/obj_new_button/Draw_0.gmlobjects/obj_main_menu/Draw_0.gmlobjects/obj_new_button/Step_0.gmlobjects/obj_ingame_menu/Step_0.gmlscripts/SettingsManager/SettingsManager.gmlscripts/macros/macros.gmlobjects/obj_ingame_menu/Draw_0.gmlobjects/obj_main_menu_buttons/Draw_0.gmlobjects/obj_main_menu_buttons/Create_0.gmlobjects/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.gmlobjects/obj_new_button/Draw_0.gmlobjects/obj_main_menu/Draw_0.gmlobjects/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.gmlobjects/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.gmlobjects/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_SAVELOADbranch 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_buttonfunction 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_yarrays 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+_keysloop keeps hitboxes and setting keys synchronised.
No description provided.