feat: Imperial Guard Auxilia#1271
Draft
KestasV wants to merge 65 commits into
Draft
Conversation
For now to recruit them you must be above your home system, press P and type "titheguard x" (x being the number of guardsmen you want (embark guard x). Then you can deploy the guard on the planet you choose via the menu option on the planet. Feel free to test them out. Guardsmen appear in the Fleet menu near the ship name to see if the embark worked.
This reverts commit e088871.
he Guard not moving on attack. move_unit_block("east") won't move a block if another block sits directly ahead of it, so Guard spawned at the rear were permanently pinned behind the Marines, who then advanced off without them. I changed the placement so the Guard are the front rank on attack as well as defense. Now they have open ground ahead, lead the charge, and reach the enemy. Lore-wise it makes them the screening first wave, which fits expendable infantry.
Updated to latest main version
Added imperial guard recruitment button option in the planet population screen under the Colonists. Guardsmen now take from the Planet PDF instead of population, cost 50 req for 1000 guardsmen.
Guardsmen are now hierlins you can take from your planet. To spawn them in type P and "guardsman X" with the number you want. Bigger numbers crash the game, but you can spawn 50 as many times as you like.
…ning in battle Basically fixed the Gaussian stat roll so the guardsmen ALWAYS spawn with positive health, making them a bit stronger, but definitely not too strong. They're still incredibly weak when compared to the original p_guardsman implementation
Added a new Guard squad mechanic. Instead of single entities - each guard squad represents 10 guardsmen. They take damage and "lose guardsmen" per each 40hp hit. cheat is "guardsquad X" for testing (x being the number.
Limits on HQ pool size removed. You can now huge numbers of guarrdsmen without either the game freezing or crashing. Still recommending to just spawn 1000 at a time. Use cheat "guardsman 1000" to spawn them, they will appear in your HQ company and should be on your home planet.
Basically you can now filter for guard units in your ships. to add guard to your planet press P and type in "guardsman 2000" or more. Or less. Not recommended going over 10k for now, as it can slow down post battle loading and takes a minute to load them in.
…irer force if you w ant
…me guardsmen logic For now the main way to recruit them is on your homeworld by pressing P for cheats and entering "guardsman 1000" for testing. You can get even morse guardsmen, i'll test how they work in combat.
Too long to describe what's implemented. Basically made working diplomacy (Buy 1000 guards for 50 req off of sector governor) added decimal unit counts (10 guard take up 1 space marine slot) and balanced their combat and survivability. Short version, lots of changes not mentioned. Needs testing and balance now.
Basically accidentally attached their damage and new cover mechanic values to old pnunit.guard instead of the current guard. Should work now as intended. Not overpowered, not incredibly weak, doing damage, also having melee. Also fixed some values with REQ and tried to set a limit on how many guardsmen you received from the Sector Governor via diplomacy. If that doesn't work then P "guardsman X" cheat is your other option
Now they should arrive lol the last patch biffed it
Also edited some other variables, but the base is that guardsmen didn't take splash damage from single weapons before as effectively. Also edited the cover system so it doesn't roll on melee.
Added the Steel legion guardsman portrait placeholder for now (i always loved their look) will replace them with a more popular variant based on a vote most likely.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
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
Adds the Leman Russ battle tank as an Astra Militarum Auxilia asset and corrects guardsman levy pricing. Leman Russ: - Purchasable from the Adeptus Mechanicus trade screen (faction 3), gated at 40 disposition, since the Mechanicus forge it. Not sold by the Sector Governor. - Musters into the Auxilia company (company 0) with the guardsmen and Chimeras, so it stays grouped with the levy and persists after battle. - New "Battle Cannon" player vehicle weapon (AP4 anti-armour with splash 8 anti-infantry), modelled on the existing enemy Battle Cannon; default loadout is Battle Cannon turret + hull Heavy Bolter. - Deploys to the armour (Predator) column with HP 300 (base x3) and armour 40, mirroring the enemy Leman Russ Battle Tank. - Registered deploy size and a post-battle recovery priority of 3 (above Chimera, below Marine heavies). - Costs 800 requisition each; purchase reduces Mechanicus disposition by 1 per tank (floor of 1). Guardsman cost fix: - The pure-guardsman levy had a hardcoded flat cost of 0.05 req each (50 per 1000), separate from the trade value. Bumped to 0.1 each, so 1000 guardsmen now cost 100 req. - Aligned the Guardsman trade value to 0.1 for consistency in mixed trades. - Disposition penalty unchanged (already scales at ~1 per 200 raised, i.e. 5 for 1000). Balance values are a tunable first pass.
Contributor
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)) { |
Contributor
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>
…, ship STC discount values do not match the panel, STC discounts apply to forge cost only Fixed Auxilia vehicles vanishing after combat (and on save/load), not destroyed, just silently erased. Root cause: - scr_vehicle_order rebuilds each company's vehicle arrays from a hardcoded whitelist of roles, resets every slot, then writes back only whitelisted vehicles. Chimera and Leman Russ were absent, so they were wiped. obj_ncombat Alarm_7 calls this on all companies 0-10 after every battle, so they despawned each fight regardless of which company they were in. - scr_start_load uses the same hardcoded vehicle list to distribute vehicles onto ships on load; Chimera and Leman Russ were missing, so a fleet-based chapter would lose them on save/reload. Fix: - Added "Chimera" and "Leman Russ" to the role whitelist in scr_vehicle_order. - Added them to the vehicle list in scr_start_load. Note for reviewers: these hardcoded vehicle whitelists silently delete any unlisted vehicle role post-battle. Consider consolidating them to a single source (e.g. the scr_unit_size vehicle map) so new vehicles cannot regress this way. Also fixed the STC issues reported by the player, so now they should accurately represent discounts.
Contributor
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
Fixed a random crash (obj_cursor Create reporting invalid -nan bounds on obj_controller) caused by fleet positions going NaN. Root cause: - obj_p_fleet Alarm_1 computed spid = point_distance(...) / action_eta. With a fleet at its destination and action_eta == 0, this is 0/0 = NaN, so the fleet x/y became NaN. obj_p_fleet Alarm_3 copies the fleet position into obj_controller.x/y, so the controller's bounding box became NaN and the engine crashed on the next room entry when obj_cursor was created. - obj_en_fleet Alarm_1 had the same divide (orbiting = dos / action_eta) with no ETA guard, which could NaN an enemy fleet position and crash collision/nearest checks. Fix: - Both fleet movement steps now divide by max(1, action_eta). When the ETA is exhausted the fleet moves its full remaining distance and arrives, instead of dividing by zero. Normal in-transit movement is unchanged. Also changed Raptor gene seed mutation to better match founding chapter.
Contributor
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); |
Contributor
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>
New combat improvements, really glitchy. Still Alpha.
Contributor
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
Comment on lines
+56
to
+57
| var _log_total = array_length(log_history); | ||
| var _log_start = (log_scroll <= 0) ? -1 : max(0, _log_total - log_view_lines - log_scroll); |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_ncombat/Draw_0.gml, line 56:
<comment>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.</comment>
<file context>
@@ -51,31 +51,75 @@ if ((display_p2 > 0) && (enemy_forces > 0)) {
+// When pinned to the bottom (log_scroll == 0) the live 45-row window is drawn exactly as before;
+// when scrolled up, page back through the retained log_history instead.
+var _log_total = array_length(log_history);
+var _log_start = (log_scroll <= 0) ? -1 : max(0, _log_total - log_view_lines - log_scroll);
+
</file context>
Suggested change
| var _log_total = array_length(log_history); | |
| var _log_start = (log_scroll <= 0) ? -1 : max(0, _log_total - log_view_lines - log_scroll); | |
| if (!variable_instance_exists(id, "log_history")) { | |
| log_history = []; | |
| log_view_lines = 45; | |
| log_scroll = 0; | |
| } | |
| var _log_total = array_length(log_history); | |
| var _log_start = (log_scroll <= 0) ? -1 : max(0, _log_total - log_view_lines - log_scroll); |
| // Mirror the freshly appended rows into the scrollback history (capped) so the log stays | ||
| // scrollable after lines[] rolls them off its 45-row live window. If the player is currently | ||
| // reading scrolled-up history, push their view down by the same amount to hold its position. | ||
| if (variable_instance_exists(id, "log_history")) { |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_newtext/scr_newtext.gml, line 27:
<comment>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.</comment>
<file context>
@@ -21,6 +21,21 @@ function scr_newtext() {
+ // Mirror the freshly appended rows into the scrollback history (capped) so the log stays
+ // scrollable after lines[] rolls them off its 45-row live window. If the player is currently
+ // reading scrolled-up history, push their view down by the same amount to hold its position.
+ if (variable_instance_exists(id, "log_history")) {
+ for (var _h = first_open; _h <= first_open + breaks - 1; _h++) {
+ array_push(log_history, { text: lines[_h], color: newline_color });
</file context>
Suggested change
| if (variable_instance_exists(id, "log_history")) { | |
| if (variable_instance_exists(id, "log_history") && variable_instance_exists(id, "log_history_max") && variable_instance_exists(id, "log_scroll") && variable_instance_exists(id, "log_view_lines")) { |
Impleneted the scrollable log made by Tavish in his LGW 1.2 Beta
Contributor
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
Implemented Tavish's combat changes, not as good as his because i had to do workarounds to make guardsmen work, but in essence still better than my older version. Not a permanent thing, just for testing how it works with guardsmen.
Contributor
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
| // Don't time out of stage 2 until the combat log has finished displaying - otherwise on a long turn | ||
| // the stage advances before `messages` drains and the "Enemy Forces at X%" status line is skipped. | ||
| // The large hard cap is anti-hang insurance in case the queue ever fails to drain. | ||
| if ((timer_stage == 2) && (((fugg > 60) && (messages == 0)) || (fugg > COMBAT_STAGE_TIMEOUT_FRAMES))) { |
Contributor
There was a problem hiding this comment.
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 exits every 60 frames.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_ncombat/Step_0.gml, line 143:
<comment>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.</comment>
<file context>
@@ -144,8 +137,11 @@ if (((fugg >= 60) || (fugg2 >= 60)) && (messages_shown == 0) && (messages_to_sho
+// Don't time out of stage 2 until the combat log has finished displaying - otherwise on a long turn
+// the stage advances before `messages` drains and the "Enemy Forces at X%" status line is skipped.
+// The large hard cap is anti-hang insurance in case the queue ever fails to drain.
+if ((timer_stage == 2) && (((fugg > 60) && (messages == 0)) || (fugg > COMBAT_STAGE_TIMEOUT_FRAMES))) {
+ timer_stage = 3;
}
</file context>
| continue; | ||
| } | ||
| for (var r = 1; r <= 30; r++) { | ||
| if (_f.dudes[r] == "" || _f.dudes_num[r] <= 0) { |
Contributor
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) { |
Contributor
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>
Suggested change
| 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, |
Contributor
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")) { |
Contributor
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>
| } | ||
| } | ||
| } | ||
| if (is_struct(mobi_item) && mobi_item.has_tag("bike")) { |
Contributor
There was a problem hiding this comment.
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.
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>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.</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>
| var _cast_word = (_e.casts == 1) ? "casting" : "castings"; | ||
| var _message = $"{_e.casts} {_cast_word} of '{_e.power}'{_e.flavour} {_e.kills} {_e.target} are {_e.verb}."; | ||
| var _size = _e.vehicle ? (_e.kills * 12) : (_e.kills * 3); | ||
| add_battle_log_message(_message, _size, 134); |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_powers/scr_powers.gml, line 373:
<comment>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.</comment>
<file context>
@@ -332,6 +336,47 @@ function scr_powers(caster_id) {
+ var _cast_word = (_e.casts == 1) ? "casting" : "castings";
+ var _message = $"{_e.casts} {_cast_word} of '{_e.power}'{_e.flavour} {_e.kills} {_e.target} are {_e.verb}.";
+ var _size = _e.vehicle ? (_e.kills * 12) : (_e.kills * 3);
+ add_battle_log_message(_message, _size, 134);
+ }
+ if (array_length(_keys) > 0) {
</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]; |
Contributor
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>
Volley fire by guard, guard divided into 100 units, added enemies being capable of choosing targets based on penetration prefarance (no more enemies shooting at tanks IF they're capable to shoot at infantry) and more.
Contributor
There was a problem hiding this comment.
6 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>
<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>
</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"]; |
Contributor
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") { |
Contributor
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) { |
Contributor
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 |
Contributor
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++) { |
Contributor
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>
| /// combat, the way the enemy's obj_enunit blocks already do. Fills a partial chunk first, | ||
| /// opens a fresh stack when the current one is full, and only as a last resort (all 71 stack | ||
| /// slots used) merges over the cap so fire is never silently dropped. | ||
| function find_capped_stack_index(weapon_name, cap) { |
Contributor
There was a problem hiding this comment.
P2: Capped Guardsman volley splitting can exhaust finite wep stack slots and cause later weapons to be silently skipped (-1 index path).
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 121:
<comment>Capped Guardsman volley splitting can exhaust finite `wep` stack slots and cause later weapons to be silently skipped (`-1` index path).</comment>
<file context>
@@ -111,6 +111,35 @@ function find_stack_index(weapon_name, head_role = false, unit = "none") {
+/// combat, the way the enemy's obj_enunit blocks already do. Fills a partial chunk first,
+/// opens a fresh stack when the current one is full, and only as a last resort (all 71 stack
+/// slots used) merges over the cap so fire is never silently dropped.
+function find_capped_stack_index(weapon_name, cap) {
+ // 1. a matching, un-led chunk that still has room
+ for (var si = 1; si < array_length(wep); si++) {
</file context>
For merging purposes
The upstream-merge commit kept the mod's combat/log loop bodies but resolved the surrounding declaration blocks toward the refactor, dropping the i initializers in obj_ncombat\Alarm_3 (crash on combat start: i not set before reading) and risking the same in other mod-owned files. Restore obj_ncombat\Alarm_3, Draw_0, Step_0, obj_pnunit\Alarm_3, obj_enunit\Alarm_0, obj_ini\Create_0, scr_flavor2, scr_player_combat_weapon_stacks, and scr_shoot to their committed versions. The refactor only reformatted these; the mod owns their logic. Upstream sync content is unaffected (auto-merged files, scr_PlanetData events, scr_marine_struct and obj_p_fleet blends remain).
Contributor
There was a problem hiding this comment.
13 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_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_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="objects/obj_fleet/Mouse_56.gml">
<violation number="1" location="objects/obj_fleet/Mouse_56.gml:43">
P2: The speed-increase guard checks `game_get_speed(gamespeed_fps) < 90` but then unconditionally adds 30, so any current speed between 61 and 89 will overshoot the 90 cap.</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="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_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="objects/obj_temp_build/Create_0.gml">
<violation number="1" location="objects/obj_temp_build/Create_0.gml:2">
P2: `planet` is self-referenced during Create and falls back to `0` instead of the previous `noone` sentinel, risking invalid downstream planet handling.</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
| var nearest_star = instance_nearest(x, y, obj_star); | ||
| orbiting = nearest_star; | ||
| // LOGGER.debug($"p_fleet id {id} deserialized: {self}"); | ||
| if (save_data.orbiting != noone && action == "") { |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_en_fleet/Create_0.gml, line 134:
<comment>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.</comment>
<file context>
@@ -116,26 +116,23 @@ deserialize = function(save_data) {
- var nearest_star = instance_nearest(x, y, obj_star);
- orbiting = nearest_star;
- // LOGGER.debug($"p_fleet id {id} deserialized: {self}");
+ if (save_data.orbiting != noone && action == "") {
+ orbiting = instance_nearest(x, y, obj_star);
}
</file context>
Suggested change
| if (save_data.orbiting != noone && action == "") { | |
| if (save_data.orbiting != noone && save_data.orbiting != 0 && action == "") { |
|
|
||
| //draw_sprite(sprite_index, 0, x, y); | ||
| if (dummy_marine == false) { | ||
| if (dummy_marine ?? true) { |
Contributor
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>
Suggested change
| 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; |
Contributor
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>
| /// @param {bool} include_trainee Whether to include trainee roles (default is false). | ||
| /// @param {bool} include_heads Whether to include head roles (default is true). | ||
| /// @returns {array} | ||
| /// @param {Real} group The group of roles to retrieve (e.g., SPECIALISTS_STANDARD, SPECIALISTS_LIBRARIANS). |
Contributor
There was a problem hiding this comment.
P2: JSDoc @param {Real} group in role_groups() is incorrect — callers must pass string macro constants (e.g. SPECIALISTS_STANDARD = "standard"), not numeric values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/is_specialist/is_specialist.gml, line 25:
<comment>JSDoc `@param {Real} group` in `role_groups()` is incorrect — callers must pass string macro constants (e.g. SPECIALISTS_STANDARD = "standard"), not numeric values.</comment>
<file context>
@@ -15,17 +15,17 @@
-/// @param {bool} include_trainee Whether to include trainee roles (default is false).
-/// @param {bool} include_heads Whether to include head roles (default is true).
-/// @returns {array}
+/// @param {Real} group The group of roles to retrieve (e.g., SPECIALISTS_STANDARD, SPECIALISTS_LIBRARIANS).
+/// @param {Bool} include_trainee Whether to include trainee roles (default is false).
+/// @param {Bool} include_heads Whether to include head roles (default is true).
</file context>
Suggested change
| /// @param {Real} group The group of roles to retrieve (e.g., SPECIALISTS_STANDARD, SPECIALISTS_LIBRARIANS). | |
| /// @param {String} group The group of roles to retrieve (e.g., SPECIALISTS_STANDARD, SPECIALISTS_LIBRARIANS). |
| 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); |
Contributor
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); |
Contributor
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); |
Contributor
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>
Suggested change
| 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. |
Contributor
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>
Suggested change
| /// @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; |
Contributor
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>
Fixed crash that was happening at random
Contributor
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="objects/obj_enunit/Alarm_0.gml">
<violation number="1" location="objects/obj_enunit/Alarm_0.gml:112">
P2: New duplicate screening algorithm in AP branch duplicates existing non-AP logic, increasing divergence/regression risk.</violation>
<violation number="2" location="objects/obj_enunit/Alarm_0.gml:113">
P2: Custom agent: **Code Quality Review**
New AP weapon target selection adds excessive nesting (more than 3 control structures deep). The screening logic inside `if (block_has_armour(enemy2))` introduces additional `if`/`else`/`if` levels on top of an already deeply nested combat function (reaching ~10 cumulative nesting levels from the function root).</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="scripts/scr_marine_struct/scr_marine_struct.gml">
<violation number="1" location="scripts/scr_marine_struct/scr_marine_struct.gml:1926">
P2: `home_planet` spawn path unconditionally returns and skips the existing fleet fallback when no valid planet is found, potentially leaving a stale `planet_location` index.</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_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>
<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_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="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_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**
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>
<violation number="3" 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/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>
<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_fleet/Mouse_56.gml">
<violation number="1" location="objects/obj_fleet/Mouse_56.gml:43">
P2: The speed-increase guard checks `game_get_speed(gamespeed_fps) < 90` but then unconditionally adds 30, so any current speed between 61 and 89 will overshoot the 90 cap.</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++) { |
Contributor
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>
…uard button obj_temp_build is created via instance_create (Create runs immediately) with planet assigned only on the following line, so Create's line-2 planet read hit an unset variable and aborted before isnew was set. scr_menu_clear_up then crashed reading isnew on the half-built instance. Guard the planet read in Create so it always completes, and guard the isnew read in scr_menu_clear_up with variable_instance_exists as a backstop. Normal building is unchanged; planet is still set right after creation. Repurpose the obsolete Recruit Guard button so it raises real units instead of filling the abstract p_guardsmen pool. On a player-owned world with at least 1000 PDF, one click draws 1000 from that planet's PDF and spawns 1000 individual Guardsman units at the home planet via scr_add_man, the same path the Sector Governor uses. Keeps the 50 requisition cost. Adds an owner re-check in the handler and runs the spawn loop under obj_controller scope. Guardsmen only: no Sergeants, Heavy Weapons Teams or vehicles.
calculate_fleet_eta only computed the warp-lane multiplier when star1/star2 were both omitted (the map preview path). Every real-travel caller passes a boolean (from_star, is_orbiting(), _target_is_sys, true/false) in those slots, so warp_lane stayed 0: lanes gave no speed bonus, the no-lane x2 penalty always applied, and the destination warp-storm check was skipped via an early return. Resolve both endpoints to real stars from the coordinates regardless of the passed args, compute the lane, apply bonus/penalty, and always run the storm check. Fixes all fleet callers so real transit matches the preview and lane routes are actually faster. Upstream has the same bug.
A fleet whose x/y went NaN before the action_eta divide-by-zero fix stays NaN (point_distance keeps propagating it) and gets copied into the persistent obj_controller via the fleet-focus path. obj_controller then carries the NaN across rooms and crashes the collision grid when obj_cursor is recreated (invalid -nan bound). scr_load now sanitizes NaN x/y on obj_controller and all fleets at load time, snapping broken instances to a valid star, and the obj_p_fleet Alarm_3 focus copy only runs when the fleet position is finite, so a NaN can never reach the controller again. Repairs existing affected saves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds Imperial Guard Auxilia as fully recruitable, embarkable, and deployable ground forces with full combat, UI, and ship-capacity support. Also improves combat flow with staggered Guard volley fire, retained/scrollable log history, smarter enemy target selection by weapon role, and multiple stability fixes including a load-time fix for a rare NaN fleet-position crash when switching rooms.
New Features
Battle Cannon+Heavy Bolter; deploys to armour column, participates in recovery; IG capacity and embark UI updated.Bug Fixes
obj_temp_build; guarded planet reads in Create.calculate_fleet_etaso real travel applies warp‑lane bonuses/penalties (not just preview); replaced-50sentinels withnooneacross fleets/ships.obj_controllerwhen focusing a fleet.Written for commit ef73a75. Summary will update on new commits.