feat: Imperial Guard Auxilia#1271
Conversation
There was a problem hiding this comment.
19 issues found and verified against the latest diff
Confidence score: 2/5
- The highest risk is save/load regression in
scripts/scr_fleet_advisor/scr_fleet_advisor.gmlandobjects/obj_ini/Create_0.gml: older or partial saves can hit missing arrays/entries (ship_guardsmen,name[_coy]) and crash during load. Add defensive existence/type checks plus a save-migration/default-initialization path before merging. scripts/scr_add_man/scr_add_man.gmlcan leave_unituninitialized for Guard roles whenother_gearis false, which can produce invalid unit creation flow or runtime errors in player-facing recruitment. Ensure Guard paths always construct_unit(or fail explicitly) regardless of theother_gearbranch before merge.objects/obj_star_select/Draw_64.gmlappears to expose Deploy Guard without the normal ownership/war/fleet constraints, anddeploy_guardsmen()does not re-validate, so players may trigger actions in invalid states. Re-apply the standard gating in the UI and enforce the same checks server-side/in the action function before merging.- State/render reliability still needs hardening:
scripts/scr_PlanetData/scr_PlanetData.gmlcan serve stale corruption values after population resets, whilescripts/scr_draw_unit_image/scr_draw_unit_image.gmlperformssprite_addin Draw and can get stuck after a-1load failure. Recompute cached corruption after zero-pop branches and move sprite loading to an init/cache path with retry/logging on failure before merge.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/scr_cheatcode/scr_cheatcode.gml">
<violation number="1" location="scripts/scr_cheatcode/scr_cheatcode.gml:108">
P2: Custom agent: **Code Quality Review**
Raw string identifier 'home_planet' used as a constant more than once without being defined as a #macro or enum.</violation>
</file>
<file name="objects/obj_pnunit/Create_0.gml">
<violation number="1" location="objects/obj_pnunit/Create_0.gml:12">
P2: Custom agent: **Code Quality Review**
Addition of an obsolete/dead variable `guard` increases maintenance burden by preserving legacy state for branches that never execute.</violation>
</file>
<file name="scripts/scr_draw_unit_image/scr_draw_unit_image.gml">
<violation number="1" location="scripts/scr_draw_unit_image/scr_draw_unit_image.gml:528">
P2: Custom agent: **Code Quality Review**
Heavy allocation (sprite_add) inside a Draw event, causing a performance hitch on first call.</violation>
<violation number="2" location="scripts/scr_draw_unit_image/scr_draw_unit_image.gml:534">
P2: Custom agent: **Code Quality Review**
Missing fail-loud pattern for runtime sprite_add failure. If the mod-provided PNG is missing or the working_directory path is wrong, the code silently skips drawing with no log output, despite LOGGER being available.</violation>
<violation number="3" location="scripts/scr_draw_unit_image/scr_draw_unit_image.gml:535">
P2: `variable_global_exists` guards `sprite_add` reload but never recovers from a failed load. If `sprite_add` returns `-1` (documented failure value), the global variable still exists, so subsequent calls skip `sprite_add` forever, leaving the portrait permanently missing. Also affects the Guard Sergeant block below.</violation>
</file>
<file name="scripts/scr_roster/scr_roster.gml">
<violation number="1" location="scripts/scr_roster/scr_roster.gml:116">
P2: Custom agent: **Code Quality Review**
Raw string literals ('Guardsman', 'Guard Squad', 'Guard Sergeant', 'guardsman') are used as repeated identifiers/constants without #macro or enum definitions.</violation>
<violation number="2" location="scripts/scr_roster/scr_roster.gml:116">
P2: Custom agent: **Code Quality Review**
Repeated guard-role string-literal checks are not abstracted despite an `auxilia_roles()` helper being added in the same PR. Use `array_contains(auxilia_roles(), _role)` (or add an `is_auxilia_role()` wrapper) to eliminate the duplicated inline list in `update_roster`, `determine_full_roster`, and `add_unit_to_battle`.</violation>
<violation number="3" location="scripts/scr_roster/scr_roster.gml:298">
P1: Guardsman filter button creation is gated by `_unit.squad == "none"`, which may miss guard-role units that have no squad type but do have a squad id.</violation>
</file>
<file name="scripts/scr_add_vehicle/scr_add_vehicle.gml">
<violation number="1" location="scripts/scr_add_vehicle/scr_add_vehicle.gml:126">
P2: Custom agent: **Code Quality Review**
Added another near-identical vehicle-type weapon assignment block to an already duplicated if-chain (Rhino, Whirlwind, Land Speeder, etc.). These repeated blocks differ only in string literals and should be abstracted into a helper function, lookup struct/ds_map, or data table rather than adding more if-blocks.</violation>
</file>
<file name="scripts/scr_add_man/scr_add_man.gml">
<violation number="1" location="scripts/scr_add_man/scr_add_man.gml:32">
P1: Guard roles can bypass both unit-construction paths when `other_gear` is false, leaving `_unit` uninitialized. The Guard role cases are inside `if (other_gear == true)`, but because they are also in `non_marine_roles`, the marine `else` path is skipped too. The code then unconditionally calls `_unit.add_exp`, `_unit.allocate_unit_to_fresh_spawn`, and `_unit.update_role` on whatever `fetch_unit` returned. Consider initializing Guard roles unconditionally or restructuring the guard so that a role in `non_marine_roles` always receives a valid `_unit`.</violation>
<violation number="2" location="scripts/scr_add_man/scr_add_man.gml:32">
P2: Custom agent: **Code Quality Review**
Three new switch cases for Guard roles are near-identical and not abstracted. The only difference between 'Guardsman', 'Guard Squad', and 'Guard Sergeant' is the role string passed to TTRPG_stats. This violates the code quality rule against repeated code fragments that should be abstracted.</violation>
</file>
<file name="objects/obj_ini/Create_0.gml">
<violation number="1" location="objects/obj_ini/Create_0.gml:339">
P1: Load path may crash on older/incomplete save data because `array_length` is called on potentially missing `name[_coy]` entries.</violation>
</file>
<file name="scripts/scr_load_all/scr_load_all.gml">
<violation number="1" location="scripts/scr_load_all/scr_load_all.gml:11">
P2: Custom agent: **Code Quality Review**
Added code exceeds the maximum of 3 nested control structures (if/else, for/while, try/catch, switch). The new block contains 4 nested structures: `if (select_units)` → `if (selecting_location == "")` → `for` → `if`, reducing readability and maintainability.</violation>
</file>
<file name="objects/obj_star_select/Draw_64.gml">
<violation number="1" location="objects/obj_star_select/Draw_64.gml:308">
P1: Deploy Guard button may bypass normal action gating—no ownership, war state, or fleet presence checks before offering the action, and deploy_guardsmen() does not independently enforce those constraints.</violation>
<violation number="2" location="objects/obj_star_select/Draw_64.gml:382">
P2: deploy_guardsmen can return 0 on failure/no-op, but the caller unconditionally shows a success popup using the returned value without validation.</violation>
</file>
<file name="scripts/scr_post_battle_events/scr_post_battle_events.gml">
<violation number="1" location="scripts/scr_post_battle_events/scr_post_battle_events.gml:191">
P2: Identical hulk teardown logic is duplicated across `space_hulk_strip` and `space_hulk_surrender`. Extract a shared helper to keep cleanup synchronized.</violation>
</file>
<file name="scripts/scr_PlanetData/scr_PlanetData.gml">
<violation number="1" location="scripts/scr_PlanetData/scr_PlanetData.gml:81">
P1: Cached `secret_corruption` and `corruption` fields can become stale when `population == 0`. The assignments were moved before the population-zero reset branch, and the end-of-function re-assignment was removed, so the local cached fields retain pre-reset values while the authoritative system arrays are zeroed.</violation>
</file>
<file name="objects/obj_star/Create_0.gml">
<violation number="1" location="objects/obj_star/Create_0.gml:87">
P2: Broad try/catch swallows all errors from _gar.update() and silently replaces the cached GarrisonForce, masking real runtime defects.</violation>
</file>
<file name="scripts/scr_fleet_advisor/scr_fleet_advisor.gml">
<violation number="1" location="scripts/scr_fleet_advisor/scr_fleet_advisor.gml:186">
P1: Custom agent: **Save Compatibility Review**
Accessing the new `ship_guardsmen` array without verifying its existence breaks compatibility with old saves. GameMaker's `game_load` does not re-run the Create event, so saves created before this field existed will lack `ship_guardsmen` on `obj_ini`, causing a runtime error when the fleet advisor iterates over ships. Add an existence check (e.g., `variable_instance_exists(obj_ini, "ship_guardsmen") && ...`) before indexing.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 40 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
| location.contents = obj_ini.ship_location[i]; | ||
| hp.contents = $"{round(obj_ini.ship_hp[i] / obj_ini.ship_maxhp[i] * 100)}%"; | ||
| carrying.contents = $"{obj_ini.ship_carrying[i]}/{obj_ini.ship_capacity[i]}"; | ||
| if (obj_ini.ship_guardsmen[i] > 0) { |
There was a problem hiding this comment.
P1: Custom agent: Save Compatibility Review
Accessing the new ship_guardsmen array without verifying its existence breaks compatibility with old saves. GameMaker's game_load does not re-run the Create event, so saves created before this field existed will lack ship_guardsmen on obj_ini, causing a runtime error when the fleet advisor iterates over ships. Add an existence check (e.g., variable_instance_exists(obj_ini, "ship_guardsmen") && ...) before indexing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_fleet_advisor/scr_fleet_advisor.gml, line 186:
<comment>Accessing the new `ship_guardsmen` array without verifying its existence breaks compatibility with old saves. GameMaker's `game_load` does not re-run the Create event, so saves created before this field existed will lack `ship_guardsmen` on `obj_ini`, causing a runtime error when the fleet advisor iterates over ships. Add an existence check (e.g., `variable_instance_exists(obj_ini, "ship_guardsmen") && ...`) before indexing.</comment>
<file context>
@@ -183,6 +183,9 @@ function scr_fleet_advisor() {
location.contents = obj_ini.ship_location[i];
hp.contents = $"{round(obj_ini.ship_hp[i] / obj_ini.ship_maxhp[i] * 100)}%";
carrying.contents = $"{obj_ini.ship_carrying[i]}/{obj_ini.ship_capacity[i]}";
+ if (obj_ini.ship_guardsmen[i] > 0) {
+ carrying.contents += $" +{scr_display_number(obj_ini.ship_guardsmen[i])} IG";
+ }
</file context>
| main_data_slate.draw(344, 160, slate_draw_scale, slate_draw_scale + 0.1); | ||
| } | ||
| // Deploy Guard auxilia: offer the 4th slot when guard-carrying ships orbit this world. | ||
| if ((button4 == "") && (obj_controller.selecting_planet > 0) && (player_guardsmen_at(target.name) > 0)) { |
There was a problem hiding this comment.
P1: Deploy Guard button may bypass normal action gating—no ownership, war state, or fleet presence checks before offering the action, and deploy_guardsmen() does not independently enforce those constraints.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_star_select/Draw_64.gml, line 308:
<comment>Deploy Guard button may bypass normal action gating—no ownership, war state, or fleet presence checks before offering the action, and deploy_guardsmen() does not independently enforce those constraints.</comment>
<file context>
@@ -304,6 +304,10 @@ try {
main_data_slate.draw(344, 160, slate_draw_scale, slate_draw_scale + 0.1);
}
+ // Deploy Guard auxilia: offer the 4th slot when guard-carrying ships orbit this world.
+ if ((button4 == "") && (obj_controller.selecting_planet > 0) && (player_guardsmen_at(target.name) > 0)) {
+ button4 = "Deploy Guard";
+ }
</file context>
| // TODO: Implement logic for Auxiliary Soldier (Race 2.5, Renegade stats) | ||
|
|
||
| switch (man_role) { | ||
| case "Guardsman": |
There was a problem hiding this comment.
P1: Guard roles can bypass both unit-construction paths when other_gear is false, leaving _unit uninitialized. The Guard role cases are inside if (other_gear == true), but because they are also in non_marine_roles, the marine else path is skipped too. The code then unconditionally calls _unit.add_exp, _unit.allocate_unit_to_fresh_spawn, and _unit.update_role on whatever fetch_unit returned. Consider initializing Guard roles unconditionally or restructuring the guard so that a role in non_marine_roles always receives a valid _unit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_add_man/scr_add_man.gml, line 32:
<comment>Guard roles can bypass both unit-construction paths when `other_gear` is false, leaving `_unit` uninitialized. The Guard role cases are inside `if (other_gear == true)`, but because they are also in `non_marine_roles`, the marine `else` path is skipped too. The code then unconditionally calls `_unit.add_exp`, `_unit.allocate_unit_to_fresh_spawn`, and `_unit.update_role` on whatever `fetch_unit` returned. Consider initializing Guard roles unconditionally or restructuring the guard so that a role in `non_marine_roles` always receives a valid `_unit`.</comment>
<file context>
@@ -26,6 +29,21 @@ function scr_add_man(man_role, target_company, spawn_exp, spawn_name, corruption
// TODO: Implement logic for Auxiliary Soldier (Race 2.5, Renegade stats)
switch (man_role) {
+ case "Guardsman":
+ spawn_exp = 10;
+ obj_ini.race[target_company][_company_slot] = eFACTION.IMPERIUM;
</file context>
| obj_ini.TTRPG = array_create(11); | ||
| for (var _coy = 0; _coy < 11; _coy++) { | ||
| for (var _mar = 0; _mar <= 500; _mar++) { | ||
| var _coy_len = max(501, array_length(obj_ini.name[_coy])); |
There was a problem hiding this comment.
P1: Load path may crash on older/incomplete save data because array_length is called on potentially missing name[_coy] entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_ini/Create_0.gml, line 339:
<comment>Load path may crash on older/incomplete save data because `array_length` is called on potentially missing `name[_coy]` entries.</comment>
<file context>
@@ -324,9 +329,16 @@ deserialize = function(save_data) {
+ obj_ini.TTRPG = array_create(11);
for (var _coy = 0; _coy < 11; _coy++) {
- for (var _mar = 0; _mar <= 500; _mar++) {
+ var _coy_len = max(501, array_length(obj_ini.name[_coy]));
+ obj_ini.TTRPG[_coy] = array_create(_coy_len);
+ for (var _mar = 0; _mar < _coy_len; _mar++) {
</file context>
| var _coy_len = max(501, array_length(obj_ini.name[_coy])); | |
| var _coy_len = 501; | |
| if (array_length(obj_ini.name) > _coy) { | |
| _coy_len = max(501, array_length(obj_ini.name[_coy])); | |
| } |
| new_squad_button("Dreadnought", "dreadnought"); | ||
| } | ||
| } | ||
| // Guardsmen and Guard Squads have no squad type, so give them their |
There was a problem hiding this comment.
P1: Guardsman filter button creation is gated by _unit.squad == "none", which may miss guard-role units that have no squad type but do have a squad id.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_roster/scr_roster.gml, line 298:
<comment>Guardsman filter button creation is gated by `_unit.squad == "none"`, which may miss guard-role units that have no squad type but do have a squad id.</comment>
<file context>
@@ -290,6 +295,13 @@ function Roster() constructor {
new_squad_button("Dreadnought", "dreadnought");
}
}
+ // Guardsmen and Guard Squads have no squad type, so give them their
+ // own filter button (added once) so they can be selected on their own.
+ var _grd_role = _unit.role();
</file context>
| var _n = deploy_guardsmen(target.name, obj_controller.selecting_planet); | ||
| scr_popup("Imperial Guard", "Deployed " + string(_n) + " Guard onto " + planet_numeral_name(obj_controller.selecting_planet, target) + ".", ""); |
There was a problem hiding this comment.
P2: deploy_guardsmen can return 0 on failure/no-op, but the caller unconditionally shows a success popup using the returned value without validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_star_select/Draw_64.gml, line 382:
<comment>deploy_guardsmen can return 0 on failure/no-op, but the caller unconditionally shows a success popup using the returned value without validation.</comment>
<file context>
@@ -374,6 +378,9 @@ try {
}
}
+ } else if (current_button == "Deploy Guard") {
+ var _n = deploy_guardsmen(target.name, obj_controller.selecting_planet);
+ scr_popup("Imperial Guard", "Deployed " + string(_n) + " Guard onto " + planet_numeral_name(obj_controller.selecting_planet, target) + ".", "");
} else if (current_button == "+Recruiting") {
</file context>
| var _n = deploy_guardsmen(target.name, obj_controller.selecting_planet); | |
| scr_popup("Imperial Guard", "Deployed " + string(_n) + " Guard onto " + planet_numeral_name(obj_controller.selecting_planet, target) + ".", ""); | |
| var _n = deploy_guardsmen(target.name, obj_controller.selecting_planet); | |
| if (_n > 0) { | |
| scr_popup("Imperial Guard", "Deployed " + string(_n) + " Guard onto " + planet_numeral_name(obj_controller.selecting_planet, target) + ".", ""); | |
| } |
| // mod cannot add compiled sprite assets. The image ships at images\units\guardsman.png. | ||
| // If alignment needs nudging, offset the draw from x_surface_offset / y_surface_offset. | ||
| if (role() == "Guardsman") { | ||
| if (!variable_global_exists("guardsman_portrait")) { |
There was a problem hiding this comment.
P2: variable_global_exists guards sprite_add reload but never recovers from a failed load. If sprite_add returns -1 (documented failure value), the global variable still exists, so subsequent calls skip sprite_add forever, leaving the portrait permanently missing. Also affects the Guard Sergeant block below.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_draw_unit_image/scr_draw_unit_image.gml, line 535:
<comment>`variable_global_exists` guards `sprite_add` reload but never recovers from a failed load. If `sprite_add` returns `-1` (documented failure value), the global variable still exists, so subsequent calls skip `sprite_add` forever, leaving the portrait permanently missing. Also affects the Guard Sergeant block below.</comment>
<file context>
@@ -524,6 +524,32 @@ function scr_draw_unit_image(_background = false) {
+ // mod cannot add compiled sprite assets. The image ships at images\units\guardsman.png.
+ // If alignment needs nudging, offset the draw from x_surface_offset / y_surface_offset.
+ if (role() == "Guardsman") {
+ if (!variable_global_exists("guardsman_portrait")) {
+ global.guardsman_portrait = sprite_add(working_directory + "/images/units/guardsman.png", 1, false, false, 0, 0);
+ }
</file context>
| obj_controller.requisition += _reqi; | ||
| scr_add_artifact("random", "random", 4, hulk_loot_loc, hulk_loot_ship + 500); | ||
|
|
||
| if (instance_exists(hulk_star)) { |
There was a problem hiding this comment.
P2: Identical hulk teardown logic is duplicated across space_hulk_strip and space_hulk_surrender. Extract a shared helper to keep cleanup synchronized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_post_battle_events/scr_post_battle_events.gml, line 191:
<comment>Identical hulk teardown logic is duplicated across `space_hulk_strip` and `space_hulk_surrender`. Extract a shared helper to keep cleanup synchronized.</comment>
<file context>
@@ -150,4 +150,89 @@ function space_hulk_explore_battle_aftermath() {
+ obj_controller.requisition += _reqi;
+ scr_add_artifact("random", "random", 4, hulk_loot_loc, hulk_loot_ship + 500);
+
+ if (instance_exists(hulk_star)) {
+ with (hulk_star) {
+ instance_destroy();
</file context>
| try { | ||
| _gar.update(); | ||
| } catch (_garrison_reload_error) { | ||
| // A garrison restored from a save keeps its data but loses its struct | ||
| // methods, so rebuild it from the planet's operatives, which persist. | ||
| system_garrison[planet] = new GarrisonForce(self, planet); | ||
| _gar = system_garrison[planet]; | ||
| _gar.star = self; | ||
| _gar.planet = planet; | ||
| } |
There was a problem hiding this comment.
P2: Broad try/catch swallows all errors from _gar.update() and silently replaces the cached GarrisonForce, masking real runtime defects.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_star/Create_0.gml, line 87:
<comment>Broad try/catch swallows all errors from _gar.update() and silently replaces the cached GarrisonForce, masking real runtime defects.</comment>
<file context>
@@ -84,7 +84,16 @@ get_garrison = function(planet){
_gar.planet = planet;
} else {
- _gar.update();
+ try {
+ _gar.update();
+ } catch (_garrison_reload_error) {
</file context>
| try { | |
| _gar.update(); | |
| } catch (_garrison_reload_error) { | |
| // A garrison restored from a save keeps its data but loses its struct | |
| // methods, so rebuild it from the planet's operatives, which persist. | |
| system_garrison[planet] = new GarrisonForce(self, planet); | |
| _gar = system_garrison[planet]; | |
| _gar.star = self; | |
| _gar.planet = planet; | |
| } | |
| if (is_method(_gar.update)) { | |
| _gar.update(); | |
| } else { | |
| // A garrison restored from a save keeps its data but loses its struct | |
| // methods, so rebuild it from the planet's operatives, which persist. | |
| system_garrison[planet] = new GarrisonForce(self, planet); | |
| _gar = system_garrison[planet]; | |
| _gar.star = self; | |
| _gar.planet = planet; | |
| } |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 23 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked by 23 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/scr_trade/scr_trade.gml">
<violation number="1" location="scripts/scr_trade/scr_trade.gml:107">
P2: Custom agent: **Code Quality Review**
Magic number 200 replaces a self-documenting derived constant expression. The literal 200 appears twice in related code (Chimera count and disposition cost) and should use a named constant or the derived expression to maintain readability and coupling.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // not the transient new_vehicles staging company, so they stay grouped with the | ||
| // levy and persist after the battle instead of being reorganised out of the | ||
| // build-staging slot. | ||
| repeat (floor(_opt.number / 200)) { |
There was a problem hiding this comment.
P2: Custom agent: Code Quality Review
Magic number 200 replaces a self-documenting derived constant expression. The literal 200 appears twice in related code (Chimera count and disposition cost) and should use a named constant or the derived expression to maintain readability and coupling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_trade/scr_trade.gml, line 107:
<comment>Magic number 200 replaces a self-documenting derived constant expression. The literal 200 appears twice in related code (Chimera count and disposition cost) and should use a named constant or the derived expression to maintain readability and coupling.</comment>
<file context>
@@ -99,12 +99,12 @@ function TradeAttempt(diplomacy) constructor {
+ // not the transient new_vehicles staging company, so they stay grouped with the
+ // levy and persist after the battle instead of being reorganised out of the
+ // build-staging slot.
+ repeat (floor(_opt.number / 200)) {
scr_add_vehicle("Chimera", 0);
}
</file context>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 24 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="objects/obj_p_fleet/Alarm_1.gml">
<violation number="1" location="objects/obj_p_fleet/Alarm_1.gml:43">
P2: Clamping the movement divisor with `max(1, action_eta)` masks non-positive ETA values. If `action_eta` enters this block at 0 or negative, it is decremented to a negative number and the arrival checks (`action_eta == 0`) will never run, leaving the fleet stuck in movement state with `action` uncleared.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| spid = point_distance(x, y, action_x, action_y); | ||
| spid = spid / action_eta; | ||
| spid = spid / max(1, action_eta); |
There was a problem hiding this comment.
P2: Clamping the movement divisor with max(1, action_eta) masks non-positive ETA values. If action_eta enters this block at 0 or negative, it is decremented to a negative number and the arrival checks (action_eta == 0) will never run, leaving the fleet stuck in movement state with action uncleared.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_p_fleet/Alarm_1.gml, line 43:
<comment>Clamping the movement divisor with `max(1, action_eta)` masks non-positive ETA values. If `action_eta` enters this block at 0 or negative, it is decremented to a negative number and the arrival checks (`action_eta == 0`) will never run, leaving the fleet stuck in movement state with `action` uncleared.</comment>
<file context>
@@ -40,7 +40,7 @@ try {
spid = point_distance(x, y, action_x, action_y);
- spid = spid / action_eta;
+ spid = spid / max(1, action_eta);
dir = point_direction(x, y, action_x, action_y);
</file context>
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="objects/obj_ncombat/Draw_0.gml">
<violation number="1" location="objects/obj_ncombat/Draw_0.gml:56">
P1: Unconditional use of potentially uninitialized `log_history`, `log_view_lines`, and `log_scroll` in Draw_0. These variables are not set in the object's Create event, and the only other usage (`scr_newtext`) explicitly checks `variable_instance_exists` before touching them. Missing initialization can crash the Draw event on combat paths that bypass the new log script.</violation>
</file>
<file name="scripts/scr_newtext/scr_newtext.gml">
<violation number="1" location="scripts/scr_newtext/scr_newtext.gml:27">
P2: Fragile partial existence check: `variable_instance_exists` guards only `log_history`, but the block then unconditionally reads/writes `log_history_max`, `log_scroll`, and `log_view_lines`. If an instance has `log_history` without the other scrollback fields, GameMaker will throw a runtime unknown-variable error.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 27 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
8 issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/macros/macros.gml">
<violation number="1" location="scripts/macros/macros.gml:122">
P2: New eROLE.BIKER and eROLE.ATTACK_BIKER enum values are not registered in existing role initialization structures. `obj_controller/Create_0.gml`'s `_roles_data` struct and `scr_initialize_custom.gml`'s `load_default_gear` calls only cover roles up to VETERANSERGEANT, so the new roles will have undefined default names, gear, and weapons.</violation>
</file>
<file name="objects/obj_ncombat/Step_0.gml">
<violation number="1" location="objects/obj_ncombat/Step_0.gml:143">
P1: The newly-added `COMBAT_STAGE_TIMEOUT_FRAMES` anti-hang timeout can never trigger when the message queue fails to drain because an earlier block resets `fugg` to 0 and unconditionally `exit`s every 60 frames.</violation>
</file>
<file name="scripts/scr_shoot/scr_shoot.gml">
<violation number="1" location="scripts/scr_shoot/scr_shoot.gml:297">
P2: Custom agent: **Code Quality Review**
Duplicate armor-piercing lookup tables `_inf_ap` and `_veh_ap` are defined identically in both `scr_shoot` and `scr_shoot_spread`. Per the code quality rule, repeated code fragments (2+ near-identical lines/blocks) should be abstracted into a shared function or helper. Extract a helper function (e.g. `get_ap_multiplier(armour_pierce, is_vehicle)`) to return the correct multiplier and eliminate the duplicated definitions.</violation>
<violation number="2" location="scripts/scr_shoot/scr_shoot.gml:307">
P1: Initial retarget guard only checks `dudes_num <= 0` but not `dudes_hp <= 0`, creating a divide-by-zero / invalid damage math risk for zombie ranks. The helper `find_next_alive_rank` explicitly guards `dudes_hp > 0` to keep callers safe from dividing by rank HP, but `scr_shoot` bypasses that protection here.</violation>
<violation number="3" location="scripts/scr_shoot/scr_shoot.gml:525">
P1: `scr_shoot_spread` filters ranks by `dudes_num > 0` but not by `dudes_hp > 0`, allowing division by zero (or negative HP) in the kill calculation and inflating `_total` with dead ranks that dilute proportional shot allocation. This contradicts the alive-rank semantics established by `find_next_alive_rank` in the same file, which explicitly skips `dudes_hp <= 0`.</violation>
</file>
<file name="scripts/scr_player_combat_weapon_stacks/scr_player_combat_weapon_stacks.gml">
<violation number="1" location="scripts/scr_player_combat_weapon_stacks/scr_player_combat_weapon_stacks.gml:298">
P2: Custom agent: **Code Quality Review**
Repeated code block pattern with existing Hammer of Wrath, primary ranged, and primary melee handling — not abstracted into a shared helper. The same find_stack_index → add_data_to_stack → if head_role → player_head_role_stack sequence appears four times in this function with only the weapon data varying. Extract a helper that performs this sequence to eliminate duplication.</violation>
<violation number="2" location="scripts/scr_player_combat_weapon_stacks/scr_player_combat_weapon_stacks.gml:298">
P2: Custom agent: **Code Quality Review**
Raw string `"bike"` used as a tag identifier without being defined as a `#macro` or `enum`. The string already appears in at least two other files (`scr_initialize_custom.gml`, `scr_company_struct.gml`), violating the rule that raw identifier strings used more than once must be centralized.</violation>
</file>
<file name="scripts/scr_powers/scr_powers.gml">
<violation number="1" location="scripts/scr_powers/scr_powers.gml:373">
P2: Custom agent: **Code Quality Review**
Raw number constant `134` used as a battle-log color/type identifier should be replaced with a `#macro`. The same value already exists on the legacy call three dozen lines above, so the PR introduces a second usage that qualifies under the "more than once" threshold.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| continue; | ||
| } | ||
| for (var r = 1; r <= 30; r++) { | ||
| if (_f.dudes[r] == "" || _f.dudes_num[r] <= 0) { |
There was a problem hiding this comment.
P1: scr_shoot_spread filters ranks by dudes_num > 0 but not by dudes_hp > 0, allowing division by zero (or negative HP) in the kill calculation and inflating _total with dead ranks that dilute proportional shot allocation. This contradicts the alive-rank semantics established by find_next_alive_rank in the same file, which explicitly skips dudes_hp <= 0.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_shoot/scr_shoot.gml, line 525:
<comment>`scr_shoot_spread` filters ranks by `dudes_num > 0` but not by `dudes_hp > 0`, allowing division by zero (or negative HP) in the kill calculation and inflating `_total` with dead ranks that dilute proportional shot allocation. This contradicts the alive-rank semantics established by `find_next_alive_rank` in the same file, which explicitly skips `dudes_hp <= 0`.</comment>
<file context>
@@ -275,280 +267,314 @@ function scr_shoot(weapon_index_position, target_object, target_type, damage_dat
+ continue;
+ }
+ for (var r = 1; r <= 30; r++) {
+ if (_f.dudes[r] == "" || _f.dudes_num[r] <= 0) {
+ continue;
+ }
</file context>
| if (!instance_exists(target_object)) { | ||
| exit; | ||
| } | ||
| if (target_object.dudes_num[target_type] <= 0) { |
There was a problem hiding this comment.
P1: Initial retarget guard only checks dudes_num <= 0 but not dudes_hp <= 0, creating a divide-by-zero / invalid damage math risk for zombie ranks. The helper find_next_alive_rank explicitly guards dudes_hp > 0 to keep callers safe from dividing by rank HP, but scr_shoot bypasses that protection here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_shoot/scr_shoot.gml, line 307:
<comment>Initial retarget guard only checks `dudes_num <= 0` but not `dudes_hp <= 0`, creating a divide-by-zero / invalid damage math risk for zombie ranks. The helper `find_next_alive_rank` explicitly guards `dudes_hp > 0` to keep callers safe from dividing by rank HP, but `scr_shoot` bypasses that protection here.</comment>
<file context>
@@ -275,280 +267,314 @@ function scr_shoot(weapon_index_position, target_object, target_type, damage_dat
+ if (!instance_exists(target_object)) {
+ exit;
+ }
+ if (target_object.dudes_num[target_type] <= 0) {
+ var _alive_rank = find_next_alive_rank(target_object, -1);
+ if (_alive_rank == -1) {
</file context>
| if (target_object.dudes_num[target_type] <= 0) { | |
| if (target_object.dudes_num[target_type] <= 0 || target_object.dudes_hp[target_type] <= 0) { |
| LIBRARIAN = 17, | ||
| SERGEANT = 18, | ||
| VETERANSERGEANT = 19, | ||
| ATTACK_BIKER = 20, |
There was a problem hiding this comment.
P2: New eROLE.BIKER and eROLE.ATTACK_BIKER enum values are not registered in existing role initialization structures. obj_controller/Create_0.gml's _roles_data struct and scr_initialize_custom.gml's load_default_gear calls only cover roles up to VETERANSERGEANT, so the new roles will have undefined default names, gear, and weapons.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/macros/macros.gml, line 122:
<comment>New eROLE.BIKER and eROLE.ATTACK_BIKER enum values are not registered in existing role initialization structures. `obj_controller/Create_0.gml`'s `_roles_data` struct and `scr_initialize_custom.gml`'s `load_default_gear` calls only cover roles up to VETERANSERGEANT, so the new roles will have undefined default names, gear, and weapons.</comment>
<file context>
@@ -104,12 +112,14 @@ enum eROLE {
LIBRARIAN = 17,
SERGEANT = 18,
VETERANSERGEANT = 19,
+ ATTACK_BIKER = 20,
LANDRAIDER = 50,
RHINO = 51,
</file context>
| } | ||
| } | ||
| } | ||
| if (is_struct(mobi_item) && mobi_item.has_tag("bike")) { |
There was a problem hiding this comment.
P2: Custom agent: Code Quality Review
Repeated code block pattern with existing Hammer of Wrath, primary ranged, and primary melee handling — not abstracted into a shared helper. The same find_stack_index → add_data_to_stack → if head_role → player_head_role_stack sequence appears four times in this function with only the weapon data varying. Extract a helper that performs this sequence to eliminate duplication.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_player_combat_weapon_stacks/scr_player_combat_weapon_stacks.gml, line 298:
<comment>Repeated code block pattern with existing Hammer of Wrath, primary ranged, and primary melee handling — not abstracted into a shared helper. The same find_stack_index → add_data_to_stack → if head_role → player_head_role_stack sequence appears four times in this function with only the weapon data varying. Extract a helper that performs this sequence to eliminate duplication.</comment>
<file context>
@@ -295,6 +295,16 @@ function scr_player_combat_weapon_stacks() {
}
}
}
+ if (is_struct(mobi_item) && mobi_item.has_tag("bike")) {
+ var _speed_force = unit.speed_force(mobi_item.has_tag("sf_ranged"));
+ var stack_index = find_stack_index(_speed_force.name, head_role, unit);
</file context>
| target_armour_value = 0; | ||
| // Armour multiplier indexed by AP rating (1..4); any AP outside that range | ||
| // leaves armour untouched. Infantry and vehicles scale differently. | ||
| var _inf_ap = [1, 3, 2, 1.5, 0]; |
There was a problem hiding this comment.
P2: Custom agent: Code Quality Review
Duplicate armor-piercing lookup tables _inf_ap and _veh_ap are defined identically in both scr_shoot and scr_shoot_spread. Per the code quality rule, repeated code fragments (2+ near-identical lines/blocks) should be abstracted into a shared function or helper. Extract a helper function (e.g. get_ap_multiplier(armour_pierce, is_vehicle)) to return the correct multiplier and eliminate the duplicated definitions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_shoot/scr_shoot.gml, line 297:
<comment>Duplicate armor-piercing lookup tables `_inf_ap` and `_veh_ap` are defined identically in both `scr_shoot` and `scr_shoot_spread`. Per the code quality rule, repeated code fragments (2+ near-identical lines/blocks) should be abstracted into a shared function or helper. Extract a helper function (e.g. `get_ap_multiplier(armour_pierce, is_vehicle)`) to return the correct multiplier and eliminate the duplicated definitions.</comment>
<file context>
@@ -275,280 +267,314 @@ function scr_shoot(weapon_index_position, target_object, target_type, damage_dat
- target_armour_value = 0;
+ // Armour multiplier indexed by AP rating (1..4); any AP outside that range
+ // leaves armour untouched. Infantry and vehicles scale differently.
+ var _inf_ap = [1, 3, 2, 1.5, 0];
+ var _veh_ap = [1, 6, 4, 2, 0];
+ var _ap_valid = (armour_pierce >= 1) && (armour_pierce <= 4);
</file context>
There was a problem hiding this comment.
5 issues found across 14 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/scr_start_load/scr_start_load.gml">
<violation number="1" location="scripts/scr_start_load/scr_start_load.gml:58">
P1: Cross-file contract mismatch: 'Basilisk' added to vehicle whitelist but missing from `get_vehicle_size_map()` in `scr_unit_size`, causing Basilisks to default to size 1.</violation>
</file>
<file name="scripts/scr_roster/scr_roster.gml">
<violation number="1" location="scripts/scr_roster/scr_roster.gml:828">
P1: Basilisk is configured for HP/AC in `add_vehicle_to_battle` but is missing from the vehicle role-to-column `switch`, causing fallback placement and missing counter update</violation>
<violation number="2" location="scripts/scr_roster/scr_roster.gml:881">
P2: New `promote_auxilia_to_veteran` function is documented as intended for a 'Promote All' button but is never called anywhere in the codebase, leaving the feature unwired and unreachable.</violation>
</file>
<file name="datafiles/data/unit_stats.json">
<violation number="1" location="datafiles/data/unit_stats.json:641">
P2: ballistic_skill multiplier set to 6 for guardsman and guard_sergeant, a significant outlier from surrounding units (commonly 1–4) that may cause unintended hit-rate scaling. Validate whether this matches the intended stat-system range.</violation>
</file>
<file name="scripts/scr_trade/scr_trade.gml">
<violation number="1" location="scripts/scr_trade/scr_trade.gml:149">
P2: Basilisk faction-retag loop mutates all Basilisks in company 0 instead of only the newly added ones</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
| // i feel like there definatly is or should be a generic function for this???? | ||
| var _vehicles = ["Rhino", "Predator", "Land Speeder", "Land Raider", "Whirlwind"]; | ||
| var _vehicles = ["Rhino", "Predator", "Land Speeder", "Land Raider", "Whirlwind", "Chimera", "Leman Russ", "Basilisk"]; |
There was a problem hiding this comment.
P1: Cross-file contract mismatch: 'Basilisk' added to vehicle whitelist but missing from get_vehicle_size_map() in scr_unit_size, causing Basilisks to default to size 1.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_start_load/scr_start_load.gml, line 58:
<comment>Cross-file contract mismatch: 'Basilisk' added to vehicle whitelist but missing from `get_vehicle_size_map()` in `scr_unit_size`, causing Basilisks to default to size 1.</comment>
<file context>
@@ -55,7 +55,7 @@ function scr_start_load(fleet, load_from_star, load_options) {
}
// i feel like there definatly is or should be a generic function for this????
- var _vehicles = ["Rhino", "Predator", "Land Speeder", "Land Raider", "Whirlwind", "Chimera", "Leman Russ"];
+ var _vehicles = ["Rhino", "Predator", "Land Speeder", "Land Raider", "Whirlwind", "Chimera", "Leman Russ", "Basilisk"];
function load_vehicles(_companies, _equip, _ship, size) {
obj_ini.veh_wid[_companies][_equip] = 0;
</file context>
| targ.veh_hp[targ.veh] = obj_ini.veh_hp[company][v] * 3; | ||
| targ.veh_hp_multiplier[targ.veh] = 3; | ||
| targ.veh_ac[targ.veh] = 40; | ||
| } else if (obj_ini.veh_role[company][v] == "Basilisk") { |
There was a problem hiding this comment.
P1: Basilisk is configured for HP/AC in add_vehicle_to_battle but is missing from the vehicle role-to-column switch, causing fallback placement and missing counter update
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_roster/scr_roster.gml, line 828:
<comment>Basilisk is configured for HP/AC in `add_vehicle_to_battle` but is missing from the vehicle role-to-column `switch`, causing fallback placement and missing counter update</comment>
<file context>
@@ -820,6 +825,13 @@ function add_vehicle_to_battle(company, veh_index, is_local) {
targ.veh_hp[targ.veh] = obj_ini.veh_hp[company][v] * 3;
targ.veh_hp_multiplier[targ.veh] = 3;
targ.veh_ac[targ.veh] = 40;
+ } else if (obj_ini.veh_role[company][v] == "Basilisk") {
+ // Mirrors the enemy Basilisk: armour 30, HP 150 (base 100 x1.5). A self-propelled
+ // artillery piece, tougher than a Chimera but lighter than a Leman Russ, built to
</file context>
| /// are tunable. Additions are flat; stat_boosts rebalances constitution into current health. | ||
| /// @param {real} [_company] optional company index to limit promotion to | ||
| /// @returns {real} number of troopers promoted | ||
| function promote_auxilia_to_veteran(_company = undefined) { |
There was a problem hiding this comment.
P2: New promote_auxilia_to_veteran function is documented as intended for a 'Promote All' button but is never called anywhere in the codebase, leaving the feature unwired and unreachable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_roster/scr_roster.gml, line 881:
<comment>New `promote_auxilia_to_veteran` function is documented as intended for a 'Promote All' button but is never called anywhere in the codebase, leaving the feature unwired and unreachable.</comment>
<file context>
@@ -854,5 +866,33 @@ function add_vehicle_to_battle(company, veh_index, is_local) {
+/// are tunable. Additions are flat; stat_boosts rebalances constitution into current health.
+/// @param {real} [_company] optional company index to limit promotion to
+/// @returns {real} number of troopers promoted
+function promote_auxilia_to_veteran(_company = undefined) {
+ var _troops = collect_role_group("all", "", false, { roles: ["Guardsman"] });
+ var _count = 0;
</file context>
| "guardsman": { | ||
| "ballistic_skill": [ | ||
| 25, | ||
| 6 |
There was a problem hiding this comment.
P2: ballistic_skill multiplier set to 6 for guardsman and guard_sergeant, a significant outlier from surrounding units (commonly 1–4) that may cause unintended hit-rate scaling. Validate whether this matches the intended stat-system range.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At datafiles/data/unit_stats.json, line 641:
<comment>ballistic_skill multiplier set to 6 for guardsman and guard_sergeant, a significant outlier from surrounding units (commonly 1–4) that may cause unintended hit-rate scaling. Validate whether this matches the intended stat-system range.</comment>
<file context>
@@ -638,7 +638,7 @@
"ballistic_skill": [
25,
- 1
+ 6
],
"base_group": "human",
</file context>
| repeat (_opt.number) { | ||
| scr_add_vehicle("Basilisk", 0); | ||
| } | ||
| for (var _bv = 1; _bv < array_length(obj_ini.veh_role[0]); _bv++) { |
There was a problem hiding this comment.
P2: Basilisk faction-retag loop mutates all Basilisks in company 0 instead of only the newly added ones
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_trade/scr_trade.gml, line 149:
<comment>Basilisk faction-retag loop mutates all Basilisks in company 0 instead of only the newly added ones</comment>
<file context>
@@ -129,6 +137,24 @@ function TradeAttempt(diplomacy) constructor {
+ repeat (_opt.number) {
+ scr_add_vehicle("Basilisk", 0);
+ }
+ for (var _bv = 1; _bv < array_length(obj_ini.veh_role[0]); _bv++) {
+ if (obj_ini.veh_role[0][_bv] == "Basilisk") {
+ obj_ini.veh_race[0][_bv] = eFACTION.IMPERIUM;
</file context>
There was a problem hiding this comment.
9 issues found across 232 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="objects/obj_managment_panel/Create_0.gml">
<violation number="1" location="objects/obj_managment_panel/Create_0.gml:48">
P2: `string_truncate` is called unconditionally on `line[l]` before the `is_struct` guard. When `line[l]` is a struct (which this function explicitly supports via `_struc.str1`), the string-manipulation function receives a struct instead of a string, relying on implicit coercion and doing redundant work before the value is overwritten inside the struct branch.</violation>
</file>
<file name="objects/obj_fleet/Alarm_7.gml">
<violation number="1" location="objects/obj_fleet/Alarm_7.gml:71">
P2: Performance regression: `instance_nearest` is now called unconditionally on every iteration of `repeat (50)` instead of being cached and only re-acquired when the previous fleet is destroyed. This causes repeated global scans of `obj_en_fleet` instances.</violation>
<violation number="2" location="objects/obj_fleet/Alarm_7.gml:125">
P1: Potential null-instance dereference: `ofleet` from `instance_nearest` is not checked against `noone` before accessing `ofleet.inquisitor` in the inquisitor-kill branch. The same file already guards a nearly identical `instance_nearest` call just lines above. A missing fleet here will throw, get swallowed by the outer try/catch, and skip the rest of the post-battle cleanup (including `instance_activate_all()` and alarm resets).</violation>
</file>
<file name="scripts/scr_array_functions/scr_array_functions.gml">
<violation number="1" location="scripts/scr_array_functions/scr_array_functions.gml:131">
P3: Stale/misleading JSDoc parameter description: after renaming the parameter from `stacktrace` to `_array`, the `@param` description still says `stacktrace.`. This contradicts the generic function description and parameter name, creating misleading API documentation.</violation>
</file>
<file name="scripts/scr_company_order/scr_company_order.gml">
<violation number="1" location="scripts/scr_company_order/scr_company_order.gml:36">
P3: Unused local variable `wanted_roles` remains in changed code, suggesting an incomplete refactor.</violation>
</file>
<file name="objects/obj_event_log/Draw_0.gml">
<violation number="1" location="objects/obj_event_log/Draw_0.gml:55">
P2: Alpha reset is no longer unconditional in `help == 0`, which may leak draw alpha state when menu condition is false.</violation>
</file>
<file name="objects/obj_drop_select/Alarm_2.gml">
<violation number="1" location="objects/obj_drop_select/Alarm_2.gml:1">
P2: Array reinitialization with `array_create(31, ...)` replaces the arrays entirely, discarding the sentinel state at index 500 that the Create event and other scripts explicitly set. The old loop only mutated indices 0..30, preserving slot 500. The new code breaks that contract, making `ship_max[500]`, `ship_all[500]`, and `ship_use[500]` access undefined/defaults rather than the intended sentinel values.</violation>
</file>
<file name="scripts/ColourItem/ColourItem.gml">
<violation number="1" location="scripts/ColourItem/ColourItem.gml:557">
P1: `dummy_marine ?? true` re-creates `DummyMarine` on every draw frame because a truthy struct passes the condition, breaking the original one-time lazy initialization semantics.</violation>
</file>
<file name="scripts/scr_load/scr_load.gml">
<violation number="1" location="scripts/scr_load/scr_load.gml:80">
P2: Inside `with (obj_controller)`, `variable_instance_set(obj_controller, ...)` passes the object index instead of the scoped instance ID. This bypasses the `with` block's instance scoping and may write save data to the wrong controller instance if more than one exists.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| //draw_sprite(sprite_index, 0, x, y); | ||
| if (dummy_marine == false) { | ||
| if (dummy_marine ?? true) { |
There was a problem hiding this comment.
P1: dummy_marine ?? true re-creates DummyMarine on every draw frame because a truthy struct passes the condition, breaking the original one-time lazy initialization semantics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/ColourItem/ColourItem.gml, line 557:
<comment>`dummy_marine ?? true` re-creates `DummyMarine` on every draw frame because a truthy struct passes the condition, breaking the original one-time lazy initialization semantics.</comment>
<file context>
@@ -554,8 +554,7 @@ function ColourItem(_xx, _yy) constructor {
- //draw_sprite(sprite_index, 0, x, y);
- if (dummy_marine == false) {
+ if (dummy_marine ?? true) {
dummy_marine = new DummyMarine();
}
</file context>
| if (dummy_marine ?? true) { | |
| if (!is_struct(dummy_marine)) { |
| ofleet = instance_nearest(room_width / 2, room_height / 2, obj_en_fleet); | ||
| killer = 1; | ||
| var ofleet = instance_nearest(room_width / 2, room_height / 2, obj_en_fleet); | ||
| killer = true; |
There was a problem hiding this comment.
P1: Potential null-instance dereference: ofleet from instance_nearest is not checked against noone before accessing ofleet.inquisitor in the inquisitor-kill branch. The same file already guards a nearly identical instance_nearest call just lines above. A missing fleet here will throw, get swallowed by the outer try/catch, and skip the rest of the post-battle cleanup (including instance_activate_all() and alarm resets).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_fleet/Alarm_7.gml, line 125:
<comment>Potential null-instance dereference: `ofleet` from `instance_nearest` is not checked against `noone` before accessing `ofleet.inquisitor` in the inquisitor-kill branch. The same file already guards a nearly identical `instance_nearest` call just lines above. A missing fleet here will throw, get swallowed by the outer try/catch, and skip the rest of the post-battle cleanup (including `instance_activate_all()` and alarm resets).</comment>
<file context>
@@ -148,8 +121,8 @@ try {
- ofleet = instance_nearest(room_width / 2, room_height / 2, obj_en_fleet);
- killer = 1;
+ var ofleet = instance_nearest(room_width / 2, room_height / 2, obj_en_fleet);
+ killer = true;
obj_controller.temp[1071] = enemy[op];
killer_tg = ofleet.inquisitor;
</file context>
| var chunk_size = scrolly / my; | ||
| var y5 = (top - 1) * chunk_size; | ||
| draw_rectangle(x1, y1 + y5, x2, y1 + y5 + cubey, 0); | ||
| draw_set_alpha(1); |
There was a problem hiding this comment.
P2: Alpha reset is no longer unconditional in help == 0, which may leak draw alpha state when menu condition is false.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_event_log/Draw_0.gml, line 55:
<comment>Alpha reset is no longer unconditional in `help == 0`, which may leak draw alpha state when menu condition is false.</comment>
<file context>
@@ -48,26 +41,25 @@ if (help == 0) {
+ var chunk_size = scrolly / my;
+ var y5 = (top - 1) * chunk_size;
draw_rectangle(x1, y1 + y5, x2, y1 + y5 + cubey, 0);
+ draw_set_alpha(1);
}
- draw_set_alpha(1);
</file context>
| } | ||
| if (instance_exists(ofleet)) { | ||
| /// @type {Id.Instance.obj_en_fleet} | ||
| var ofleet = instance_nearest(room_width / 2, room_height / 2, obj_en_fleet); |
There was a problem hiding this comment.
P2: Performance regression: instance_nearest is now called unconditionally on every iteration of repeat (50) instead of being cached and only re-acquired when the previous fleet is destroyed. This causes repeated global scans of obj_en_fleet instances.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_fleet/Alarm_7.gml, line 71:
<comment>Performance regression: `instance_nearest` is now called unconditionally on every iteration of `repeat (50)` instead of being cached and only re-acquired when the previous fleet is destroyed. This causes repeated global scans of `obj_en_fleet` instances.</comment>
<file context>
@@ -54,57 +46,43 @@ try {
- }
- if (instance_exists(ofleet)) {
+ /// @type {Id.Instance.obj_en_fleet}
+ var ofleet = instance_nearest(room_width / 2, room_height / 2, obj_en_fleet);
+ if (ofleet != noone) {
if (ofleet.trade_goods == "player_hold") {
</file context>
| // LOGGER.debug($"obj_controller var: {var_name} - val: {loaded_value}"); | ||
| try { | ||
| variable_struct_set(obj_controller, var_name, loaded_value); | ||
| variable_instance_set(obj_controller, var_name, loaded_value); |
There was a problem hiding this comment.
P2: Inside with (obj_controller), variable_instance_set(obj_controller, ...) passes the object index instead of the scoped instance ID. This bypasses the with block's instance scoping and may write save data to the wrong controller instance if more than one exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_load/scr_load.gml, line 80:
<comment>Inside `with (obj_controller)`, `variable_instance_set(obj_controller, ...)` passes the object index instead of the scoped instance ID. This bypasses the `with` block's instance scoping and may write save data to the wrong controller instance if more than one exists.</comment>
<file context>
@@ -75,9 +76,8 @@ function scr_load(save_part, save_id) {
- // LOGGER.debug($"obj_controller var: {var_name} - val: {loaded_value}");
try {
- variable_struct_set(obj_controller, var_name, loaded_value);
+ variable_instance_set(obj_controller, var_name, loaded_value);
} catch (e) {
LOGGER.debug(e);
</file context>
| variable_instance_set(obj_controller, var_name, loaded_value); | |
| variable_instance_set(id, var_name, loaded_value); |
| /// @function array_to_string_list | ||
| /// @description Converts an array into a string, with each element on a newline. | ||
| /// @param {array} stacktrace stacktrace. | ||
| /// @param {array} _array stacktrace. |
There was a problem hiding this comment.
P3: Stale/misleading JSDoc parameter description: after renaming the parameter from stacktrace to _array, the @param description still says stacktrace.. This contradicts the generic function description and parameter name, creating misleading API documentation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_array_functions/scr_array_functions.gml, line 131:
<comment>Stale/misleading JSDoc parameter description: after renaming the parameter from `stacktrace` to `_array`, the `@param` description still says `stacktrace.`. This contradicts the generic function description and parameter name, creating misleading API documentation.</comment>
<file context>
@@ -128,13 +128,13 @@ function array_delete_random_index(choice_array) {
/// @function array_to_string_list
/// @description Converts an array into a string, with each element on a newline.
-/// @param {array} stacktrace stacktrace.
+/// @param {array} _array stacktrace.
/// @return {string}
function array_to_string_list(_array, _pop_last = false) {
</file context>
| /// @param {array} _array stacktrace. | |
| /// @param {array} _array The array to convert. |
|
|
||
| //at this point check that all squads have the right types and numbers of units in them | ||
| var _squad, wanted_roles; | ||
| var wanted_roles; |
There was a problem hiding this comment.
P3: Unused local variable wanted_roles remains in changed code, suggesting an incomplete refactor.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_company_order/scr_company_order.gml, line 36:
<comment>Unused local variable `wanted_roles` remains in changed code, suggesting an incomplete refactor.</comment>
<file context>
@@ -36,9 +33,9 @@ function scr_company_order(company) {
//at this point check that all squads have the right types and numbers of units in them
- var _squad, wanted_roles;
+ var wanted_roles;
var _squad_ids = get_squad_ids();
- for (i = 0; i < array_length(_squad_ids); i++) {
</file context>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="objects/obj_en_fleet/Create_0.gml">
<violation number="1" location="objects/obj_en_fleet/Create_0.gml:134">
P1: Save-compatibility regression: legacy saves used `orbiting = 0` for non-orbiting fleets. Since `0 != noone` is `true` in GameMaker, old saves now incorrectly pass this condition and force-assign fleets to the nearest star on load.</violation>
</file>
<file name="scripts/scr_cheatcode/scr_cheatcode.gml">
<violation number="1" location="scripts/scr_cheatcode/scr_cheatcode.gml:108">
P2: Custom agent: **Code Quality Review**
Raw string identifier 'home_planet' used as a constant more than once without being defined as a #macro or enum.</violation>
</file>
<file name="objects/obj_pnunit/Create_0.gml">
<violation number="1" location="objects/obj_pnunit/Create_0.gml:12">
P2: Custom agent: **Code Quality Review**
Addition of an obsolete/dead variable `guard` increases maintenance burden by preserving legacy state for branches that never execute.</violation>
</file>
<file name="scripts/scr_draw_unit_image/scr_draw_unit_image.gml">
<violation number="1" location="scripts/scr_draw_unit_image/scr_draw_unit_image.gml:528">
P2: Custom agent: **Code Quality Review**
Heavy allocation (sprite_add) inside a Draw event, causing a performance hitch on first call.</violation>
<violation number="2" location="scripts/scr_draw_unit_image/scr_draw_unit_image.gml:534">
P2: Custom agent: **Code Quality Review**
Missing fail-loud pattern for runtime sprite_add failure. If the mod-provided PNG is missing or the working_directory path is wrong, the code silently skips drawing with no log output, despite LOGGER being available.</violation>
<violation number="3" location="scripts/scr_draw_unit_image/scr_draw_unit_image.gml:535">
P2: `variable_global_exists` guards `sprite_add` reload but never recovers from a failed load. If `sprite_add` returns `-1` (documented failure value), the global variable still exists, so subsequent calls skip `sprite_add` forever, leaving the portrait permanently missing. Also affects the Guard Sergeant block below.</violation>
</file>
<file name="scripts/scr_roster/scr_roster.gml">
<violation number="1" location="scripts/scr_roster/scr_roster.gml:116">
P2: Custom agent: **Code Quality Review**
Raw string literals ('Guardsman', 'Guard Squad', 'Guard Sergeant', 'guardsman') are used as repeated identifiers/constants without #macro or enum definitions.</violation>
<violation number="2" location="scripts/scr_roster/scr_roster.gml:116">
P2: Custom agent: **Code Quality Review**
Repeated guard-role string-literal checks are not abstracted despite an `auxilia_roles()` helper being added in the same PR. Use `array_contains(auxilia_roles(), _role)` (or add an `is_auxilia_role()` wrapper) to eliminate the duplicated inline list in `update_roster`, `determine_full_roster`, and `add_unit_to_battle`.</violation>
<violation number="3" location="scripts/scr_roster/scr_roster.gml:298">
P1: Guardsman filter button creation is gated by `_unit.squad == "none"`, which may miss guard-role units that have no squad type but do have a squad id.</violation>
<violation number="4" location="scripts/scr_roster/scr_roster.gml:828">
P1: Basilisk is configured for HP/AC in `add_vehicle_to_battle` but is missing from the vehicle role-to-column `switch`, causing fallback placement and missing counter update</violation>
<violation number="5" location="scripts/scr_roster/scr_roster.gml:881">
P2: New `promote_auxilia_to_veteran` function is documented as intended for a 'Promote All' button but is never called anywhere in the codebase, leaving the feature unwired and unreachable.</violation>
</file>
<file name="scripts/scr_add_vehicle/scr_add_vehicle.gml">
<violation number="1" location="scripts/scr_add_vehicle/scr_add_vehicle.gml:126">
P2: Custom agent: **Code Quality Review**
Added another near-identical vehicle-type weapon assignment block to an already duplicated if-chain (Rhino, Whirlwind, Land Speeder, etc.). These repeated blocks differ only in string literals and should be abstracted into a helper function, lookup struct/ds_map, or data table rather than adding more if-blocks.</violation>
</file>
<file name="scripts/scr_add_man/scr_add_man.gml">
<violation number="1" location="scripts/scr_add_man/scr_add_man.gml:32">
P1: Guard roles can bypass both unit-construction paths when `other_gear` is false, leaving `_unit` uninitialized. The Guard role cases are inside `if (other_gear == true)`, but because they are also in `non_marine_roles`, the marine `else` path is skipped too. The code then unconditionally calls `_unit.add_exp`, `_unit.allocate_unit_to_fresh_spawn`, and `_unit.update_role` on whatever `fetch_unit` returned. Consider initializing Guard roles unconditionally or restructuring the guard so that a role in `non_marine_roles` always receives a valid `_unit`.</violation>
<violation number="2" location="scripts/scr_add_man/scr_add_man.gml:32">
P2: Custom agent: **Code Quality Review**
Three new switch cases for Guard roles are near-identical and not abstracted. The only difference between 'Guardsman', 'Guard Squad', and 'Guard Sergeant' is the role string passed to TTRPG_stats. This violates the code quality rule against repeated code fragments that should be abstracted.</violation>
</file>
<file name="objects/obj_ini/Create_0.gml">
<violation number="1" location="objects/obj_ini/Create_0.gml:339">
P1: Load path may crash on older/incomplete save data because `array_length` is called on potentially missing `name[_coy]` entries.</violation>
</file>
<file name="scripts/scr_load_all/scr_load_all.gml">
<violation number="1" location="scripts/scr_load_all/scr_load_all.gml:11">
P2: Custom agent: **Code Quality Review**
Added code exceeds the maximum of 3 nested control structures (if/else, for/while, try/catch, switch). The new block contains 4 nested structures: `if (select_units)` → `if (selecting_location == "")` → `for` → `if`, reducing readability and maintainability.</violation>
</file>
<file name="objects/obj_star_select/Draw_64.gml">
<violation number="1" location="objects/obj_star_select/Draw_64.gml:308">
P1: Deploy Guard button may bypass normal action gating—no ownership, war state, or fleet presence checks before offering the action, and deploy_guardsmen() does not independently enforce those constraints.</violation>
<violation number="2" location="objects/obj_star_select/Draw_64.gml:382">
P2: deploy_guardsmen can return 0 on failure/no-op, but the caller unconditionally shows a success popup using the returned value without validation.</violation>
</file>
<file name="scripts/scr_post_battle_events/scr_post_battle_events.gml">
<violation number="1" location="scripts/scr_post_battle_events/scr_post_battle_events.gml:191">
P2: Identical hulk teardown logic is duplicated across `space_hulk_strip` and `space_hulk_surrender`. Extract a shared helper to keep cleanup synchronized.</violation>
</file>
<file name="scripts/scr_PlanetData/scr_PlanetData.gml">
<violation number="1" location="scripts/scr_PlanetData/scr_PlanetData.gml:81">
P1: Cached `secret_corruption` and `corruption` fields can become stale when `population == 0`. The assignments were moved before the population-zero reset branch, and the end-of-function re-assignment was removed, so the local cached fields retain pre-reset values while the authoritative system arrays are zeroed.</violation>
</file>
<file name="objects/obj_star/Create_0.gml">
<violation number="1" location="objects/obj_star/Create_0.gml:87">
P2: Broad try/catch swallows all errors from _gar.update() and silently replaces the cached GarrisonForce, masking real runtime defects.</violation>
</file>
<file name="objects/obj_star_select/Create_0.gml">
<violation number="1" location="objects/obj_star_select/Create_0.gml:55">
P2: Custom agent: **Code Quality Review**
Raw number 1000 used as a repeated constant for guard recruitment batch size; should be defined as a named #macro or enum.</violation>
</file>
<file name="scripts/scr_fleet_advisor/scr_fleet_advisor.gml">
<violation number="1" location="scripts/scr_fleet_advisor/scr_fleet_advisor.gml:186">
P1: Custom agent: **Save Compatibility Review**
Accessing the new `ship_guardsmen` array without verifying its existence breaks compatibility with old saves. GameMaker's `game_load` does not re-run the Create event, so saves created before this field existed will lack `ship_guardsmen` on `obj_ini`, causing a runtime error when the fleet advisor iterates over ships. Add an existence check (e.g., `variable_instance_exists(obj_ini, "ship_guardsmen") && ...`) before indexing.</violation>
</file>
<file name="scripts/scr_trade/scr_trade.gml">
<violation number="1" location="scripts/scr_trade/scr_trade.gml:107">
P2: Custom agent: **Code Quality Review**
Magic number 200 replaces a self-documenting derived constant expression. The literal 200 appears twice in related code (Chimera count and disposition cost) and should use a named constant or the derived expression to maintain readability and coupling.</violation>
<violation number="2" location="scripts/scr_trade/scr_trade.gml:149">
P2: Basilisk faction-retag loop mutates all Basilisks in company 0 instead of only the newly added ones</violation>
</file>
<file name="objects/obj_p_fleet/Alarm_1.gml">
<violation number="1" location="objects/obj_p_fleet/Alarm_1.gml:43">
P2: Clamping the movement divisor with `max(1, action_eta)` masks non-positive ETA values. If `action_eta` enters this block at 0 or negative, it is decremented to a negative number and the arrival checks (`action_eta == 0`) will never run, leaving the fleet stuck in movement state with `action` uncleared.</violation>
</file>
<file name="scripts/scr_shoot/scr_shoot.gml">
<violation number="1" location="scripts/scr_shoot/scr_shoot.gml:297">
P2: Custom agent: **Code Quality Review**
Duplicate armor-piercing lookup tables `_inf_ap` and `_veh_ap` are defined identically in both `scr_shoot` and `scr_shoot_spread`. Per the code quality rule, repeated code fragments (2+ near-identical lines/blocks) should be abstracted into a shared function or helper. Extract a helper function (e.g. `get_ap_multiplier(armour_pierce, is_vehicle)`) to return the correct multiplier and eliminate the duplicated definitions.</violation>
<violation number="2" location="scripts/scr_shoot/scr_shoot.gml:307">
P1: Initial retarget guard only checks `dudes_num <= 0` but not `dudes_hp <= 0`, creating a divide-by-zero / invalid damage math risk for zombie ranks. The helper `find_next_alive_rank` explicitly guards `dudes_hp > 0` to keep callers safe from dividing by rank HP, but `scr_shoot` bypasses that protection here.</violation>
<violation number="3" location="scripts/scr_shoot/scr_shoot.gml:525">
P1: `scr_shoot_spread` filters ranks by `dudes_num > 0` but not by `dudes_hp > 0`, allowing division by zero (or negative HP) in the kill calculation and inflating `_total` with dead ranks that dilute proportional shot allocation. This contradicts the alive-rank semantics established by `find_next_alive_rank` in the same file, which explicitly skips `dudes_hp <= 0`.</violation>
</file>
<file name="scripts/scr_player_combat_weapon_stacks/scr_player_combat_weapon_stacks.gml">
<violation number="1" location="scripts/scr_player_combat_weapon_stacks/scr_player_combat_weapon_stacks.gml:121">
P2: Capped Guardsman volley splitting can exhaust finite `wep` stack slots and cause later weapons to be silently skipped (`-1` index path).</violation>
<violation number="2" location="scripts/scr_player_combat_weapon_stacks/scr_player_combat_weapon_stacks.gml:298">
P2: Custom agent: **Code Quality Review**
Repeated code block pattern with existing Hammer of Wrath, primary ranged, and primary melee handling — not abstracted into a shared helper. The same find_stack_index → add_data_to_stack → if head_role → player_head_role_stack sequence appears four times in this function with only the weapon data varying. Extract a helper that performs this sequence to eliminate duplication.</violation>
</file>
<file name="scripts/macros/macros.gml">
<violation number="1" location="scripts/macros/macros.gml:122">
P2: New eROLE.BIKER and eROLE.ATTACK_BIKER enum values are not registered in existing role initialization structures. `obj_controller/Create_0.gml`'s `_roles_data` struct and `scr_initialize_custom.gml`'s `load_default_gear` calls only cover roles up to VETERANSERGEANT, so the new roles will have undefined default names, gear, and weapons.</violation>
</file>
<file name="scripts/scr_start_load/scr_start_load.gml">
<violation number="1" location="scripts/scr_start_load/scr_start_load.gml:58">
P1: Cross-file contract mismatch: 'Basilisk' added to vehicle whitelist but missing from `get_vehicle_size_map()` in `scr_unit_size`, causing Basilisks to default to size 1.</violation>
</file>
<file name="datafiles/data/unit_stats.json">
<violation number="1" location="datafiles/data/unit_stats.json:641">
P2: ballistic_skill multiplier set to 6 for guardsman and guard_sergeant, a significant outlier from surrounding units (commonly 1–4) that may cause unintended hit-rate scaling. Validate whether this matches the intended stat-system range.</violation>
</file>
<file name="objects/obj_managment_panel/Create_0.gml">
<violation number="1" location="objects/obj_managment_panel/Create_0.gml:48">
P2: `string_truncate` is called unconditionally on `line[l]` before the `is_struct` guard. When `line[l]` is a struct (which this function explicitly supports via `_struc.str1`), the string-manipulation function receives a struct instead of a string, relying on implicit coercion and doing redundant work before the value is overwritten inside the struct branch.</violation>
</file>
<file name="objects/obj_fleet/Alarm_7.gml">
<violation number="1" location="objects/obj_fleet/Alarm_7.gml:71">
P2: Performance regression: `instance_nearest` is now called unconditionally on every iteration of `repeat (50)` instead of being cached and only re-acquired when the previous fleet is destroyed. This causes repeated global scans of `obj_en_fleet` instances.</violation>
<violation number="2" location="objects/obj_fleet/Alarm_7.gml:125">
P1: Potential null-instance dereference: `ofleet` from `instance_nearest` is not checked against `noone` before accessing `ofleet.inquisitor` in the inquisitor-kill branch. The same file already guards a nearly identical `instance_nearest` call just lines above. A missing fleet here will throw, get swallowed by the outer try/catch, and skip the rest of the post-battle cleanup (including `instance_activate_all()` and alarm resets).</violation>
</file>
<file name="scripts/scr_array_functions/scr_array_functions.gml">
<violation number="1" location="scripts/scr_array_functions/scr_array_functions.gml:131">
P3: Stale/misleading JSDoc parameter description: after renaming the parameter from `stacktrace` to `_array`, the `@param` description still says `stacktrace.`. This contradicts the generic function description and parameter name, creating misleading API documentation.</violation>
</file>
<file name="scripts/is_specialist/is_specialist.gml">
<violation number="1" location="scripts/is_specialist/is_specialist.gml:25">
P2: JSDoc `@param {Real} group` in `role_groups()` is incorrect — callers must pass string macro constants (e.g. SPECIALISTS_STANDARD = "standard"), not numeric values.</violation>
</file>
<file name="scripts/scr_company_order/scr_company_order.gml">
<violation number="1" location="scripts/scr_company_order/scr_company_order.gml:36">
P3: Unused local variable `wanted_roles` remains in changed code, suggesting an incomplete refactor.</violation>
</file>
<file name="objects/obj_event_log/Draw_0.gml">
<violation number="1" location="objects/obj_event_log/Draw_0.gml:55">
P2: Alpha reset is no longer unconditional in `help == 0`, which may leak draw alpha state when menu condition is false.</violation>
</file>
<file name="objects/obj_drop_select/Alarm_2.gml">
<violation number="1" location="objects/obj_drop_select/Alarm_2.gml:1">
P2: Array reinitialization with `array_create(31, ...)` replaces the arrays entirely, discarding the sentinel state at index 500 that the Create event and other scripts explicitly set. The old loop only mutated indices 0..30, preserving slot 500. The new code breaks that contract, making `ship_max[500]`, `ship_all[500]`, and `ship_use[500]` access undefined/defaults rather than the intended sentinel values.</violation>
</file>
<file name="scripts/ColourItem/ColourItem.gml">
<violation number="1" location="scripts/ColourItem/ColourItem.gml:557">
P1: `dummy_marine ?? true` re-creates `DummyMarine` on every draw frame because a truthy struct passes the condition, breaking the original one-time lazy initialization semantics.</violation>
</file>
<file name="scripts/scr_load/scr_load.gml">
<violation number="1" location="scripts/scr_load/scr_load.gml:80">
P2: Inside `with (obj_controller)`, `variable_instance_set(obj_controller, ...)` passes the object index instead of the scoped instance ID. This bypasses the `with` block's instance scoping and may write save data to the wrong controller instance if more than one exists.</violation>
</file>
<file name="scripts/scr_promote/scr_promote.gml">
<violation number="1" location="scripts/scr_promote/scr_promote.gml:218">
P2: Expanded promotion-role loop from hardcoded 11 to `array_length(role_name)` without verifying that newly exposed role indices have corresponding `role_exp` entries or equipment requirements in `calculate_equipment_needs()`. Non-promotable or unsupported roles could appear in the UI and bypass equipment checks.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| role_y = 0; | ||
| if (target_comp != -1) { | ||
| for (var r = 1; r <= 11; r++) { | ||
| for (var r = 1; r < array_length(role_name); r++) { |
There was a problem hiding this comment.
P2: Expanded promotion-role loop from hardcoded 11 to array_length(role_name) without verifying that newly exposed role indices have corresponding role_exp entries or equipment requirements in calculate_equipment_needs(). Non-promotable or unsupported roles could appear in the UI and bypass equipment checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_promote/scr_promote.gml, line 218:
<comment>Expanded promotion-role loop from hardcoded 11 to `array_length(role_name)` without verifying that newly exposed role indices have corresponding `role_exp` entries or equipment requirements in `calculate_equipment_needs()`. Non-promotable or unsupported roles could appear in the UI and bypass equipment checks.</comment>
<file context>
@@ -215,7 +215,7 @@ function draw_popup_promotion() {
role_y = 0;
if (target_comp != -1) {
- for (var r = 1; r <= 11; r++) {
+ for (var r = 1; r < array_length(role_name); r++) {
if (role_name[r] != "") {
draw_set_alpha(1);
</file context>
…s formation The combat-orders leapfrog fired during default auto-advance: blocks moving in the same tick routinely find the block ahead still in its slot (processing order, or a front line engaged and stationary), and where vanilla stalled them, preserving formation shape, the leapfrog vaulted them over their own line. A raid with no orders given reordered itself, tacticals jumping the engaged front (tester report). Blocks now carry an order_manual flag set only when the player clicks them: untouched blocks advance with vanilla stall behavior and formations hold their authored shape, while clicked blocks retain full leapfrog movement. This matches the feature's intent: leapfrogging belongs to orders the player gives, not the ambient march.
… psychic helpers Two merge-seam defects broke compilation. The seam restoration of incoming_damage_flavor in scr_flavor2 had appended a truncated fragment (anchored on the doc comment rather than the definition), ending the script mid-declaration; GameMaker degraded the unparseable script to zero-argument functions, cascading into "scr_flavor2 expects 0 arguments" errors at the scr_clean and scr_shoot call sites. The complete 23-line function is restored verbatim from the merge's pre-merge parent. scr_powers carried duplicate definitions of accumulate_psychic_cast and flush_psychic_summary: the fork's Tavish-lineage copies (with a legacy 3-argument add_battle_log_message call) survived alongside upstream's rebuilt versions; the duplicates are excised, keeping upstream's eMSG_COLOR-API copies. Verified: single definitions, the 8-parameter scr_flavor2 signature intact, and zero legacy 3-argument log calls remaining anywhere.
…udes chaos specials GM2043 triage found one live defect among the warnings: var slaa was declared inside the Daemon Forces block (CHAOS + ship_demon/ruins_eldar) but read 39 times inside the mutually exclusive CSM warband composition block, so any plain Chaos ground battle whose composition reached an if (slaa) case read an uninitialized local, a hard crash in this build. Present in vanilla's old numbering as well. The declaration is hoisted above both blocks. The triage also exposed a gate gap from the spawn-side renumbering: the CSM warband block's CHAOS gate lacked the chaos-special exclusions (ship_demon, ruins_eldar, fallen1/2, WL10_reveal/later) that its HERETICS sibling carries, so those special battles would have spawned a CSM warband on top of their own composition; the exclusions are added. Remaining GM2043/GM2044 warnings are vanilla lint (function-scoped locals, write-before-read event assignments, redeclarations), runtime-safe and deliberately left untouched to avoid merge churn against upstream.
Changed it so it's bigger, matches the size of portraits. Experimental feature for all future portraits of vehicles
…alizer lost in merge
Starting any new campaign crashed to desktop at obj_controller Alarm_1:491
("Variable obj_controller.go not set before reading it"). The craftworld
placement loop's state variable was initialized by "var go = 0;" immediately
above the loop; upstream's initialization cleanup removed that line in a hunk
that auto-applied through the merge without conflicting, while the fork's
side of the file kept every read. Galaxy generation therefore died at the
first "if (go == 0)" on every new game, any chapter or seed; existing saves
were unaffected since Alarm_1 only runs at creation. The declaration is
restored above the loop with a comment. Region diff confirms this was the
only lost line there; upstream's other changes in the block are kept.
… posts in Alarm_5 Clicking Attack crashed the drop screen on draw: upstream's UI refactor renamed the formation selector button to btn_formation in obj_drop_select's Create (auto-merged), while the fork's kept drop_select_unit_selection still addressed the old name. The five references are renamed to match. A systematic sweep of all Create-event initializers deleted or renamed by the merge, cross-referenced against every OURS-kept file, found no further unset reads (remaining hits were cross-object or comment false positives) but exposed two blended log defects in obj_ncombat Alarm_5: the plasma-bomb-ineffective path pushed the stale _newline in red beside its real message, re-posting the previous log line on every tomb resolution, and the Exterminatus announcement pushed stale _newline instead of its own text (legacy newline assignments beside upstream's push). Both sites now post one correct line each.
Upstream's attack refactor removed the cached ranged_damage_data and melee_damage_data fields; the fork's stack builder read them on the first shooting alarm of every battle (hard crash) and hammer_of_wrath read them inside the struct. Call ranged_attack()/melee_attack() once and reuse the returned array, which keeps the same [0] attack / [3] primary weapon layout and includes the packless-Hellgun swap. Also rename the adopted scr_clean debug line's hostile_armour_pierce to the fork's hostile_arp parameter; the unset read threw before damage applied, so enemy infantry volleys were silently skipped.
The per-casualty cover roll in scr_clean gave every non-Guard unit the marine
cover save, Dreadnoughts included ("The Dreadnought weather the fire from
effective cover"). A walker chassis cannot crouch behind rubble: any unit
whose armour has the dreadnought tag now takes a flat zero cover save. The
same rule is documented at the site for the enemy walker types (Deff Dreads,
Wraithlords, Helbrutes, T'au battlesuits) for when the per-race enemy cover
system is built; enemy cover does not exist in code yet.
…liers Terminator-tagged armour now takes no cover, matching the dreadnought rule: massive tactical plate does not crouch behind rubble. Thread the real melee/ranged flag from scr_shoot through scr_clean into damage_infantry and zero the cover factor for melee volleys entirely: there is no rubble between locked blades, for Guard and Astartes alike. Add posture to the save for player formations only (the enemy AI has no posture control, so it neither suffers nor benefits): a block that stepped this sweep covers at COVER_SAVE_MOVING_MULT 0.75, one ordered to Hold digs in at COVER_SAVE_HOLDING_MULT 1.25, and advance-ordered but stationary blocks stay at the base rate. Both multipliers are macros beside the other cover tunables.
The AI battle resolver flipped p_owner the moment the owner side was spent, but it is region-blind, and the region layer's extinct-owner liberation reads the population model while the resolver fights on the level scalars: a fresh conqueror with troops but no settled populace looked extinct, so the deed was stripped and retaken every turn, firing "have taken" and "authority is restored" endlessly. Make the liberation level-aware (a faction with a live force level is not extinct) and stage conquest through the new region_npc_conquer_step: each crushing round overruns ONE region, lowest first, capital last, skipping player-held footholds, flipping the region's owner on the map, logging one overrun event, and resetting its stored garrison so the winner's re-derives from its own level. p_owner flips and the "have taken" event fires only when the capital falls. Building effects gate on region owner already, so fallen regions go dormant until recaptured; the future destroy-on-full-capture mechanic is a documented placeholder.
The Tau spread block seeded fresh landings onto neighbouring planets on nothing but an established beachhead, so an invasion whose fleet the player had destroyed kept exporting troops with no transports in the sky. Add orbital presence (present_fleet[eFACTION.TAU] > 0) to the spread's existing disqualifier, mirroring the CSM landing's fleet gate. Troops already landed stay and fight on as a stranded force for the Imperium or the chapter to mop up, exactly as before.
The armour soak ladder in damage_vehicles was non-monotonic: arp 2 soaked at x4 while arp 1 soaked at x3, so a mid-grade piercing weapon pierced player vehicles worse than a low-grade one, a leftover from the arp 1 fix that never re-laddered its neighbour. Set arp 2 to x2.5, between arp 1 (x3) and arp 3 (x2); all other rungs and every per-hull penetration chance are unchanged. In the live enemy table arp 2 covers the Eldar arsenal, Tau pulse and missile systems, Chaos heavy weapons, Necron gauss and Tyranid bio-guns, whose mid-tier pieces stop reading as zero against medium hulls. Correct the stale comment claiming enemy AT is arp 1: Kannon, Rokkit and Lascannon are arp 4 in the live scr_en_weapon switch and bypass soak entirely, gated by the penetration roll.
update_roster only admits units from active ship toggles, so troops aboard ships that had spent their assault uses were silently excluded and a follow-up launch proceeded with whatever remained, often a fraction, against a full region garrison: the "all forces present but only some fight" report. Add a roster count of units locked aboard spent ships, paint it red beside the Selected Squads header, and log a DROP ROSTER line (launching / locked / left behind) at every launch so future reports carry the numbers. The per-ship cap itself is unchanged.
spent_ship_stranded_count read assault_locked off every ship button, but not every roster path builds buttons with the assault economy fields (the Necron tomb / Dead-world attack roster does not), throwing a caught error every draw frame of that drop select. Probe for the fields first; a button without them is not locked and such paths correctly report zero stranded.
…e hordes The engaged force on a world becomes a slice with a reserve behind it. Each planet seeds p_pdf_reserve once from its garrison's deterrent tier (lore anchors tier 1 ~ 2,000 souls to tier 6 ~ 50,000,000; battles stay at force point scale) and replaces 60% of every war-room round's PDF losses from it until it is spent, announced once. Hostile fleets carry an invasion_reserve sized by hulls, faction and a ±70% swing (Orks 12x, Tyranids 40x with 5% per turn regeneration while in orbit, others 4x), feeding 12% of ceiling to the ground each losing round: total-war races refill their headcount, levy races regain a strength level. No fleet in orbit means no feeding, matching the stranded invasion rule. Pools initialise lazily (old saves covered, generic serializer persists them), log their size once, and every rate is a macro.
region_count_for_planet returned 1 whenever p_max_population read as zero, and region generation can run in the load window before a star's population fields are restored, so full worlds (the Sinophia II Hive report) were stored with permanent capital-only layouts that a non-empty array never regenerates. Let the planet TYPE decide the count before any population reading, keeping the zero gate only for unlisted types, and teach regions_ensure to heal undersized layouts: append the missing outlying regions exactly as generation builds them (persisted names, first_owner, reseeded force weights, rebuilt adjacency, civil totals re-spread) while leaving footholds, garrisons, buildings and owners on existing regions untouched. Layouts never shrink, and each heal logs a REGIONS_HEAL line for the repair report.
man_sel only extends when an index is written, but the display roster can grow while the manage screen is open: promoting a techmarine into a new company appended a row past the selection array's high-water mark, and the squad row renderer's per-member selection read then threw out-of-range every frame. Ensure at the scr_ui_manage entry that man_sel spans the display roster (one write to the last index, zero-filled), covering every read site for any roster growth mid-session.
…ns panel Imperial-owned regions showed a numberless forces label because their defense lives in the live PDF and Guardsmen pools rather than stored garrison records: derive each region's share of those pools with the same capital-remainder split hostile garrisons use, so every region shows a real number that falls as the background war bleeds the pools and holds while the PDF reserve feeds them. Add the invader to the panel: a red assault line with the attacker's remaining headcount on the front region, computed by the new region_npc_front (now also the conquest step's single source of the front rule), chosen by planet_strongest_attacker across the hostile classes, with a tooltip explaining the region-by-region grind toward the capital.
Promoting a marine into a new company shrinks or reorders the company view while the squad rows still hold member indices captured against the old list, so the per-member selection reads threw out-of-range every frame and growing man_sel could not help: a stale index points past both arrays. Filter each row's members against the current display roster and selection array at the top of BOTH squad row drawers, skip rows left empty, and let the click-toggle loops share the filtered array so every access in the row is covered. The entry guard from the first attempt remains for rosters that grow mid-screen. Supersedes the man_sel elasticity fix.
…circuit validate_modular_item resets _sub_comps and _shadows for each item but never _overides, which is only written inside modular_mandatory_checks, and base_modulars_checks skipped that function entirely for items whose keys are all mandatory: bare_head, bare_neck and bare_eyes. Drawing a bare-headed marine (a Scout company) read an unset _overides and threw, and since assign_modulars catches the exception the whole loop aborted, silently dropping every modular item after index 89 of 156 from the sprite. Reset _overides with its siblings, and let the short-circuit path return modular_mandatory_checks so the documented always-run contract holds; blocked is empty in this build so today's output is unchanged. Present verbatim in upstream, relay to Mystic and Etty.
Two force models can desync: a cleanse that zeroed only the legacy 0-6 level left p_race_pop standing, so the survivors read as present to every population consumer while every level-gated consumer saw nothing, producing an army that cannot be fought, cannot be bled and outlives rival invasions (Quarth I, 1,119 Tyranids with no level and no cult feature). Add faction_pop_orphan_sweep, run once per planet per turn from the ownership pass, clearing population with no force level and logging each cleanup so existing saves heal. Separately, an infiltrating Genestealer Cult legitimately holds population at level 0 and fields no force, but the regions panel read the raw headcount and painted it as an assaulting army (Memlok II, 69,060 under a hidden cult): add faction_pop_host_is_concealed and skip concealed hosts in planet_strongest_attacker. Guard faction_pop_clamp_to_level with the same predicate so no path wipes a hidden host.
The resource bar and banner draw on the fixed 1600x900 GUI layer, but
scr_ui_tooltip anchored its hover rects to the camera view: identical at the
default framing, drifting at any wheel zoom. Zoomed out, the banner's Turn
rect slid under the top-left bar and the gene-seed rect slid over the Forge
Points box (the wrong-tooltip report), while the true rects shrank off their
boxes ("hard to mouse over"). Convert all seven HUD rects (requisition,
forge points, loyalty, gene-seed, marines/command, turn banner, penitence)
to GUI-space testing via scr_hit's force_gui flag with their existing
offsets, which the draw code confirms are correct for the bar's real layout.
Map-object tooltips stay world-anchored.
Mirror the player's per-type formations onto the enemy. The spawn's mixed one-block-per-column line now divides into per-category segments sharing each column: Warlords, Elite Heavy Infantry, Elites, Fire Support, Assault Troops, Skirmishers, Walkers, Vehicles and Line Infantry, classified by name keyword (most specific first) with an unrecognised unit falling through to line. enemy_formation_split runs at the end of the spawn before any Alarm_1, so each segment derives its own weapons, size and force accounting; the largest category keeps the original instance, vacated entries are zeroed for Alarm_1's compaction, and totals are unchanged. Segments draw stacked with the same gap and compression rules as player formations, are individually hoverable, and name themselves in the tooltip and debug labels. Add enemy doctrine in place of an AI planner: Assault segments leap the last stretch into the player's front line once per battle, Fire Support holds and shoots unless the race is melee doctrine (Orks, Tyranids, Genestealers). Since segments share an instance x and y, retarget fire to the segment facing the shooter, or a line's whole output would resolve to one instance.
region_set_owner was the designated entry point for regions changing hands and was never called: no region ever became player-owned, so the adjacency advance could not start and Hold Ground bought a foothold the campaign layer could not see. Take a region when it is cleared AND the chapter has troops booked ashore there, so a raid differs from a conquest. Extend the assault gate beyond orbital-gun worlds: the first landing is free, a contested beachhead is the only assaultable region until cleared, then the front advances into any region bordering held ground. The sector selector skips locked regions and tags them. Replace the flat REGION_GARRISON_CEILING for hostile garrisons with a weighted share (deployment doctrine x persistent per-region random modifier x capital bias), since the tactical fight is bounded by the threat cap and capping the store only hid the campaign. Let the capital field the full tier (ENEMY_BATTLE_THREAT_CAP_CAPITAL) while outlying sectors keep the slice. Cap T'au fleet growth so only TAU_WAR_FLEETS_MAX fleets exceed picket size. Fix the perils daemon reading dudes off noone when the last enemy block dies, and the Hive Tyrant's "Lashwhip" typo that fell through to range 0 every turn.
Enemy size derives from the world's faction strength tier and the region attacked, never from what the chapter commits: the spawner's only reads of player units are positional (where the enemy line forms). Log the whole threat decision each battle so any future change that sneaks a player-force term into the spawn is visible in the next tester log. Tier 4 capped every fight at 423-7,093 bodies depending on race, which read as thin against a world holding millions; raise the general cap to 5 (957-15,034) with the capital at 6, and record the measured per-tier spawn weights above the macro so tuning starts from data.
Testing warfare
Red bar fix, landing on allied regions fix
The Orbital Gun Array rule forces planetfall at the safe landing zone before any inland strike, but that zone is just the highest-index region and on a contested world it can belong to your own side (Sotha I, Imperium-held Sundered Coast). The gun rule then demanded a landing on the one region the allied-ground guard forbids and the whole planet became unattackable. If your side holds any region on the world you already have a beachhead: skip the forced landing and allow any assault bordering friendly ground. Centralise the friendly test (chapter, or Imperium while not at war) and use it for the allied-ground guard, the staging check and the adjacency advance, so all three agree. Suppress the make-planetfall popup when friendly ground exists.
region_contested_beachhead tested owner == PLAYER, so troops stationed in an Imperium-held region counted as a landing force mid-battle and locked every other region on the world behind "already committed" (480 troops in Imperium-held Sundered Coast blocked the whole planet). Skip all friendly ground: a beachhead is contested only where the ground still belongs to a hostile faction.
The spawn tables size an army from the world's strength tier, which says how much a faction HAS, not how much can stand in contact on the ground being fought over. Derive each region's terrain from its generated name (falling back to planet type), give it a persistent front width (500-3000, terrain base times a stored +/-25% modifier) and a terrain-scaled reinforcement rate, and trim the spawned army to the width by scaling the roster so composition is preserved and the surplus stays in reserve. Add the last stand: a faction holding only one region on a world has nowhere to send reserves and nothing to hold back, so the threat cap lifts to the top of the ladder and the width clamp is suspended, on any world and for any faction. Tag the sector [LAST STAND] before launch and announce it in the combat log. Show the front width and the player's committed force at the drop select; the player side is displayed but not yet enforced.
The widths were sized against the chapter's few hundred bodies, so they throttled engagements down instead of varying them up. Rescale to 2,500 (mountain) through 24,000 (open) and treat the width as a TARGET bounded by the region's real garrison rather than only a ceiling: a narrow front trims a deep garrison and holds the rest in reserve, a wide front scales a modest tier roll up to fill the line. Scaling the roster preserves composition. Cap scale-up at 3x so a ground-down remnant cannot balloon, and total fielded bodies at 60,000. The chapter is never bound by the line, only shown it.
The column-pierce walk collected blocks at strictly lower x, but per-type formations and the close-up advance pack several blocks into one column, so the block get_rightmost returns can be armour while infantry stands at the same x beside it. Those men were invisible to the walk: partially packed the volley skipped them for a smaller block two ranks back with soak applied, and fully closed up it found no men at all and fell through to plinking hulls. Check the same rank first in both paths, with no soak since men beside armour are not screened by it, and mirror it for AP fire finding a vehicle block beside a men-only front block.
…nal volley Two rules combined to make a tank wall unshootable. Blocks sharing a column were each counted as a separate armour line, so one physical rank soaked several times over, and each line took a flat share of the ORIGINAL volley, so three ranks at 0.33 absorbed 99% and a fourth was impossible while the depth cap stopped the search there anyway. Group lines by column and soak once per armour RANK, taking that share of what is still flying, so the volley thins geometrically and always leaves something for the men behind (3 ranks now pass ~30%, 5 ranks ~13%). Count depth in ranks and raise it to 6. Also remove a duplicated DROP ROSTER log line introduced when that file was rebuilt.
First playable slice of Uxie's combat redesign as a fully isolated GUI overlay. Type "gridbattle" in the console to open it: deployment bar with per-type formation creation popup, points budget, rectangle block placement with Terminator deep strike, Deploy All, formation level orders (advance, hold, move, focus fire), sergeant pips, enemy ASCII mobs with a Weirdboy and a mid-battle reinforcement wave, cover tiles, battle log, speed and pause controls, and a victory screen. Generated test forces only; campaign state is never read or written and the map restores on exit.
The overlay boot guarded cursor reactivation with instance_exists, which cannot see deactivated instances, so the guard always failed and the pointer stayed frozen and hidden. Activate unconditionally and draw a fallback spr_cursor from the overlay if no cursor instance is active. While placing a formation, the mouse wheel now widens or narrows the block footprint and R rotates it; the popup Deploy button resets to the near-square default. The speed button cycle gains a 0.5x notch (x0.5, x1, x2, x4).
Formations gain a melee stance button (Auto, Charge, Avoid): Auto keeps the per-squad preference where Assault types close in, Charge forces everyone into melee, Avoid kites away from adjacent foes and fights at range, only drawing blades when cornered. Combat feedback now floats above the units Caves of Qud style: per-tick casualty numbers, sergeant losses, wipes, and Weirdboy zaps rise and fade over the battlefield, and any squad being struck flashes red. Speed settings become named (Slow, Normal, Fast, Very Fast) with Normal as the default. Deployment points stay usable after battle begins: the type buttons and Deploy All field reinforcements mid-fight, letting Terminators deep strike onto a live battlefield. Opening the popup or aiming a drop freezes the sim.
Summary by cubic
Adds Imperial Guard Auxilia, multi‑region planets with per‑region builds and income, and a deeper ground‑combat layer with per‑formation orders. Rebuilds the Eldar craftworld hunt, overhauls bombardment with per‑ship spend and live previews, decouples ship actions, adds sector war directives, and ships broad UX/stability fixes.
New Features
Autogun;Hellgunnow forgeable, requires and auto‑pairs aPower Pack); Flak Armour for Guard.Bug Fixes
Written for commit e44cab0. Summary will update on new commits.