Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion objects/obj_en_fleet/Alarm_1.gml
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ try {
} else if (action_eta == 0) {
action = "";
if (array_length(complex_route) > 0) {
var target_loc = star_by_name(complex_route[0]);
var target_loc = find_star_by_name(complex_route[0]);
if (target_loc != "none") {
array_delete(complex_route, 0, 1);
action_x = target_loc.x;
Expand Down
2 changes: 1 addition & 1 deletion objects/obj_event_log/Draw_0.gml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ if (__b__) {
draw_text_ext(xx + 25, yy + 120 + (p * 26), $"{cur_event.date} (Turn {cur_event.turn}) - {cur_event.text}", -1, 1554);
if (cur_event.event_target != "none") {
if (point_and_click(draw_unit_buttons([xx + 1400, yy + 120 + (p * 26)], "View", [1, 1], c_green,, fnt_40k_14b, 1, true))) {
var view_star = star_by_name(cur_event.event_target);
var view_star = find_star_by_name(cur_event.event_target);
if (view_star != "none") {
main_map_defaults();
obj_controller.x = view_star.x;
Expand Down
2 changes: 1 addition & 1 deletion objects/obj_ncombat/Alarm_7.gml
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ try {
}
if ((string_count("ruins", battle_special) > 0) && (defeat == 1)) {
//TODO this logic is wrong assumes all player units died in ruins
var _combat_star = star_by_name(obj_ncombat.battle_loc);
var _combat_star = find_star_by_name(obj_ncombat.battle_loc);
if (_combat_star != "none") {
_combat_star.p_player[obj_ncombat.battle_id] -= obj_ncombat.world_size;
}
Expand Down
4 changes: 2 additions & 2 deletions objects/obj_p_fleet/Draw_0.gml
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ if (action != "") {
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 = star_by_name(complex_route[i]);
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]);
Comment on lines +110 to +114
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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;
+        }
     }
 }

}
}
}
Expand Down
4 changes: 2 additions & 2 deletions objects/obj_popup/Step_0.gml
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ try {
var mission_star, onceh;
onceh = 0;
obj_controller.temp[200] = string(loc);
var mission_star = star_by_name(obj_controller.temp[200]);
var mission_star = find_star_by_name(obj_controller.temp[200]);
if (add_new_problem(planet, "recon", estimate, mission_star)) {
title = "Inquisition Mission Demand";
text = $"The Inquisition demands that your Chapter demonstrate its loyalty to the Imperium of Mankind and the Emperor. {global.chapter_name} are to land Astartes on {mission_star.name} {scr_roman(planet)} to investigate the planet within {estimate} months.";
Expand All @@ -243,7 +243,7 @@ try {
var mission_star, onceh;
mission_star = 0;
onceh = 0;
var mission_star = star_by_name(obj_controller.temp[200]);
var mission_star = find_star_by_name(obj_controller.temp[200]);
var mission_is_go = false;
if (mission_star != "none" && planet > 0) {
var _estimate = estimate;
Expand Down
2 changes: 1 addition & 1 deletion objects/obj_turn_end/Alarm_0.gml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ try {
ii = 0;
good = 0;

var battle_star = star_by_name(battle_location[current_battle]);
var battle_star = find_star_by_name(battle_location[current_battle]);

if (battle_star != "none") {
// trying to find the star
Expand Down
2 changes: 1 addition & 1 deletion objects/obj_turn_end/Alarm_4.gml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ instance_activate_object(obj_star);

if ((battles > 0) && (current_battle <= battles)) {
var ii = 0, good = 0;
var battle_star = star_by_name(battle_location[current_battle]);
var battle_star = find_star_by_name(battle_location[current_battle]);
obj_controller.temp[1060] = battle_location[current_battle];

if (battle_star != "none") {
Expand Down
6 changes: 3 additions & 3 deletions scripts/scr_ancient_ruins/scr_ancient_ruins.gml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function scr_ruins_suprise_attack_player() {
instance_activate_object(obj_star_select);
instance_activate_object(obj_star);
instance_activate_object(obj_ground_mission);
var _star = star_by_name(obj_ground_mission.loc);
var _star = find_star_by_name(obj_ground_mission.loc);
var _planet = obj_ground_mission.num;
Comment on lines +28 to 29
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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

var _units = obj_ground_mission.display_unit;

Expand Down Expand Up @@ -360,7 +360,7 @@ function ruins_exploration_main_sequence() {
} else {
var obj = obj_ground_mission.obj;
instance_activate_object(obj_star);
scr_ruins_reward(star_by_name(obj_ground_mission.battle_loc), obj_ground_mission.num, obj_ground_mission.explore_feature);
scr_ruins_reward(find_star_by_name(obj_ground_mission.battle_loc), obj_ground_mission.num, obj_ground_mission.explore_feature);
instance_destroy();
exit;
}
Expand Down Expand Up @@ -394,7 +394,7 @@ function scr_ruins_combat_end() {
var _star = 0;
ruins_battle = choose(6, 7, 9, 10, 11, 12);

_star = star_by_name(obj_ground_mission.battle_loc);
_star = find_star_by_name(obj_ground_mission.battle_loc);
var planet = obj_ground_mission.num;
var _battle_threat = obj_ground_mission.battle_threat;
if (obj_ground_mission.defeat == 0) {
Expand Down
4 changes: 2 additions & 2 deletions scripts/scr_company_struct/scr_company_struct.gml
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ function CompanyStruct(comp) constructor {
}
}
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));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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);
+    }
 }

}
bound_height[0] += 180;
} else {
Expand Down Expand Up @@ -285,7 +285,7 @@ function CompanyStruct(comp) constructor {
if (cur_assignment.type == "garrison") {
var garrison_but = draw_unit_buttons([cancel_but[2] + 10, cancel_but[1]], "View Garrison", [1, 1], c_red,,,, true);
if (point_and_click(garrison_but)) {
var garrrison_star = star_by_name(cur_assignment.location);
var garrrison_star = find_star_by_name(cur_assignment.location);
obj_controller.view_squad = false;
if (garrrison_star != "none") {
Comment on lines +288 to 290
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
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;
}

scr_toggle_manage();
Expand Down
4 changes: 2 additions & 2 deletions scripts/scr_dialogue/scr_dialogue.gml
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ function scr_dialogue(diplo_keyphrase, data = {}) {
if (!_found) {
var _master = fetch_unit([0, 0]);
if (_master.planet_location > 0) {
var _master_star = star_by_name(_master.location_string);
var _master_star = find_star_by_name(_master.location_string);
if (_master_star != "none") {
_found = true;
_planet = _master.planet_location;
Expand Down Expand Up @@ -2183,7 +2183,7 @@ function scr_dialogue(diplo_keyphrase, data = {}) {
var _event = audience_data;
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.";
Comment on lines 2184 to 2187
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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)}";


add_diplomacy_option({
Expand Down
4 changes: 2 additions & 2 deletions scripts/scr_event_code/scr_event_code.gml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function event_end_turn_action() {
// Removes planetary governor installed by the chapter
if (_event.e_id == "remove_surf") {
var _star_name = _event.system;
var _event_star = star_by_name(_event.system);
var _event_star = find_star_by_name(_event.system);
var _planet = _event.planet;
if (_event_star != "none") {
_event_star.dispo[_planet] = -10; // Resets
Expand Down Expand Up @@ -356,7 +356,7 @@ function strange_build_event() {

var marine_is_planetside = _unit.planet_location > 0;
if (marine_is_planetside && heritical_item) {
var _system = star_by_name(_unit.location_string);
var _system = find_star_by_name(_unit.location_string);
var _planet = _unit.planet_location;
if (_system != "none") {
with (_system) {
Expand Down
2 changes: 1 addition & 1 deletion scripts/scr_fleet_functions/scr_fleet_functions.gml
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function set_fleet_movement(fastest_route = true, new_action = "move", minimum_e
var star_travel = new FastestRouteAlgorithm(x, y, action_x, action_y, self.id, is_orbiting());
var path = star_travel.final_array_path();
if (array_length(path) > 1) {
var targ = star_by_name(path[1]);
var targ = find_star_by_name(path[1]);
if (targ != "none") {
array_delete(path, 0, 2);
complex_route = path;
Expand Down
2 changes: 1 addition & 1 deletion scripts/scr_ini_ship_cleanup/scr_ini_ship_cleanup.gml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function scr_kill_ship(index) {
var _available_ships = [];
var _ship_fleet = find_ships_fleet(index);
if (!in_warp) {
var _nearest_star = star_by_name(ship_location[index]);
var _nearest_star = find_star_by_name(ship_location[index]);
}
if (_ship_fleet != "none") {
delete_ship_from_fleet(index, _ship_fleet);
Expand Down
10 changes: 5 additions & 5 deletions scripts/scr_inquisition_mission/scr_inquisition_mission.gml
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ function mission_inquisition_tomb_world(tomb_worlds) {

/// @mixin
function init_mission_inquisition_tomb_world() {
mission_star = star_by_name(pop_data.system);
mission_star = find_star_by_name(pop_data.system);
if (mission_star == "none") {
popup_default_close();
exit;
Expand Down Expand Up @@ -349,7 +349,7 @@ function add_new_inquis_mission() {
}

function init_mission_hunt_inquisitor() {
mission_star = star_by_name(pop_data.system);
mission_star = find_star_by_name(pop_data.system);
if (mission_star == "none") {
popup_default_close();
exit;
Expand Down Expand Up @@ -714,7 +714,7 @@ function setup_necron_tomb_raid(planet) {

/// @mixin obj_popup
function necron_tomb_mission_start() {
mission_star = star_by_name(pop_data.loc);
mission_star = find_star_by_name(pop_data.loc);
planet = pop_data.planet;
Comment on lines +717 to 718
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Warning: Unguarded access detected in the tomb mission liturgy, Tech-Priest.

The variable mission_star receives the result of find_star_by_name(pop_data.loc) without verification against "none". On line 733, mission_star.p_player[planet] is accessed, which shall cause corruption should the star not be found.

🛡️ Proposed defensive implementation
 function necron_tomb_mission_start() {
     mission_star = find_star_by_name(pop_data.loc);
+    if (mission_star == "none") {
+        instance_destroy();
+        exit;
+    }
     planet = pop_data.planet;
📝 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.

Suggested change
mission_star = find_star_by_name(pop_data.loc);
planet = pop_data.planet;
mission_star = find_star_by_name(pop_data.loc);
if (mission_star == "none") {
instance_destroy();
exit;
}
planet = pop_data.planet;


title = $"Necron Tunnels : {pop_data.mission_stage}";
Expand Down Expand Up @@ -771,7 +771,7 @@ function necron_tomb_mission_sequence() {

alter_disposition(eFACTION.INQUISITION, obj_controller.demanding ? choose(0, 0, 1) : 1);

mission_star = star_by_name(pop_data.loc);
mission_star = find_star_by_name(pop_data.loc);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unshielded invocation within the tomb sequence, Tech-Priest.

The mission_star is reassigned via find_star_by_name(pop_data.loc) without verification. The subsequent calls to remove_planet_problem, seal_tomb_world, and property accesses on lines 775-780 shall fail catastrophically if the star resolves to void.

🛡️ Proposed sanctification
             mission_star = find_star_by_name(pop_data.loc);
+            if (mission_star == "none") {
+                reset_popup_options();
+                exit;
+            }
             remove_planet_problem(planet, "necron", mission_star);

remove_planet_problem(planet, "necron", mission_star);
seal_tomb_world(mission_star.p_feature[planet]);
// mission_star.p_feature[planet][search_planet_features(mission_star.p_feature[planet], eP_FEATURES.NECRON_TOMB)[0]].sealed = 1;
Expand Down Expand Up @@ -829,7 +829,7 @@ function necron_tomb_mission_sequence() {
}
delete _roster;

mission_star = star_by_name(pop_data.loc);
mission_star = find_star_by_name(pop_data.loc);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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;
}


obj_ncombat.battle_object = mission_star;
instance_deactivate_object(obj_star);
Expand Down
6 changes: 3 additions & 3 deletions scripts/scr_marine_struct/scr_marine_struct.gml
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,7 @@ function TTRPG_stats(faction, comp, mar, class = "marine", other_spawn_data = {}
obj_ini.ship_carrying[ship] += size; //update ship capacity

if (star == "none") {
star = star_by_name(system);
star = find_star_by_name(system);
}
if (star != "none") {
if (star.p_player[current_location[1]] > 0) {
Expand Down Expand Up @@ -1779,9 +1779,9 @@ function TTRPG_stats(faction, comp, mar, class = "marine", other_spawn_data = {}
var homestar = "none";
var spawn_location_chosen = false;
if (((type == "home") || (type == "default")) && (obj_ini.fleet_type == ePLAYER_BASE.HOME_WORLD)) {
var homestar = star_by_name(obj_ini.home_name);
var homestar = find_star_by_name(obj_ini.home_name);
} else if (type != "ship") {
var homestar = star_by_name(type);
var homestar = find_star_by_name(type);
}
/* if (!spawn_location_chosen){

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function cancel_fleet_movement() {

function set_new_player_fleet_course(target_array) {
if (array_length(target_array) > 0) {
var target_planet = star_by_name(target_array[0]);
var target_planet = find_star_by_name(target_array[0]);
var nearest_planet = instance_nearest(x, y, obj_star);
var from_star = point_distance(nearest_planet.x, nearest_planet.y, x, y) < 75;
var valid = target_planet != "none";
Expand All @@ -80,7 +80,7 @@ function set_new_player_fleet_course(target_array) {
}
if (!valid) {
if (array_length(target_array) > 1) {
target_planet = star_by_name(target_array[1]);
target_planet = find_star_by_name(target_array[1]);
array_delete(target_array, 0, 2);
} else {
return "complex_route_finish";
Expand Down Expand Up @@ -282,7 +282,7 @@ function player_retreat_from_fleet_combat() {

_roll_100 = roll_dice_chapter(1, 100, "low");

var _loc_star = 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]);

obj_controller.temp[2001] = real(_loc_star.id);
obj_controller.temp[2002] = real(obj_turn_end.battle_opponent[obj_turn_end.current_battle]);
Comment on lines +285 to 288
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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]);

Expand Down
2 changes: 1 addition & 1 deletion scripts/scr_post_battle_events/scr_post_battle_events.gml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function necron_tomb_raid_post_battle_sequence() {
}
}

var _star_obj = star_by_name(battle_loc);
var _star_obj = find_star_by_name(battle_loc);
if (_star_obj != "none") {
with (_star_obj) {
var planet = obj_ncombat.battle_id;
Expand Down
18 changes: 5 additions & 13 deletions scripts/scr_system_search_helpers/scr_system_search_helpers.gml
Original file line number Diff line number Diff line change
Expand Up @@ -205,17 +205,11 @@ function planet_imperium_ground_total(planet_check) {
return p_guardsmen[planet_check] + p_pdf[planet_check] + p_sisters[planet_check] + p_player[planet_check];
}

/// @function star_by_name(search_name)
/// @description
/// Searches all `obj_star` instances and returns the one with a matching name.
///
/// @param {String} search_name
/// The name of the star to find.
///
/// @returns {Instance | String}
/// Returns the `obj_star` instance that matches `search_name`,
/// or the string `"none"` if no matching star is found.
function star_by_name(search_name) {
/// @function find_star_by_name(search_name)
/// @description Searches all `obj_star` instances and returns the one with a matching name.
/// @param {String} search_name The name of the star to find.
/// @returns {Id.Instance.obj_star | String} Returns the `obj_star` instance that matches `search_name`, or the string `"none"` if no matching star is found.
function find_star_by_name(search_name) {
if (!instance_exists(obj_star)) {
assert_error_popup("Not a single instance of obj_star exists!");
return "none";
Comment thread
EttyKitty marked this conversation as resolved.
Expand All @@ -227,8 +221,6 @@ function star_by_name(search_name) {
}
}

assert_error_popup($"Star {search_name} wasn't found!");

return "none";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ function find_player_spawn_star() {

/*if (obj_ini.recruiting_name!="random"){
array_push(global.name_generator.star_used_names, obj_ini.recruiting_name);
if (star_by_name(obj_ini.recruiting_name) != "none" ){
star_by_name(obj_ini.recruiting_name).name = global.name_generator.generate_star_name();
if (find_star_by_name(obj_ini.recruiting_name) != "none" ){
find_star_by_name(obj_ini.recruiting_name).name = global.name_generator.generate_star_name();
}
name=obj_ini.recruiting_name;
}*/
Expand All @@ -68,8 +68,8 @@ function player_home_star(home_planet) {

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();
}
Comment on lines 69 to 73
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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

name = obj_ini.home_name;
}
Expand Down Expand Up @@ -125,16 +125,16 @@ function set_player_recruit_planet(recruit_planet) {
var recruit_name = obj_ini.recruiting_name;
if (recruit_name != "random") {
array_push(global.name_generator.star_used_names, recruit_name);
if (star_by_name(recruit_name) != "none") {
star_by_name(recruit_name).name = global.name_generator.generate_star_name();
if (find_star_by_name(recruit_name) != "none") {
find_star_by_name(recruit_name).name = global.name_generator.generate_star_name();
}
name = recruit_name;
}
} else {
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();
}
name = obj_ini.home_name;
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/scr_ui_manage/scr_ui_manage.gml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// @mixin
function load_marines_into_ship(system, ship, units, reload = false) {
static _load_into_ship = function(system, ship, units, size, loop, reload) {
var load_from_star = star_by_name(system);
var load_from_star = find_star_by_name(system);
if (is_struct(units[loop])) {
units[loop].load_marine(sh_ide[ship], load_from_star);
ma_loc[loop] = sh_loc[ship];
Expand Down
Loading