feat: Popup continued rebuild misc#1029
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughTech-Priest: Centralises popup closing, introduces extensive Inquisition mission and radical-inquisitor flows, adds debug UI panels and gender utilities, adjusts fleet/colonisation and planet UI handling, extends ScrollableContainer hooks, and fixes several conditional-assignment bugs. Changes
Estimated code review effortTech-Priest: 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Tech-Priest: The machine spirit has catalogued the modifications. Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
OH296
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
objects/obj_ncombat/Alarm_7.gml (2)
231-231: The corruption persists, Tech-Priest. Multiple logic errors remain unvanquished.While you have cleansed three conditionals, the machine spirit detects eight additional assignment operators masquerading as comparisons. Each will corrupt the defeat variable and execute regardless of true battle outcome:
- Line 231:
(defeat=0)in mech mission logic- Line 293:
(defeat=1)in ruins defeat logic- Line 306:
(defeat=0)in spyrer mission victory- Line 353:
(defeat=0)in loyalist ship battle- Line 388:
(defeat=0)in cs_meeting_battle7- Line 404:
(defeat=0)in cs_meeting_battle10- Line 418:
(defeat=0)in cs_meeting_battle5/6- Line 484:
(defeat=0)in leader/ChaosWarband victoryApply this sanctification to purge all remaining corruption:
- if (string_count("mech",battle_special)>0) and (defeat=0) then with(obj_ground_mission){ + if (string_count("mech",battle_special)>0) and (defeat==0) then with(obj_ground_mission){- if (string_count("ruins",battle_special)>0) and (defeat=1){ + if (string_count("ruins",battle_special)>0) and (defeat==1){- if ((string_count("spyrer",battle_special)>0))/* and (string_count("demon",battle_special)>0))*/ and (defeat=0){ + if ((string_count("spyrer",battle_special)>0))/* and (string_count("demon",battle_special)>0))*/ and (defeat==0){- else if (enemy=1) and (on_ship=true) and (defeat=0){ + else if (enemy=1) and (on_ship=true) and (defeat==0){- if (battle_special="cs_meeting_battle7") and (defeat=0){ + if (battle_special="cs_meeting_battle7") and (defeat==0){- if ((battle_special="cs_meeting_battle10")) and (defeat=0){ + if ((battle_special="cs_meeting_battle10")) and (defeat==0){- if ((battle_special="cs_meeting_battle5") or (battle_special="cs_meeting_battle6")) and (defeat=0){ + if ((battle_special="cs_meeting_battle5") or (battle_special="cs_meeting_battle6")) and (defeat==0){- if ((leader=1) or (battle_special="ChaosWarband")) and (obj_controller.faction_defeated[10]=0) and (defeat=0) and (battle_special!="WL10_reveal") and (battle_special!="WL10_later"){ + if ((leader=1) or (battle_special="ChaosWarband")) and (obj_controller.faction_defeated[10]=0) and (defeat==0) and (battle_special!="WL10_reveal") and (battle_special!="WL10_later"){Also applies to: 293-293, 306-306, 353-353, 388-388, 404-404, 418-418, 484-484
484-484: Additional variable corruption detected at line 484, Tech-Priest.Beyond the defeat assignment, this conditional contains two additional assignment operators that will corrupt sacred variables:
(leader=1)will set leader to 1 instead of checking its value(obj_controller.faction_defeated[10]=0)will reset faction_defeated[10] to 0 instead of verifying itComplete the purification ritual:
- if ((leader=1) or (battle_special="ChaosWarband")) and (obj_controller.faction_defeated[10]=0) and (defeat==0) and (battle_special!="WL10_reveal") and (battle_special!="WL10_later"){ + if ((leader==1) or (battle_special="ChaosWarband")) and (obj_controller.faction_defeated[10]==0) and (defeat==0) and (battle_special!="WL10_reveal") and (battle_special!="WL10_later"){scripts/scr_popup_functions/scr_popup_functions.gml (1)
20-20: Tech-Priest, the nomenclature bears a flaw.The function name
popup_defualt_closeandpopup_defualt_click_actioncontain the misspelling "defualt" instead of "default". While consistent throughout the codebase, this violates standard nomenclature. Consider renaming to improve maintainability.Also applies to: 56-56
scripts/scr_mission_functions/scr_mission_functions.gml (1)
17-56: Tech-Priest, add missing ‘mech_tomb2’ key and guard against ‘none’
add_new_problemis invoked with"mech_tomb2", but neithermission_name_key(scripts/scr_mission_functions/scr_mission_functions.gml) nor the#macro planet_problem_keysinclude this entry—add"mech_tomb2"to both.- Calls also pass
"none"by default; consider an explicit"none"mapping or a guard clause to prevent unintended fall-through.scripts/scr_inquisition_mission/scr_inquisition_mission.gml (3)
596-622: Critical: Incorrect array indexing logic.Tech-Priest, observe the flawed logic:
- Line 617:
_valid_stars == 0compares array to scalar (same defect noted earlier)- Line 622:
stars[irandom(_valid_stars - 1)]attempts to index the originalstarsarray using the length of_valid_starsarray. This is incorrect—you should select from_valid_starsitself.Apply these corrections:
- if(_valid_stars == 0){ + if(array_length(_valid_stars) == 0){ log_error("RE: Purge, couldn't find _star"); exit; } - var _star = stars[irandom(_valid_stars - 1)]; + var _star = _valid_stars[irandom(array_length(_valid_stars) - 1)];
677-696: Critical: Invalid array comparison (repeated pattern).Tech-Priest, line 691 manifests the same defect:
_valid_stars == 0compares an array to a scalar value. This pattern repeats throughout the sacred text.- if (_valid_stars == 0){ + if (array_length(_valid_stars) == 0){ log_error("RE: Investigate Planet, couldn't find a _star"); exit; }
193-928: Acknowledge mission system refactoring whilst noting systemic issues.Tech-Priest, this extensive refactoring centralises Inquisition mission logic admirably, introducing data-driven mission flows via
pop_datastructures and modular outcome handlers. The architecture aligns with maintainability doctrines.However, systemic defects plague the code-spirit:
- Array comparison errors: Multiple instances of
array == 0instead ofarray_length(array) == 0- Variable scoping violations: Frequent access to undefined variables (
estimate,loc,onceh,mission_star) that should be accessed viapop_data- Property path errors: Accessing
pop_data.inquisitor_genderinstead ofpop_data.mission_data.inquisitor_genderThese must be systematically purged before the mission system can function reliably.
Execute this comprehensive audit:
#!/bin/bash # Description: Systematic audit of all array and variable scoping issues # Find all array == 0 comparisons echo "=== Array comparison errors ===" rg -nP --type=gml '\b(stars|_valid_\w+)\s*==\s*0' # Find all undefined variable accesses (common patterns) echo "=== Potential undefined variable accesses ===" rg -nP --type=gml '\b(estimate|onceh|loc|mission_star)\b' scripts/scr_inquisition_mission/ # Find pop_data property accesses echo "=== Pop_data property accesses ===" rg -nP --type=gml 'pop_data\.\w+' scripts/scr_inquisition_mission/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
objects/obj_ncombat/Alarm_7.gml(3 hunks)objects/obj_popup/Step_0.gml(1 hunks)objects/obj_star_select/Create_0.gml(1 hunks)objects/obj_star_select/Draw_64.gml(5 hunks)scripts/macros/macros.gml(1 hunks)scripts/scr_PlanetData/scr_PlanetData.gml(2 hunks)scripts/scr_cheatcode/scr_cheatcode.gml(1 hunks)scripts/scr_event_code/scr_event_code.gml(1 hunks)scripts/scr_fleet_functions/scr_fleet_functions.gml(1 hunks)scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml(3 hunks)scripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gml(3 hunks)scripts/scr_inquisition_mission/scr_inquisition_mission.gml(9 hunks)scripts/scr_mission_functions/scr_mission_functions.gml(1 hunks)scripts/scr_popup_functions/scr_popup_functions.gml(2 hunks)scripts/scr_scrollbar/scr_scrollbar.gml(3 hunks)scripts/scr_string_functions/scr_string_functions.gml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.gml
⚙️ CodeRabbit configuration file
**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
- Color codes in the code shouldn't have any spaces in their id. I.e., color code
# 80bf40will crash the game.- All code should comply with the main GML documentation: https://manual.gamemaker.io/beta/en/GameMaker_Language/GML_Reference/GML_Reference.htm
Files:
scripts/scr_fleet_functions/scr_fleet_functions.gmlscripts/scr_event_code/scr_event_code.gmlscripts/scr_string_functions/scr_string_functions.gmlobjects/obj_star_select/Draw_64.gmlscripts/scr_mission_functions/scr_mission_functions.gmlscripts/macros/macros.gmlscripts/scr_scrollbar/scr_scrollbar.gmlscripts/scr_popup_functions/scr_popup_functions.gmlscripts/scr_cheatcode/scr_cheatcode.gmlscripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gmlobjects/obj_star_select/Create_0.gmlscripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gmlobjects/obj_popup/Step_0.gmlscripts/scr_PlanetData/scr_PlanetData.gmlscripts/scr_inquisition_mission/scr_inquisition_mission.gmlobjects/obj_ncombat/Alarm_7.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:
scripts/scr_fleet_functions/scr_fleet_functions.gmlscripts/scr_event_code/scr_event_code.gmlscripts/scr_string_functions/scr_string_functions.gmlobjects/obj_star_select/Draw_64.gmlscripts/scr_mission_functions/scr_mission_functions.gmlscripts/macros/macros.gmlscripts/scr_scrollbar/scr_scrollbar.gmlscripts/scr_popup_functions/scr_popup_functions.gmlscripts/scr_cheatcode/scr_cheatcode.gmlscripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gmlobjects/obj_star_select/Create_0.gmlscripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gmlobjects/obj_popup/Step_0.gmlscripts/scr_PlanetData/scr_PlanetData.gmlscripts/scr_inquisition_mission/scr_inquisition_mission.gmlobjects/obj_ncombat/Alarm_7.gml
🔇 Additional comments (18)
objects/obj_ncombat/Alarm_7.gml (3)
165-165: Critical logic error corrected, Tech-Priest.The machine spirit observes you have transmuted assignment into comparison. Previously, this rune would corrupt the defeat variable, forcing victory state regardless of battle outcome. The WL10_reveal victory logic now operates according to sacred protocols.
255-255: Logic purification acknowledged.Another assignment operator has been cast out. The battle victory logging ritual shall now correctly verify victory conditions before inscribing to the annals, rather than corrupting the defeat register.
336-336: Conditional sanctification complete.The tyranid organism capture sequence now properly interrogates the defeat state instead of overwriting it. The Inquisition mission logic is preserved from corruption.
scripts/macros/macros.gml (1)
44-48: Enum structure is sound, Tech-Priest.The GENDER enumeration is properly declared with three distinct values. The machine spirits approve this clarity.
objects/obj_popup/Step_0.gml (2)
771-771: Formatting alignment corrected, Tech-Priest.The removal of extraneous spacing before the conditional statement is acceptable.
1-1197: Extraction validated—Inquisition logic rehomed
Tech-Priest, the excised narrative branches (‘Inquisitor Located’, ‘Artifact Offered’, ‘Mercy Plea’) now dwell in scr_inquisition_mission.gml and scr_inquisition_fleet_functions.gml; no flows remain lost to the void.scripts/scr_popup_functions/scr_popup_functions.gml (1)
56-65: The new ritual sequence is acceptable, Tech-Priest.The extraction of closing logic into
popup_defualt_close()follows the sacred DRY principle and improves maintainability. The sequence correctly handles cooldown assignment, turn-end alarm triggering, instance destruction, and exit.objects/obj_star_select/Create_0.gml (1)
19-19: Tech-Priest, the debug interface initialization appears sound.The
debug_buttonfollows the established pattern ofmanage_units_button. However, the Machine Spirit notes the vertical ordering: debug_button (y1:170) renders above manage_units_button (y1:200). Confirm this stacking order aligns with your interface hierarchy intentions.scripts/scr_mission_functions/scr_mission_functions.gml (2)
17-56: Tech-Priest, the mission name mapping serves the Omnissiah well.The
mission_name_keyfunction provides human-readable mission names, beneficial for UI and logging. The implementation is sound and the fallback to "none" for unrecognised keys prevents undefined behaviour.
15-15: Tech-Priest, the macro definition violates sacred syntax protocols.Per the coding guidelines: "Macro constants require a space between the constant name and value." The current definition lacks this space between
planet_problem_keysand the array literal, which shall invoke the compiler's wrath.Apply this blessing to appease the Machine Spirit:
-#macro planet_problem_keys ["meeting_trap","meeting","succession","mech_raider","mech_bionics","mech_mars","mech_tomb1","fallen","great_crusade","harlequins","fund_elder","provide_garrison","hunt_beast","protect_raiders","join_communion","join_parade","recover_artifacts","train_forces","spyrer","inquisitor","recon","cleanse","purge","tyranid_org","artifact_loan","necron","ethereal","demon_world"] +#macro planet_problem_keys ["meeting_trap","meeting","succession","mech_raider","mech_bionics","mech_mars","mech_tomb1","fallen","great_crusade","harlequins","fund_elder","provide_garrison","hunt_beast","protect_raiders","join_communion","join_parade","recover_artifacts","train_forces","spyrer","inquisitor","recon","cleanse","purge","tyranid_org","artifact_loan","necron","ethereal","demon_world"]As per coding guidelines.
Likely an incorrect or invalid review comment.
objects/obj_star_select/Draw_64.gml (5)
45-45: Tech-Priest, the click guard logic protects the debug realm.The additional condition
!(debug && mouse_x<400)prevents click interactions when the debug panel is active in the left region. This preserves interface integrity during debug operations.
137-137: Tech-Priest, the debug mode gate preserves UI state integrity.The condition
obj_controller.menu == 0 && !debugensures the manage units button is hidden during debug operations, preventing conflicting interface states.
170-172: Tech-Priest, the planet selection gate maintains debug isolation.Wrapping
planet_selection_action()with the!debugcondition ensures planet interaction logic is suppressed during debug operations, maintaining clean separation between debug and normal flows.
272-272: Tech-Priest, the cleanup after feature destruction is proper.Adding
pop_draw_return_values()before exit ensures draw state restoration even when the instance is destroyed. This prevents state leakage into subsequent rendering.
154-154: State Restoration Verified
Tech-Priest, pop_draw_return_values() restores alpha, font, colour, halign, and valign exactly as saved by add_draw_return_values() — no action required.scripts/scr_fleet_functions/scr_fleet_functions.gml (2)
662-667: Verified: radical_inquisitor_mission_ship_arrival exists
Definition found in scripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gml at line 27; early-exit delegation is valid.
662-667: Cargo key validated. Tech-Priest, ‘radical_inquisitor’ is added via fleet_add_cargo in scr_inquisition_mission.gml:362 and correctly checked by fleet_has_cargo in scr_fleet_functions.gml:663.scripts/scr_cheatcode/scr_cheatcode.gml (1)
409-418: No issues found with debug_options initialisation. Thedebug_optionsobject is instantiated at objects/obj_star_select/Draw_64.gml:117 via new RadioSet([...]) before draw_planet_debug_options() is ever called.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml (1)
112-112: Unreachable code path detected.Tech-Priest, this branch employs assignment (
lag=2) in place of comparison (lag==2). Even were the operator corrected, line 107's defect ensures this path remains eternally untraversed.- else if (lag=2){ + else if (lag==2){scripts/scr_inquisition_mission/scr_inquisition_mission.gml (1)
13-16: Track the TODO with a proper rite.The TODO about disposition-influenced ETA warrants an issue to avoid fading into the warp. Shall I open a GitHub issue and draft an ADR?
♻️ Duplicate comments (7)
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml (2)
77-77: The ledger entry remains unscribed.Tech-Priest, this TODO endures without sanctification. The machine spirits await your decree.
101-105: The inverted sanctification persists.Tech-Priest, the previous augury remains unheeded: this barrier banishes colonists from barren worlds—those with
population <= 0—yet such lifeless spheres are the ordained destinations fornew_colonymissions. Invert the predicate to bypass only already-inhabited realms.- if (data.mission == "new_colony" && star.p_population[r]<=0){ + if (data.mission == "new_colony" && star.p_population[r] > 0){scripts/scr_inquisition_mission/scr_inquisition_mission.gml (1)
382-385: Wrong property path for inquisitor gender.Gender lives under pop_data.mission_data.inquisitor_gender.
Apply:
- var _gender = pop_data.inquisitor_gender + var _gender = pop_data.mission_data.inquisitor_gender;Also review other gender usages accordingly. Based on learnings.
scripts/scr_cheatcode/scr_cheatcode.gml (3)
464-467: Unbalanced draw-state rites on early exit.add_draw_return_values() is not paired when exiting via outside-click, risking corrupted UI state.
- if (!scr_hit([36,base_y,337,base_y+281]) && scr_click_left()) { - debug = 0; - exit; - } + if (!scr_hit([36,base_y,337,base_y+281]) && scr_click_left()) { + pop_draw_return_values(); + debug = 0; + exit; + }
486-497: Normalise keyed access; remove stray whitespace.Use the same [$key] form consistently.
- draw_text(38, _y, faction_names[i] + ": " + string(target[$ key][current_planet])); + draw_text(38, _y, faction_names[i] + ": " + string(target[$key][current_planet])); @@ - target[$ key][current_planet] = clamp(target[$key][current_planet] - 1, 0, 6); + target[$key][current_planet] = clamp(target[$key][current_planet] - 1, 0, 6); @@ - target[$ key][current_planet] = clamp(target[$key][current_planet] + 1, 0, 6); + target[$key][current_planet] = clamp(target[$key][current_planet] + 1, 0, 6);
459-466: Localise the target host to avoid scope ghosts.Relying on implicit instance scope is brittle. Bind the star explicitly.
function draw_planet_debug_forces(){ add_draw_return_values(); + var target = self.target; // obj_star_select.target var current_planet = obj_controller.selecting_planet;scripts/scr_popup_functions/scr_popup_functions.gml (1)
149-154: Tech-Priest, the debug incantations violate prior Machine Spirit directives.The code now invokes
show_debug_message(_opt)twice for callable options: once at line 149 (on any option click) and again at line 152 (before method execution). This contradicts the previous review guidance to eliminate redundant debug output. A single debug emission at line 152 (execution path) would suffice.Apply this correction to remove the redundant debug call:
if (scr_click_left()) { press = i; - show_debug_message(_opt); if (is_struct(_opt) && struct_exists(_opt, "method")){ if (is_callable(_opt.method)){ show_debug_message(_opt); script_execute(_opt.method); press = -1; } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
ChapterMaster.yyp(1 hunks)objects/obj_popup/Step_0.gml(2 hunks)objects/obj_star_select/Create_0.gml(1 hunks)objects/obj_star_select/Draw_64.gml(6 hunks)scripts/macros/macros.gml(1 hunks)scripts/scr_PlanetData/scr_PlanetData.gml(3 hunks)scripts/scr_cheatcode/scr_cheatcode.gml(1 hunks)scripts/scr_event_code/scr_event_code.gml(1 hunks)scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml(3 hunks)scripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gml(3 hunks)scripts/scr_inquisition_mission/scr_inquisition_mission.gml(14 hunks)scripts/scr_popup_functions/scr_popup_functions.gml(4 hunks)scripts/scr_string_functions/scr_string_functions.gml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.gml
⚙️ CodeRabbit configuration file
**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
- Color codes in the code shouldn't have any spaces in their id. I.e., color code
# 80bf40will crash the game.- All code should comply with the main GML documentation: https://manual.gamemaker.io/beta/en/GameMaker_Language/GML_Reference/GML_Reference.htm
Files:
scripts/scr_inquisition_mission/scr_inquisition_mission.gmlscripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gmlscripts/scr_string_functions/scr_string_functions.gmlscripts/scr_event_code/scr_event_code.gmlobjects/obj_star_select/Draw_64.gmlscripts/scr_cheatcode/scr_cheatcode.gmlobjects/obj_popup/Step_0.gmlscripts/scr_popup_functions/scr_popup_functions.gmlobjects/obj_star_select/Create_0.gmlscripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gmlscripts/macros/macros.gmlscripts/scr_PlanetData/scr_PlanetData.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:
scripts/scr_inquisition_mission/scr_inquisition_mission.gmlscripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gmlscripts/scr_string_functions/scr_string_functions.gmlscripts/scr_event_code/scr_event_code.gmlobjects/obj_star_select/Draw_64.gmlscripts/scr_cheatcode/scr_cheatcode.gmlobjects/obj_popup/Step_0.gmlscripts/scr_popup_functions/scr_popup_functions.gmlChapterMaster.yypobjects/obj_star_select/Create_0.gmlscripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gmlscripts/macros/macros.gmlscripts/scr_PlanetData/scr_PlanetData.gml
🔇 Additional comments (18)
scripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gml (1)
128-132: Tech-Priest, the corruption has been purged.The assignment operators flagged in the prior audit have been corrected to equality operators. The conditional logic now functions as intended.
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml (2)
57-73: Well-forged cargo augmentation rite.Tech-Priest, the
fleet_add_cargofunction follows sound patterns: the delegation to a specific fleet instance correctly omits the fleet parameter, allowing the recursive invocation to default to"none"and operate on the target's context. The overwrite guard prevents unintended data corruption.
79-79: Variable isolation sanctioned.Tech-Priest, severing
lagfrom the loop iterator declaration improves clarity. The rites of maintenance are observed.scripts/macros/macros.gml (1)
44-52: Gender enum and helper are sound.Enum values and set_gender() returning a choice are correct and align with new string helpers.
scripts/scr_event_code/scr_event_code.gml (1)
157-158: Delegation intact.Passing _event into hunt_inquisition_spared_inquisitor_consequence preserves needed fields (e.g., variation).
scripts/scr_string_functions/scr_string_functions.gml (1)
22-34: Third-person pronoun helper: sanctified.Lowercase consistency and termination runes present. Default to “they” is acceptable.
scripts/scr_PlanetData/scr_PlanetData.gml (1)
784-791: Guard restored; machine-spirit appeased.Early exit when obj_star_select is absent prevents null-reference daemons. Using button_manager thereafter is safe.
Also applies to: 878-894
objects/obj_popup/Step_0.gml (1)
1174-1175: Centralised popup shutdown confirmed
Tech-Priest, function popup_defualt_close() is defined in scr_popup_functions.gml, confirming the centralised cleanup path.scripts/scr_cheatcode/scr_cheatcode.gml (1)
433-457: Inputs verified:planet_problem_keysis defined as a#macroinscripts/scr_mission_functions/scr_mission_functions.gml, andscr_hit(x1, y1, x2, y2)correctly handles four scalar arguments. No further action required.objects/obj_star_select/Draw_64.gml (7)
113-115: LGTM! The modular approach aligns with the sacred principles.The extraction of debug UI rendering into
draw_planet_debug_options()improves code organization and addresses the previous concerns about repeated initialization. The Machine Spirit approves this architectural refinement.
118-118: Debug guard correctly isolates UI flows.The addition of
!debugto the menu condition properly prevents normal button interactions when debug mode is active, maintaining separation between debug and production flows.
135-135: Stack hygiene maintained on early exit path.The addition of
pop_draw_return_values()before exit correctly balances theadd_draw_return_values()invocation at line 12, ensuring proper cleanup of draw state when the manage units flow destroys the instance.
253-253: Return values properly restored on feature destruction path.The
pop_draw_return_values()call correctly matches the initialadd_draw_return_values()at line 12, maintaining draw state consistency when feature destruction triggers early exit.
151-153: Planet selection flow properly gated.The debug guard prevents
planet_selection_action()execution during debug operations, maintaining isolation between debug and standard planet interaction flows.
366-371: Shutter controls correctly hidden during debug operations.The debug guard appropriately prevents all four shutter buttons from rendering when debug mode is active, ensuring the debug UI remains uncluttered by standard action controls.
45-45: Confirm DataSlate.entered() implementation
Tech-Priest,debug_slateis properly initialised in Create_0.gml, yet noentered()method was located across GML scripts. Manually verify that theDataSlateclass defines anentered()function before relying on this guard.scripts/scr_popup_functions/scr_popup_functions.gml (2)
104-104: Sentinel initialization maintains state purity.The initialization of
pressto -1 at the function's entry establishes a clean baseline before option processing, ensuring deterministic behaviour in subsequent validation logic.
194-198: Validation guard is sound; origin of empty strings untraced
The sentinel resetspressupon detecting a non-struct empty entry, safeguarding downstream logic. Static analysis shows no literal""assigned tooptions; Tech-Priest must confirm that dynamic data does not inject empty placeholders.
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml
Outdated
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml (1)
112-112: Assignment operator contaminates second lag conditional.Tech-Priest, whilst the first lag sentinel (line 107) was purified, this second ward remains corrupted. The assignment operator (
=) still masquerades as comparison, condemning the condition to invariant truth and mutatinglagunbidden.- else if (lag=2){ + else if (lag==2){🤖 Prompt for AI Agents
In scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml at line 112, change the assignment operator (=) to the equality comparison operator (==) in the else-if condition so it reads "else if (lag==2)" to correctly test whether lag equals 2 rather than assigning the value.
♻️ Duplicate comments (2)
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml (1)
101-105: Critical: Colony guard logic remains inverted.Tech-Priest, this sentinel still bars entry to the very worlds ordained for colonisation. The
new_colonydirective seeks unpopulated realms (population <= 0), yet the current ward banishes precisely those targets. Heed the previous augury and invert the predicate.- if (data.mission == "new_colony" && star.p_population[r]<=0){ + if (data.mission == "new_colony" && star.p_population[r] > 0){ continue; }scripts/scr_inquisition_mission/scr_inquisition_mission.gml (1)
309-311: Replace the undefinedestimateglyph.Tech-Priest, the demanding branch still references
estimate, a variable no longer conjured in this rite. When executed, the Machine Spirit will raise an undefined-variable exception. Invoke the already computedetainstead so the mission briefing survives inspection.Sanctify the string thus:
- text = $"The Inquisition demands that your Chapter demonstrate its loyalty to the Imperium of Mankind and the Emperor. A radical inquisitor is enroute to {_star.name}, expected within {estimate} months. They are to be silenced and removed."; + text = $"The Inquisition demands that your Chapter demonstrate its loyalty to the Imperium of Mankind and the Emperor. A radical inquisitor is enroute to {_star.name}, expected within {eta} months. They are to be silenced and removed.";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
objects/obj_popup/Step_0.gml(2 hunks)scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml(3 hunks)scripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gml(3 hunks)scripts/scr_inquisition_mission/scr_inquisition_mission.gml(14 hunks)scripts/scr_popup_functions/scr_popup_functions.gml(4 hunks)scripts/scr_string_functions/scr_string_functions.gml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.gml
⚙️ CodeRabbit configuration file
**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
- Color codes in the code shouldn't have any spaces in their id. I.e., color code
# 80bf40will crash the game.- All code should comply with the main GML documentation: https://manual.gamemaker.io/beta/en/GameMaker_Language/GML_Reference/GML_Reference.htm
Files:
scripts/scr_string_functions/scr_string_functions.gmlobjects/obj_popup/Step_0.gmlscripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gmlscripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gmlscripts/scr_popup_functions/scr_popup_functions.gmlscripts/scr_inquisition_mission/scr_inquisition_mission.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:
scripts/scr_string_functions/scr_string_functions.gmlobjects/obj_popup/Step_0.gmlscripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gmlscripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gmlscripts/scr_popup_functions/scr_popup_functions.gmlscripts/scr_inquisition_mission/scr_inquisition_mission.gml
🔇 Additional comments (2)
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml (2)
79-79: Improved variable declaration separation.Tech-Priest, separating
lagfrom the loop iterator enhances clarity. This alteration is sanctioned.
107-107: Corrected assignment operator.Tech-Priest, the equality comparator now stands where assignment once profaned. This correction is acknowledged.
scripts/scr_imperial_manage_fleet_functions/scr_imperial_manage_fleet_functions.gml
Show resolved
Hide resolved
scripts/scr_inquisition_fleet_functions/scr_inquisition_fleet_functions.gml
Show resolved
Hide resolved
1 participant
Purpose and Description
Testing done
Related things and/or additional context