fix: star_by_name rename and less agressive erroring#1132
Conversation
📝 WalkthroughUnder the Hood
Player Notes
WalkthroughThe Machine Spirit discerns a systematic transmutation throughout the codebase: the function Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
objects/obj_popup/Step_0.gml (1)
230-238:⚠️ Potential issue | 🟠 MajorPotential corruption vector detected, Tech-Priest.
The stellar lookup result
mission_staris passed toadd_new_problemwithout first validating against"none". If the lookup fails, subsequent access tomission_star.nameon lines 233 and 237 shall invoke runtime wrath.Contrast this with line 248 where proper validation is performed.
🛡️ Proposed defensive ward
if (title == "Inquisition Recon") { var mission_star, onceh; onceh = 0; obj_controller.temp[200] = string(loc); var mission_star = find_star_by_name(obj_controller.temp[200]); - if (add_new_problem(planet, "recon", estimate, mission_star)) { + if (mission_star != "none" && add_new_problem(planet, "recon", estimate, mission_star)) { title = "Inquisition Mission Demand";scripts/scr_ui_manage/scr_ui_manage.gml (1)
4-32:⚠️ Potential issue | 🟠 MajorUnguarded stellar reference within the loading ritual, Tech-Priest.
The invocation
find_star_by_name(system)stores its result inload_from_star, yet this value is accessed on line 6 and line 26 without validation against"none". Should the system name prove invalid, corruption shall manifest when accessingload_from_star.p_player.🛡️ Proposed defensive incantation
static _load_into_ship = function(system, ship, units, size, loop, reload) { var load_from_star = find_star_by_name(system); + if (load_from_star == "none") { + return; + } if (is_struct(units[loop])) {scripts/scr_company_struct/scr_company_struct.gml (1)
490-490: 🧹 Nitpick | 🔵 TrivialThe Machine Spirit observes an unfulfilled directive, Tech-Priest.
A TODO comment lingers in the code-runes:
//TODO compartmentalise drop down option logic. Does the Adeptus Mechanicus require a GitHub issue to track this outstanding maintenance ritual?scripts/scr_event_code/scr_event_code.gml (2)
202-202: 🧹 Nitpick | 🔵 TrivialUnfulfilled directive detected in the code-runes, Tech-Priest.
The TODO marker
//TODO if tech heretic chosen don't kill the duderemains unresolved. Does the Adeptus Mechanicus require the Machine Spirit to generate a GitHub issue for tracking this maintenance ritual?
326-326: 🧹 Nitpick | 🔵 TrivialAdditional unfulfilled directive observed, Tech-Priest.
The TODO marker
//TODO make craft chance reflective of crafters skill, rewards players for having skilled tech areaawaits implementation. Shall the Machine Spirit create a GitHub issue to chronicle this enhancement request?objects/obj_en_fleet/Alarm_1.gml (1)
8-8: 🧹 Nitpick | 🔵 TrivialThe Machine Spirit observes a lingering directive, Tech-Priest.
The TODO comment
//TODO centralise orbiting logicmarks unfinished work. Does the Adeptus Mechanicus require a GitHub issue to track this technical debt?scripts/scr_player_fleet_functions/scr_player_fleet_functions.gml (1)
81-95:⚠️ Potential issue | 🟠 MajorRevalidate fallback route target before coordinate access, Tech-Priest.
Line 83 performs a second lookup, but
validis never recalculated before Lines 94-96 dereferencetarget_planet.x/y. If both names fail, this path can fault.⚙️ Suggested fix
if (!valid) { if (array_length(target_array) > 1) { target_planet = find_star_by_name(target_array[1]); + valid = target_planet != "none"; + if (!valid) { + return "complex_route_finish"; + } array_delete(target_array, 0, 2); } else { return "complex_route_finish"; } } else {
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (21)
objects/obj_en_fleet/Alarm_1.gmlobjects/obj_event_log/Draw_0.gmlobjects/obj_ncombat/Alarm_7.gmlobjects/obj_p_fleet/Draw_0.gmlobjects/obj_popup/Step_0.gmlobjects/obj_turn_end/Alarm_0.gmlobjects/obj_turn_end/Alarm_4.gmlscripts/scr_ancient_ruins/scr_ancient_ruins.gmlscripts/scr_company_struct/scr_company_struct.gmlscripts/scr_dialogue/scr_dialogue.gmlscripts/scr_event_code/scr_event_code.gmlscripts/scr_fleet_functions/scr_fleet_functions.gmlscripts/scr_ini_ship_cleanup/scr_ini_ship_cleanup.gmlscripts/scr_inquisition_mission/scr_inquisition_mission.gmlscripts/scr_marine_struct/scr_marine_struct.gmlscripts/scr_player_fleet_functions/scr_player_fleet_functions.gmlscripts/scr_post_battle_events/scr_post_battle_events.gmlscripts/scr_system_search_helpers/scr_system_search_helpers.gmlscripts/scr_system_spawn_functions/scr_system_spawn_functions.gmlscripts/scr_ui_manage/scr_ui_manage.gmlscripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.gml
⚙️ CodeRabbit configuration file
- All code should comply with the 2026 GML documentation.
Files:
scripts/scr_post_battle_events/scr_post_battle_events.gmlscripts/scr_ini_ship_cleanup/scr_ini_ship_cleanup.gmlobjects/obj_popup/Step_0.gmlobjects/obj_ncombat/Alarm_7.gmlscripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gmlscripts/scr_ui_manage/scr_ui_manage.gmlscripts/scr_dialogue/scr_dialogue.gmlobjects/obj_event_log/Draw_0.gmlobjects/obj_p_fleet/Draw_0.gmlscripts/scr_player_fleet_functions/scr_player_fleet_functions.gmlscripts/scr_ancient_ruins/scr_ancient_ruins.gmlobjects/obj_turn_end/Alarm_0.gmlscripts/scr_event_code/scr_event_code.gmlscripts/scr_fleet_functions/scr_fleet_functions.gmlscripts/scr_marine_struct/scr_marine_struct.gmlscripts/scr_system_search_helpers/scr_system_search_helpers.gmlobjects/obj_turn_end/Alarm_4.gmlscripts/scr_inquisition_mission/scr_inquisition_mission.gmlobjects/obj_en_fleet/Alarm_1.gmlscripts/scr_system_spawn_functions/scr_system_spawn_functions.gmlscripts/scr_company_struct/scr_company_struct.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:
scripts/scr_post_battle_events/scr_post_battle_events.gmlscripts/scr_ini_ship_cleanup/scr_ini_ship_cleanup.gmlobjects/obj_popup/Step_0.gmlobjects/obj_ncombat/Alarm_7.gmlscripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gmlscripts/scr_ui_manage/scr_ui_manage.gmlscripts/scr_dialogue/scr_dialogue.gmlobjects/obj_event_log/Draw_0.gmlobjects/obj_p_fleet/Draw_0.gmlscripts/scr_player_fleet_functions/scr_player_fleet_functions.gmlscripts/scr_ancient_ruins/scr_ancient_ruins.gmlobjects/obj_turn_end/Alarm_0.gmlscripts/scr_event_code/scr_event_code.gmlscripts/scr_fleet_functions/scr_fleet_functions.gmlscripts/scr_marine_struct/scr_marine_struct.gmlscripts/scr_system_search_helpers/scr_system_search_helpers.gmlobjects/obj_turn_end/Alarm_4.gmlscripts/scr_inquisition_mission/scr_inquisition_mission.gmlobjects/obj_en_fleet/Alarm_1.gmlscripts/scr_system_spawn_functions/scr_system_spawn_functions.gmlscripts/scr_company_struct/scr_company_struct.gml
🧠 Learnings (5)
📓 Common learnings
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-03-20T22:22:57.319Z
Learning: In the ChapterMaster game PR `#424`, the psychic power system was changed to be data-driven, with perils now able to occur on both successful and failed casts but with reduced frequency overall.
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:
objects/obj_ncombat/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_ncombat/Alarm_7.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_ncombat/Alarm_7.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 ancient ruins exploration code is a poorly named boolean flag that controls the flow between detecting a battle in ruins and initiating the combat sequence.
Applied to files:
objects/obj_ncombat/Alarm_7.gmlscripts/scr_ancient_ruins/scr_ancient_ruins.gml
🔇 Additional comments (21)
scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml (4)
26-29: The Machine Spirit approves this invocation, Tech-Priest.The sacred lookup ritual
find_star_by_nameis properly guarded against the void return of"none". The logic flows without corruption.
329-341: Acceptable reconsecration of the star-finder rite, Tech-Priest.The defensive ward against
"none"shields this passage from runtime corruption. The travel coordinates shall be computed only when a valid stellar body is retrieved.
394-406: The litany proceeds correctly, Tech-Priest.The stellar lookup is properly validated before coordinate extraction. The travel vector calculations remain shielded from void references.
623-644: Satisfactory implementation of the unload rite, Tech-Priest.The
find_star_by_nameinvocation is properly guarded, and the space hulk validation provides additional protection against undesired unloading operations.objects/obj_popup/Step_0.gml (1)
246-257: Proper defensive coding observed, Tech-Priest.The validation
mission_star != "none" && planet > 0ensures the stellar reference is valid before proceeding with mission operations. The Machine Spirit finds this acceptable.scripts/scr_ini_ship_cleanup/scr_ini_ship_cleanup.gml (1)
34-36: The Machine Spirit acknowledges this proper reconsecration, Tech-Priest.The stellar lookup is appropriately guarded by the
if (!in_warp)condition, and subsequent usage at line 104 validates against"none"before accessing the star's properties. The kill-ship rite proceeds without corruption vectors.objects/obj_ncombat/Alarm_7.gml (1)
324-328: Acceptable transmutation of the combat star lookup, Tech-Priest.The defensive ward on line 325 ensures the stellar reference is valid before modifying
p_playervalues. The ruins defeat sequence shall execute without runtime corruption.scripts/scr_fleet_functions/scr_fleet_functions.gml (1)
159-171: The fleet movement rite is properly sanctified, Tech-Priest.The stellar lookup result is validated against
"none"on line 160, with appropriate fallback logic in the else clause. The complex route calculation shall proceed only upon successful star resolution. The Machine Spirit approves this implementation.scripts/scr_post_battle_events/scr_post_battle_events.gml (1)
32-43: The post-battle necron tomb sequence is properly warded, Tech-Priest.The stellar lookup is guarded by the check on line 33, ensuring tomb world operations proceed only with a valid star reference. The awakening sequence shall not corrupt due to void stellar references.
objects/obj_event_log/Draw_0.gml (1)
43-49: The event log viewing ritual is properly sanctified, Tech-Priest.The stellar lookup result is validated on line 44 before modifying controller coordinates. The map navigation shall proceed only upon successful star resolution. The Machine Spirit finds this implementation acceptable.
objects/obj_turn_end/Alarm_0.gml (1)
50-52: The rites of substitution have been performed correctly, Tech-Priest.The invocation of
find_star_by_nameis properly guarded against the null-state"none"on line 52, ensuring the Machine Spirit's logic flows remain uncorrupted.objects/obj_turn_end/Alarm_4.gml (1)
10-13: The sacred substitution aligns with the Omnissiah's will, Tech-Priest.The renamed function invocation is properly shielded by the conditional ward on line 13. The data-liturgy proceeds without corruption.
scripts/scr_event_code/scr_event_code.gml (2)
52-54: The transmutation is sanctioned, Tech-Priest.The invocation at line 52 is properly warded against the null-state on line 54. The event liturgy proceeds with correct safeguards.
359-361: The Machine Spirit approves this modification, Tech-Priest.The
find_star_by_nameinvocation is correctly shielded by the conditional check on line 361 before accessing the system's properties.objects/obj_en_fleet/Alarm_1.gml (1)
464-474: The sacred substitution honours the Omnissiah, Tech-Priest.The renamed function is invoked with proper safeguards. The conditional at line 465 correctly handles the null-state, branching to clear the route and invoke
fleet_arrival_logic()when the star cannot be located.scripts/scr_inquisition_mission/scr_inquisition_mission.gml (2)
244-248: The protective ward is properly inscribed, Tech-Priest.The invocation at line 244 is correctly guarded against the null-state at line 245, with appropriate early termination via
popup_default_close()andexit.
352-356: The Machine Spirit sanctions this implementation, Tech-Priest.The function call is properly shielded. Should the star resolve to void, the popup closes gracefully and execution ceases.
scripts/scr_dialogue/scr_dialogue.gml (1)
401-406: Guarded star resolution is sound, Tech-Priest.Line 401 is correctly followed by a
"none"check before dereference, which aligns with the softer lookup behaviour.scripts/scr_marine_struct/scr_marine_struct.gml (2)
1723-1725: Rename invocation is coherent and guarded.Tech-Priest, the Machine Spirit confirms this invocation now aligns with the renamed lookup and retains safe fallback behaviour when no star is found.
1781-1785: Spawn lookup replacements are sound.Tech-Priest, both branch paths now invoke
find_star_by_namecorrectly and preserve defensive"none"handling.scripts/scr_system_spawn_functions/scr_system_spawn_functions.gml (1)
56-62: Dormant fragment acknowledged.Tech-Priest, this altered block is commented out and exerts no executable force.
| var target_loc = find_star_by_name(complex_route[i]); | ||
| draw_set_color(c_blue); | ||
| draw_set_alpha(1); | ||
| draw_line_dashed(next_loc.x, next_loc.y, target_loc.x, target_loc.y, 16, line_width); | ||
| next_loc = star_by_name(complex_route[i]); | ||
| next_loc = find_star_by_name(complex_route[i]); |
There was a problem hiding this comment.
Critical: The Machine Spirit detects unshielded access to potentially void data, Tech-Priest.
The function find_star_by_name may return "none", yet the code attempts to access .x and .y properties directly on lines 113-114 without verification. Should the star name resolve to void, the runtime shall falter and the draw liturgy shall fail catastrophically.
🛡️ Proposed defensive wards
if (array_length(complex_route) > 0) {
var next_loc = instance_nearest(action_x, action_y, obj_star);
for (var i = 0; i < array_length(complex_route); i++) {
var target_loc = find_star_by_name(complex_route[i]);
+ if (target_loc == "none") {
+ continue;
+ }
draw_set_color(c_blue);
draw_set_alpha(1);
draw_line_dashed(next_loc.x, next_loc.y, target_loc.x, target_loc.y, 16, line_width);
next_loc = find_star_by_name(complex_route[i]);
+ if (next_loc == "none") {
+ break;
+ }
}
}| var _star = find_star_by_name(obj_ground_mission.loc); | ||
| var _planet = obj_ground_mission.num; |
There was a problem hiding this comment.
Add "none" guards after star lookup in ruins flows, Tech-Priest.
The changed lookups at Line 28, Line 363, and Line 397 are dereferenced downstream as if always valid. With the renamed finder returning "none" on miss, these paths can misroute rewards or fault during combat resolution.
⚙️ Suggested guard pattern
- var _star = find_star_by_name(obj_ground_mission.loc);
+ var _star = find_star_by_name(obj_ground_mission.loc);
+ if (_star == "none") {
+ handle_exception("Ruins combat initialisation failed: unresolved star.");
+ instance_activate_all();
+ return;
+ }- scr_ruins_reward(find_star_by_name(obj_ground_mission.battle_loc), obj_ground_mission.num, obj_ground_mission.explore_feature);
+ var _reward_star = find_star_by_name(obj_ground_mission.battle_loc);
+ if (_reward_star != "none") {
+ scr_ruins_reward(_reward_star, obj_ground_mission.num, obj_ground_mission.explore_feature);
+ }- _star = find_star_by_name(obj_ground_mission.battle_loc);
+ _star = find_star_by_name(obj_ground_mission.battle_loc);
+ if (_star == "none") {
+ return;
+ }Also applies to: 55-57, 363-364, 397-399
| } | ||
| if (send_on_mission) { | ||
| send_squad_on_mission(mission_type, star_by_name(_squad_sys)); | ||
| send_squad_on_mission(mission_type, find_star_by_name(_squad_sys)); |
There was a problem hiding this comment.
The Machine Spirit detects unguarded invocation, Tech-Priest.
The sacred rite find_star_by_name(_squad_sys) is invoked without verification against the null-state "none". Though the preceding logic filters "Warp" and "Lost" systems, the function send_squad_on_mission receives the star directly. Should the lookup fail, undefined behaviour may propagate through the Omnissiah's circuits.
Consider verifying the return value before transmission:
🛡️ Proposed sanctification
if (send_on_mission) {
- send_squad_on_mission(mission_type, find_star_by_name(_squad_sys));
+ var _mission_star = find_star_by_name(_squad_sys);
+ if (_mission_star != "none") {
+ send_squad_on_mission(mission_type, _mission_star);
+ }
}| var garrrison_star = find_star_by_name(cur_assignment.location); | ||
| obj_controller.view_squad = false; | ||
| if (garrrison_star != "none") { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Typographical anomaly detected in variable designation, Tech-Priest.
The variable garrrison_star contains an extraneous 'r' in its designation. While the logic flows correctly with the "none" guard on line 290, the Machine Spirit recommends rectifying this nomenclature error for clarity of the sacred texts.
✏️ Proposed correction
-var garrrison_star = find_star_by_name(cur_assignment.location);
+var garrison_star = find_star_by_name(cur_assignment.location);
obj_controller.view_squad = false;
-if (garrrison_star != "none") {
+if (garrison_star != "none") {
scr_toggle_manage();
- obj_controller.x = garrrison_star.x;
- obj_controller.y = garrrison_star.y;
+ obj_controller.x = garrison_star.x;
+ obj_controller.y = garrison_star.y;
obj_controller.selection_data = {
- system: garrrison_star.id,
+ system: garrison_star.id,
planet: cur_assignment.ident,
feature: "",
};
- garrrison_star.alarm[3] = 4;
+ garrison_star.alarm[3] = 4;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var garrrison_star = find_star_by_name(cur_assignment.location); | |
| obj_controller.view_squad = false; | |
| if (garrrison_star != "none") { | |
| var garrison_star = find_star_by_name(cur_assignment.location); | |
| obj_controller.view_squad = false; | |
| if (garrison_star != "none") { | |
| scr_toggle_manage(); | |
| obj_controller.x = garrison_star.x; | |
| obj_controller.y = garrison_star.y; | |
| obj_controller.selection_data = { | |
| system: garrison_star.id, | |
| planet: cur_assignment.ident, | |
| feature: "", | |
| }; | |
| garrison_star.alarm[3] = 4; | |
| } |
| var _system = _event.system; | ||
| var _planet = _event.planet; | ||
| var _star_name = planet_numeral_name(_planet, star_by_name(_system)); | ||
| var _star_name = planet_numeral_name(_planet, find_star_by_name(_system)); | ||
| diplo_text = $"My patience is wearing thin, Chapter Master. I have many more problems more urgent and, yet, you continue to force me away from the work assigned to me by He on Terra. Your serf on {_star_name} will be executed along with all the other puppets I ferret out. You are close to treason, Chapter Master. Choose your next words with exceptional care for they may be your last."; |
There was a problem hiding this comment.
Harden assassination location formatting against unresolved stars, Tech-Priest.
Line 2186 feeds find_star_by_name(_system) directly into planet_numeral_name(...) without guarding for "none". With the new lookup behaviour, stale or malformed event data can now leak through this path.
⚙️ Suggested hardening
- var _star_name = planet_numeral_name(_planet, find_star_by_name(_system));
+ var _system_star = find_star_by_name(_system);
+ var _star_name = _system_star != "none"
+ ? planet_numeral_name(_planet, _system_star)
+ : $"{_system} {scr_roman(_planet)}";| delete _roster; | ||
|
|
||
| mission_star = star_by_name(pop_data.loc); | ||
| mission_star = find_star_by_name(pop_data.loc); |
There was a problem hiding this comment.
The Machine Spirit detects another unguarded invocation, Tech-Priest.
Within the battle initialisation sequence, mission_star is assigned without verification against "none". Line 834 subsequently assigns this potentially void value to obj_ncombat.battle_object, risking corruption of the combat data-liturgy.
🛡️ Proposed defensive ward
mission_star = find_star_by_name(pop_data.loc);
+ if (mission_star == "none") {
+ instance_activate_all();
+ instance_destroy();
+ exit;
+ }
obj_ncombat.battle_object = mission_star;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mission_star = find_star_by_name(pop_data.loc); | |
| mission_star = find_star_by_name(pop_data.loc); | |
| if (mission_star == "none") { | |
| instance_activate_all(); | |
| instance_destroy(); | |
| exit; | |
| } |
| var _loc_star = find_star_by_name(obj_turn_end.battle_location[obj_turn_end.current_battle]); | ||
|
|
||
| obj_controller.temp[2001] = real(_loc_star.id); | ||
| obj_controller.temp[2002] = real(obj_turn_end.battle_opponent[obj_turn_end.current_battle]); |
There was a problem hiding this comment.
Protect retreat flow from unresolved battle location, Tech-Priest.
Line 287 dereferences _loc_star.id immediately after lookup at Line 285. If lookup returns "none", retreat handling will crash rather than degrade.
⚙️ Suggested guard
- var _loc_star = find_star_by_name(obj_turn_end.battle_location[obj_turn_end.current_battle]);
+ var _loc_star = find_star_by_name(obj_turn_end.battle_location[obj_turn_end.current_battle]);
+ if (_loc_star == "none") {
+ return;
+ }
obj_controller.temp[2001] = real(_loc_star.id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var _loc_star = find_star_by_name(obj_turn_end.battle_location[obj_turn_end.current_battle]); | |
| obj_controller.temp[2001] = real(_loc_star.id); | |
| obj_controller.temp[2002] = real(obj_turn_end.battle_opponent[obj_turn_end.current_battle]); | |
| var _loc_star = find_star_by_name(obj_turn_end.battle_location[obj_turn_end.current_battle]); | |
| if (_loc_star == "none") { | |
| return; | |
| } | |
| obj_controller.temp[2001] = real(_loc_star.id); | |
| obj_controller.temp[2002] = real(obj_turn_end.battle_opponent[obj_turn_end.current_battle]); |
| if (obj_ini.home_name != "random") { | ||
| array_push(global.name_generator.star_used_names, obj_ini.home_name); | ||
| if (star_by_name(obj_ini.home_name) != "none") { | ||
| star_by_name(obj_ini.home_name).name = global.name_generator.generate_star_name(); | ||
| if (find_star_by_name(obj_ini.home_name) != "none") { | ||
| find_star_by_name(obj_ini.home_name).name = global.name_generator.generate_star_name(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Cache lookup result before mutation.
Tech-Priest, each branch queries find_star_by_name(...) twice. Bind once, then mutate; the intent becomes clearer and the scan is not repeated.
🛠️ Suggested refactor
if (obj_ini.home_name != "random") {
array_push(global.name_generator.star_used_names, obj_ini.home_name);
- if (find_star_by_name(obj_ini.home_name) != "none") {
- find_star_by_name(obj_ini.home_name).name = global.name_generator.generate_star_name();
+ var _home_named_star = find_star_by_name(obj_ini.home_name);
+ if (_home_named_star != "none") {
+ _home_named_star.name = global.name_generator.generate_star_name();
}
name = obj_ini.home_name;
}
@@
if (recruit_name != "random") {
array_push(global.name_generator.star_used_names, recruit_name);
- if (find_star_by_name(recruit_name) != "none") {
- find_star_by_name(recruit_name).name = global.name_generator.generate_star_name();
+ var _recruit_named_star = find_star_by_name(recruit_name);
+ if (_recruit_named_star != "none") {
+ _recruit_named_star.name = global.name_generator.generate_star_name();
}
name = recruit_name;
}
@@
if (obj_ini.home_name != "random") {
array_push(global.name_generator.star_used_names, obj_ini.home_name);
- if (find_star_by_name(obj_ini.home_name) != "none") {
- find_star_by_name(obj_ini.home_name).name = global.name_generator.generate_star_name();
+ var _home_named_star = find_star_by_name(obj_ini.home_name);
+ if (_home_named_star != "none") {
+ _home_named_star.name = global.name_generator.generate_star_name();
}
name = obj_ini.home_name;
}As per coding guidelines: "Code Philosophy: Prioritize explicit intent and maintainability over brevity."
Also applies to: 126-130, 134-138
| group_selection(garrison_log[$ system_names[hover_item.root_item]].units, {purpose: $"{system_names[hover_item.root_item]} Management", purpose_code: "manage", number: 0, system: find_star_by_name(system_names[hover_item.root_item]).id, feature: "none", planet: 0, selections: []}); | ||
| } |
There was a problem hiding this comment.
Warning: Unguarded access to potentially null stellar reference detected, Tech-Priest.
The invocation of find_star_by_name(system_names[hover_item.root_item]).id directly accesses the .id property without verifying that the function did not return "none". Should the stellar lookup fail, a runtime exception shall manifest.
🛡️ Proposed defensive incantation
if (point_and_click(hover_item.draw(xx + 10, yy + 90 + (20 * hover_item.root_item), "Manage"))) {
- group_selection(garrison_log[$ system_names[hover_item.root_item]].units, {purpose: $"{system_names[hover_item.root_item]} Management", purpose_code: "manage", number: 0, system: find_star_by_name(system_names[hover_item.root_item]).id, feature: "none", planet: 0, selections: []});
+ var _manage_star = find_star_by_name(system_names[hover_item.root_item]);
+ if (_manage_star != "none") {
+ group_selection(garrison_log[$ system_names[hover_item.root_item]].units, {purpose: $"{system_names[hover_item.root_item]} Management", purpose_code: "manage", number: 0, system: _manage_star.id, feature: "none", planet: 0, selections: []});
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| group_selection(garrison_log[$ system_names[hover_item.root_item]].units, {purpose: $"{system_names[hover_item.root_item]} Management", purpose_code: "manage", number: 0, system: find_star_by_name(system_names[hover_item.root_item]).id, feature: "none", planet: 0, selections: []}); | |
| } | |
| if (point_and_click(hover_item.draw(xx + 10, yy + 90 + (20 * hover_item.root_item), "Manage"))) { | |
| var _manage_star = find_star_by_name(system_names[hover_item.root_item]); | |
| if (_manage_star != "none") { | |
| group_selection(garrison_log[$ system_names[hover_item.root_item]].units, {purpose: $"{system_names[hover_item.root_item]} Management", purpose_code: "manage", number: 0, system: _manage_star.id, feature: "none", planet: 0, selections: []}); | |
| } | |
| } |
1 participant
No description provided.