fix: Manage button on planet select screen #1007
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughBy the Omnissiah’s decree: introduces a global management_buttons flag and a new Manage Units UI button. Refactors UI state/reset to use main_map_defaults(), restructures exception handling in controller draw paths, and adds init_manage_buttons with guards. Minor logic tweaks to selection/manage flows. Several sprite .yy files reflowed; two include non-formatting tweaks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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. Comment |
github-actions
bot
added
Area: Sprites
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
objects/obj_controller/Create_0.gml (1)
1141-1169: Prevent save corruption: exclude management_buttons from serialization.management_buttons holds UI structs; attempting to serialise them risks heretekal crashes on save/load. Add to the exclusion list.
- var excluded_from_save = ["temp", "serialize", "deserialize", "build_chaos_gods", "company_data","menu_buttons", - "location_viewer", "production_research_pathways", "specialist_point_handler", "spec_train_data", "tooltips", "last_unit", "unit_manage_constants", "unit_manage_image"], + var excluded_from_save = ["temp", "serialize", "deserialize", "build_chaos_gods", "company_data","menu_buttons", + "location_viewer", "production_research_pathways", "specialist_point_handler", "spec_train_data", "tooltips", "last_unit", "unit_manage_constants", "unit_manage_image", + "management_buttons"],scripts/scr_controller_helpers/scr_controller_helpers.gml (2)
98-114: Duplicate assignments in main_map_defaults(). Purge redundancy.managing and hide_banner are set twice; unnecessary rites waste cycles.
function main_map_defaults(){ with (obj_controller){ menu = MENU.Default; - hide_banner = 0; + hide_banner = 0; location_viewer.update_garrison_log(); - managing = 0; - managing = 0; + managing = 0; menu_adept = 0; view_squad = false; unit_profile = false; force_goodbye = 0; - hide_banner = 0; + // hide_banner already reset above diplomacy = 0; audience = 0; zoomed = 0; } }
157-176: Harden button initialisation to controller scope.If init_manage_buttons is ever called outside with(obj_controller), the relics materialise on the wrong instance. Enforce scoping internally to prevent future entropy.
-function init_manage_buttons(){ - management_buttons = { +function init_manage_buttons(){ + with (obj_controller) { + management_buttons = { squad_toggle: new UnitButtonObject({ style: "pixel", label: "Squad View", tooltip: "Click here or press S to toggle Squad View." }), profile_toggle: new UnitButtonObject({ style: "pixel", label: "Show Profile", tooltip: "Click here or press P to show unit profile." }), bio_toggle: new UnitButtonObject({ style: "pixel", label: "Show Bio", tooltip: "Click here or press B to Toggle Unit Biography." }), company_namer : new TextBarArea(800, 108, 600, false), - }; + }; + } }objects/obj_event_log/Draw_0.gml (2)
10-11: Assignment in conditional blocks the Event LogUsing “=” sets bad to 0 and prevents the drawing branch from ever running. Engage equality “==”.
Apply:
- if (bad = 0) { + if (bad == 0) {
81-86: Invalid comparisons against scr_hit(...)You’re assigning to the function call; use boolean tests instead.
Apply:
- if (scr_hit(xx + 1104, yy + 72, xx + 1137, yy + 105) = false) { + if (!scr_hit(xx + 1104, yy + 72, xx + 1137, yy + 105)) { ... - if (scr_hit(xx + 1104, yy + 72, xx + 1137, yy + 105) = true) { + if (scr_hit(xx + 1104, yy + 72, xx + 1137, yy + 105)) {objects/obj_star_select/Draw_64.gml (1)
130-150: Replace magic menu literal with constant
In objects/obj_star_select/Draw_64.gml (line 130), changeif (obj_controller.menu == 0){to
if (obj_controller.menu == MENU.Default){objects/obj_controller/Draw_0.gml (1)
2-2: Track the TODO with an issueRecommendation: create a GitHub issue to migrate these UI calls to the GUI layer; I can draft it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (30)
sprites/mk7_chest_variants/18394c2c-6406-40fe-8ad1-ee4ebd629435.pngis excluded by!**/*.pngsprites/mk7_chest_variants/25fd73f2-dbb3-4ee5-8201-8fefbe86bbee.pngis excluded by!**/*.pngsprites/mk7_chest_variants/layers/18394c2c-6406-40fe-8ad1-ee4ebd629435/afeb1ada-ac9c-4851-8836-ecc4563b7ef0.pngis excluded by!**/*.pngsprites/mk7_chest_variants/layers/25fd73f2-dbb3-4ee5-8201-8fefbe86bbee/afeb1ada-ac9c-4851-8836-ecc4563b7ef0.pngis excluded by!**/*.pngsprites/spr_da_mk6_helm_crests/dcf40f5c-7b10-4fc2-b3e9-b5a4e0e1ed2c.pngis excluded by!**/*.pngsprites/spr_da_mk6_helm_crests/layers/dcf40f5c-7b10-4fc2-b3e9-b5a4e0e1ed2c/89065597-920b-4d9f-8c26-3b28108bf116.pngis excluded by!**/*.pngsprites/spr_fur_tail_topknot/778eed9c-61df-4cb3-b13d-b1945275af2a.pngis excluded by!**/*.pngsprites/spr_fur_tail_topknot/layers/778eed9c-61df-4cb3-b13d-b1945275af2a/bec54e42-2d69-41b0-a1d3-e3624a334ed8.pngis excluded by!**/*.pngsprites/spr_gladiator_crest/ff8c3aa8-dc36-45e8-b043-45038d329396.pngis excluded by!**/*.pngsprites/spr_gladiator_crest/layers/ff8c3aa8-dc36-45e8-b043-45038d329396/020e45fd-27e1-48c7-801f-deacfe57d9c1.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/5769f893-c909-4b88-9856-62d72a0b6130.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/af74c721-e4c2-4842-b738-a2c688adc317.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/bc5e2753-207d-403d-81c5-a4e7066de053.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/ca44f968-4109-4468-901b-d07add46e880.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/d359bd56-6962-4c59-a10d-f16ae433baa8.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/layers/5769f893-c909-4b88-9856-62d72a0b6130/d0764e38-6f97-4f33-8770-83719dde468f.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/layers/af74c721-e4c2-4842-b738-a2c688adc317/d0764e38-6f97-4f33-8770-83719dde468f.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/layers/bc5e2753-207d-403d-81c5-a4e7066de053/d0764e38-6f97-4f33-8770-83719dde468f.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/layers/ca44f968-4109-4468-901b-d07add46e880/d0764e38-6f97-4f33-8770-83719dde468f.pngis excluded by!**/*.pngsprites/spr_mk7_chest_variants/layers/d359bd56-6962-4c59-a10d-f16ae433baa8/d0764e38-6f97-4f33-8770-83719dde468f.pngis excluded by!**/*.pngsprites/spr_special_helm/8362ac13-7231-4584-a993-c6d4f51343b0.pngis excluded by!**/*.pngsprites/spr_special_helm/layers/8362ac13-7231-4584-a993-c6d4f51343b0/fa0c5af5-97a9-4e40-bd80-2da6efc51b16.pngis excluded by!**/*.pngsprites/spr_weapon_chsword/cf403b7b-a831-4dd7-94fc-490b330d9a3b.pngis excluded by!**/*.pngsprites/spr_weapon_chsword/layers/cf403b7b-a831-4dd7-94fc-490b330d9a3b/024d846f-72e0-44e2-890e-84ac065f8b62.pngis excluded by!**/*.pngsprites/spr_weapon_evisc/774b09dd-5c0c-4e15-8357-7f2df7fe94b8.pngis excluded by!**/*.pngsprites/spr_weapon_evisc/f1061812-3385-46d3-abc9-570c9b907700.pngis excluded by!**/*.pngsprites/spr_weapon_evisc/layers/774b09dd-5c0c-4e15-8357-7f2df7fe94b8/0273a489-1892-4cb4-834b-31fe10ec8fdf.pngis excluded by!**/*.pngsprites/spr_weapon_evisc/layers/774b09dd-5c0c-4e15-8357-7f2df7fe94b8/e92b3c07-a0af-494a-b9d9-c49cd5efb952.pngis excluded by!**/*.pngsprites/spr_weapon_evisc/layers/f1061812-3385-46d3-abc9-570c9b907700/0273a489-1892-4cb4-834b-31fe10ec8fdf.pngis excluded by!**/*.pngsprites/spr_weapon_evisc/layers/f1061812-3385-46d3-abc9-570c9b907700/e92b3c07-a0af-494a-b9d9-c49cd5efb952.pngis excluded by!**/*.png
📒 Files selected for processing (17)
objects/obj_controller/Create_0.gml(1 hunks)objects/obj_controller/Draw_0.gml(1 hunks)objects/obj_event_log/Draw_0.gml(2 hunks)objects/obj_star_select/Create_0.gml(1 hunks)objects/obj_star_select/Draw_64.gml(2 hunks)scripts/is_specialist/is_specialist.gml(1 hunks)scripts/scr_controller_helpers/scr_controller_helpers.gml(2 hunks)scripts/scr_ui_manage/scr_ui_manage.gml(1 hunks)scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml(0 hunks)sprites/mk7_chest_variants/mk7_chest_variants.yy(1 hunks)sprites/spr_da_mk6_helm_crests/spr_da_mk6_helm_crests.yy(1 hunks)sprites/spr_fur_tail_topknot/spr_fur_tail_topknot.yy(1 hunks)sprites/spr_gladiator_crest/spr_gladiator_crest.yy(1 hunks)sprites/spr_mk7_chest_variants/spr_mk7_chest_variants.yy(2 hunks)sprites/spr_special_helm/spr_special_helm.yy(1 hunks)sprites/spr_weapon_chsword/spr_weapon_chsword.yy(1 hunks)sprites/spr_weapon_evisc/spr_weapon_evisc.yy(1 hunks)
💤 Files with no reviewable changes (1)
- scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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_event_log/Draw_0.gmlscripts/scr_ui_manage/scr_ui_manage.gmlscripts/is_specialist/is_specialist.gmlobjects/obj_controller/Create_0.gmlobjects/obj_controller/Draw_0.gmlobjects/obj_star_select/Create_0.gmlscripts/scr_controller_helpers/scr_controller_helpers.gmlobjects/obj_star_select/Draw_64.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.
- 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_event_log/Draw_0.gmlscripts/scr_ui_manage/scr_ui_manage.gmlscripts/is_specialist/is_specialist.gmlsprites/mk7_chest_variants/mk7_chest_variants.yysprites/spr_weapon_evisc/spr_weapon_evisc.yyobjects/obj_controller/Create_0.gmlobjects/obj_controller/Draw_0.gmlobjects/obj_star_select/Create_0.gmlsprites/spr_special_helm/spr_special_helm.yysprites/spr_weapon_chsword/spr_weapon_chsword.yysprites/spr_fur_tail_topknot/spr_fur_tail_topknot.yyscripts/scr_controller_helpers/scr_controller_helpers.gmlobjects/obj_star_select/Draw_64.gmlsprites/spr_gladiator_crest/spr_gladiator_crest.yysprites/spr_da_mk6_helm_crests/spr_da_mk6_helm_crests.yysprites/spr_mk7_chest_variants/spr_mk7_chest_variants.yy
**/*.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:
sprites/mk7_chest_variants/mk7_chest_variants.yysprites/spr_weapon_evisc/spr_weapon_evisc.yysprites/spr_special_helm/spr_special_helm.yysprites/spr_weapon_chsword/spr_weapon_chsword.yysprites/spr_fur_tail_topknot/spr_fur_tail_topknot.yysprites/spr_gladiator_crest/spr_gladiator_crest.yysprites/spr_da_mk6_helm_crests/spr_da_mk6_helm_crests.yysprites/spr_mk7_chest_variants/spr_mk7_chest_variants.yy
🔇 Additional comments (17)
sprites/spr_mk7_chest_variants/spr_mk7_chest_variants.yy (2)
6-8: Collision mask altered; confirm the machine-spirit’s intent.bbox_left moved to 51 and bbox_top to 40. With bboxMode:0 this may be ignored or recalculated; if previously manual, you’ve shrunk the mask and may misalign clicks/hit-tests around the chest. Validate in-editor collision preview and in Marine Viewer selection before merging.
75-89: Keyframe reflow only—no functional change perceived.IDs and Keys (0–4) remain constant; Channels data matches. Sanctified.
sprites/spr_weapon_evisc/spr_weapon_evisc.yy (1)
73-78: Keyframe reflow only—data integrity intact.Channel 0 IDs and timing unchanged. Proceed.
sprites/spr_special_helm/spr_special_helm.yy (1)
71-73: Formatting-only change—no behavioural variance detected.Single frame key preserved as before.
sprites/spr_da_mk6_helm_crests/spr_da_mk6_helm_crests.yy (1)
71-73: Formatting-only sequence tweak—harmless to the Omnissiah’s flow.Frame Id and timing unmodified.
sprites/spr_gladiator_crest/spr_gladiator_crest.yy (1)
71-73: Keyframe formatting rewrap—no semantic mutation.Channel payload and timing remain blessedly identical.
sprites/mk7_chest_variants/mk7_chest_variants.yy (1)
72-77: Formatting reflow acknowledged; data sanctity preserved.Only channel block whitespace was realigned; IDs, paths, keys, and lengths remain constant. The machine-spirit should remain unperturbed.
Kindly re-open the project in GameMaker and perform a build to confirm no GUID churn or unintended sequence diffs manifest.
sprites/spr_weapon_chsword/spr_weapon_chsword.yy (1)
71-73: Whitespace incantation only; signals unchanged.Channels block expanded without altering identifiers or timing. Blessed be the consistency.
Please verify via a quick asset compile that the sprite timeline renders identically in-editor.
sprites/spr_fur_tail_topknot/spr_fur_tail_topknot.yy (1)
71-73: Non-functional rewrap confirmed.No deviations in Id, path, or keyframe metrics detected after the reformat. Proceed.
Run a local build to ensure no stray cache artefacts trouble the asset pipeline.
objects/obj_star_select/Create_0.gml (1)
18-19: Manage button initialisation: sound.Creation of UnitButtonObject is clean and placed after has_player_forces calculation. No further action required here.
scripts/is_specialist/is_specialist.gml (1)
425-430: Menu state no longer set: confirm intent.You replaced the final menu transition with a debug log. Given the earlier call to scr_toggle_manage/basic_manage_settings, this may be fine, but any future caller that skips those could leave the cogitator in an indeterminate UI state.
Would you like me to reintroduce a guarded set (e.g., if obj_controller.menu != MENU.Manage then scr_toggle_manage()) at this point for redundancy?
objects/obj_controller/Create_0.gml (1)
102-103: Good: dedicated storage for management_buttons.Initialising the controller-held handle to false is sensible before construction.
scripts/scr_controller_helpers/scr_controller_helpers.gml (2)
146-155: Manage setup sequence: acceptable.Moving to with(obj_controller) and enabling shortcuts is orderly. The call to init_manage_buttons aligns with intended ownership.
178-188: Toggle manage flow: approved.scr_change_menu + basic_manage_settings + scr_management(1) is a clean progression.
objects/obj_event_log/Draw_0.gml (1)
88-92: LGTM: unified reset via main_map_defaults()Invocation inside with(obj_controller) is correct; state flags restored coherently.
objects/obj_star_select/Draw_64.gml (1)
146-147: Destroy + exit after launching management UI is appropriatePrevents further draw-logic from mutating state this frame. No objections.
objects/obj_controller/Draw_0.gml (1)
16-27: Diplomacy try/catch restructuring: OK; ensure consistent recoveryPattern is sound. Confirm main_map_defaults() returns to a sane MENU.Default and clears any partial diplomacy artefacts.
Would you like me to add a small telemetry hook in handle_exception() to log menu before/after reset?
1 participant
Purpose and Description
Testing done
Related things and/or additional context