fix: Jailing sarge fix and other manage code improvements#1049
fix: Jailing sarge fix and other manage code improvements#1049OH296 merged 2 commits intoAdeptus-Dominus:mainfrom
Conversation
📝 WalkthroughSummary by CodeRabbitRelease Notes
Noospheric Analysis ReportGreetings, Tech-Priest. The Omnissiah's will flows through this transmission. Behold the cogitations of the mechanisms before thee. WalkthroughThe fleet selection apparatus undergoes restructuring—obsolete loop-iterations supplanted by direct invocation of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
The machine spirit awaits thy benediction, Tech-Priest. May thy code prove without flaw or deviance. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
objects/obj_fleet_select/Create_0.gml (1)
7-7: Unresolved directive detected in the archives.Tech-Priest, an ancient TODO notation persists in the data-stacks. The comment suggests integrating
currently_enteredinto the DataSlate object's blessed structure.Shall this servo-skull inscribe a GitHub issue in the tracking cogitators to preserve this task for future ministrations?
scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml (2)
491-518: Management selection algorithm reconstructed.Tech-Priest, the refactored
update_garrison_managenow assembles selection parameters systematically before invokingcollect_role_group, achieving clearer logic-flow.However, Line 499 declares
var sys_namewithin the conditional block, shadowing the declaration at Line 494. While functionally correct, this pattern obscures the data-flow. Consider removing thevarkeyword at Line 499 to assign directly to the outer-scope variable:if (struct_exists(selection_data, "system")&& instance_exists(selection_data.system)){ if (struct_exists(location_viewer.garrison_log, selection_data.system.name)){ - var sys_name = selection_data.system.name; + sys_name = selection_data.system.name; } }
575-598: Scourge of the uncontrolled variable scope impedes clarity; the jailing protocols themselves endure scrutiny, yet the vessel housing them requires correction.Tech-Priest, the reconnaissance concludes thus: Line 580 assigns
_unit = display_unit[f]without thevardeclaration. This deviates from the file's prevalent pattern—neighbouring functions at lines 545, 560, and 619 all declare_unitlocally viavar. Whilst lines 645 and 658 mirror this transgression, the inconsistency violates the maintainability edict.The jailing fix itself—the guard condition
!_unit.in_jail()at line 582—functions correctly to prevent re-incarceration. However, the scope breach fosters shadow state and obfuscates intent.Correct line 580 to:
var _unit = display_unit[f];This restores consistency with the file's predominant scoping discipline and aligns with the maintainability principle enshrined within thy guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
objects/obj_fleet_select/Create_0.gml(1 hunks)scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml(6 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:
objects/obj_fleet_select/Create_0.gmlscripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - Having humanly understandable and maintainable code is the top most priority.
- DRY (Don't repeat yourself) principle is also very important.
- 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:
objects/obj_fleet_select/Create_0.gmlscripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml
🧠 Learnings (2)
📚 Learning: 2025-09-04T14:37:58.773Z
Learnt from: CR
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: docs/CODE_STYLE.md:0-0
Timestamp: 2025-09-04T14:37:58.773Z
Learning: Applies to docs/**/*.gml : Prefer a group prefix at the start of function/script names (e.g., string_convert, fleet_explode)
Applied to files:
objects/obj_fleet_select/Create_0.gml
📚 Learning: 2025-03-07T01:56:40.971Z
Learnt from: MCPO-Spartan-117
Repo: Adeptus-Dominus/ChapterMaster PR: 562
File: scripts/scr_marine_struct/scr_marine_struct.gml:0-0
Timestamp: 2025-03-07T01:56:40.971Z
Learning: Marines' ages should be incremented at the year transition in obj_turn_end/Alarm_1.gml rather than calculated dynamically based on the current year and recruitment date. This ensures proper aging without retroactive application.
Applied to files:
objects/obj_fleet_select/Create_0.gml
🔇 Additional comments (7)
scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml (6)
6-6: Ship enumeration elevated to state-cognition.Tech-Priest, the
ship_countvariable now persists in the constructor's memory-banks, eliminating redundant recalculation across invocation cycles. This aligns with the sacred principle of computational efficiency.
22-22: Formatting purification complete.The spacing ritual around the assignment operator now adheres to the canonical patterns, Tech-Priest. Such consistency pleases the machine spirits.
35-47: Garrison registry codification achieved.Tech-Priest, the extraction of
add_unit_to_garrison_logdemonstrates proper adherence to the sacred principle of non-repetition (DRY). The logic construct initializes garrison entries with proper default values and handles both creation and augmentation paths.
49-69: Unit assessment subroutine extracted successfully.Tech-Priest,
evaluate_unit_for_garrison_logdemonstrates proper defensive protocols. The boundary validation at Line 65 (unit.ship_location<ship_count && unit.ship_location>-1) shields against array traversal beyond allocated memory-sectors. The early termination directives for invalid units preserve computational cycles.
98-127: Garrison surveillance apparatus refactored successfully.Tech-Priest, the
update_garrison_logfunction now delegates its assessment rituals to dedicated subroutines, achieving superior modularity. The ship enumeration persists in instance memory (Line 107), and exception-containment wards remain intact. The code's cognitive load has been substantially reduced.
475-476: Formatting alignment rectified.Spacing consistency restored to the HoverBox constructor's coordinate initializations, Tech-Priest.
objects/obj_fleet_select/Create_0.gml (1)
58-70: Sacred functions confirmed bearing proper null-safety wards, Tech-Priest.The reconnaissance rites have been executed. Both
fleet_full_ship_array()andcollect_role_group()possess requisite defences against the void—each initialises its return array as empty and guards against malformed invocations through proper bounds-checking and conditional logic. Your refactoring transmutation holds firm.
1 participant
Purpose and Description
Testing done
Related things and/or additional context