Conversation
There was a problem hiding this comment.
40 issues found across 16 files
Confidence score: 1/5
- The highest risk is widespread compile-time breakage in
scripts/MissionHelper/MissionHelper.gmlandscripts/scr_mission_functions/scr_mission_functions.gml: malformed switches, invalid:syntax, duplicate cases, and an unclosed function can prevent the scripts from parsing—restore the syntax and verify the project builds. - Runtime problem handling is also likely to fail in
scripts/scr_enemy_ai_d/scr_enemy_ai_d.gmlandscripts/scr_PlanetData/scr_PlanetData.gmlbecause code references undefined or wrong variables and deletes array elements at index0; correct the names and deletion index before exercising turn-end and expiry flows. MissionHelperconstructors leavetimer,data, andplanetunset due to parameter shadowing, whilescripts/scr_inquisition_mission/scr_inquisition_mission.gmlpasses the wrong estimate field; initialize the instance fields explicitly and usepop_data.estimateto avoid broken countdowns and prematurely due missions.
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_enemy_ai_d/scr_enemy_ai_d.gml">
<violation number="1" location="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml:84">
P0: `p_problems` is never defined anywhere in the codebase (the actual parallel array is `p_problem`, initialized in objects/obj_star/Create_0.gml and populated via PlanetData.register_problem). Reading the undefined instance variable `p_problems` throws a runtime error on this line on every star step, so the entire problem-processing loop is dead before it runs. This looks like a typo for `p_problem[i]`.</violation>
<violation number="2" location="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml:87">
P1: `problem_count_down` was removed from scr_mission_functions (where it was a global, obj_star-scoped function) and is now a method bound to the PlanetData struct (scripts/scr_PlanetData/scr_PlanetData.gml:790, `function problem_count_down(...)` inside the constructor). scr_enemy_ai_d() runs with self = obj_star, which no longer has this method, so this call raises an unknown-function/variable error. Call it on the PlanetData instance instead (it is already fetched as `_pdata` on the next line).</violation>
<violation number="3" location="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml:95">
P1: Custom agent: **Code Quality Review**
The removed end-turn path has no working equivalent: `PlanetProblem.basic_turn_end()` does not normally decrement timers and dispatches `hunt_beast` to the undefined `resolve_hunt_beast`. Preserve the legacy countdown, storm adjustment, and completion mappings in the new mission object.</violation>
</file>
<file name="scripts/scr_mission_functions/scr_mission_functions.gml">
<violation number="1" location="scripts/scr_mission_functions/scr_mission_functions.gml:91">
P0: Custom agent: **Code Quality Review**
Deleting the closing brace leaves `problem_end_turn_checks` unclosed, so the script is malformed and the following function declarations are not reliably parsed. Restore the closing brace after the function, or remove the obsolete declaration entirely.</violation>
</file>
<file name="scripts/MissionHelper/MissionHelper.gml">
<violation number="1" location="scripts/MissionHelper/MissionHelper.gml:8">
P0: The constructor shadows its instance fields, so `timer = timer` and `data = data` assign the arguments to themselves. Every problem then lacks the fields used by countdown and mission callbacks; rename the parameters or assign through `self`.</violation>
<violation number="2" location="scripts/MissionHelper/MissionHelper.gml:8">
P0: The constructor never initializes the `timer`, `data`, and `planet` struct members because those names collide with the constructor parameters. In GML, constructor arguments are local variables, so `timer = timer;`, `data = data;`, and `planet = planet;` each assign the local to itself and never create a member on the struct (the docs require `self.timer = timer;` for same-name members). Every static method that reads `timer`, `data`, or `planet` (`basic_turn_end`, `refresh_p_data`, `increment_mission_completion`, `per_turn_check_mech_*`, etc.) then reads an unset member, so the whole PlanetProblem object is broken. Use `self.` on these three assignments.</violation>
<violation number="3" location="scripts/MissionHelper/MissionHelper.gml:8">
P1: Custom agent: **Code Quality Review**
The constructor never stores the mission timer or data on the generated struct. Because the arguments shadow the intended instance fields, both assignments are self-assignment no-ops; use `self.timer = timer` and `self.data = data` (or rename the arguments) so `basic_turn_end` and completion tracking can access the state.</violation>
<violation number="4" location="scripts/MissionHelper/MissionHelper.gml:16">
P2: Custom agent: **Code Quality Review**
This new GML file uses tab indentation in many blocks, violating CODE_STYLE.md's requirement to use 4 spaces and avoid tabs. Replace leading tabs throughout the file with four spaces.</violation>
<violation number="5" location="scripts/MissionHelper/MissionHelper.gml:35">
P0: The per-turn dispatch switch (lines 34-50) is syntactically invalid and will not compile, breaking the whole build. `case "mech_raider";` and `case "awaiting_player";` use semicolons instead of colons, `switch(stage_id):` (line 43) uses a colon instead of braces with a stray `break;` immediately after, and `case "mech_bionics":` is duplicated (lines 39 and 42). Separately, line 36 assigns `_func = per_turn_check_raider_failed;`, but that function is never defined anywhere in the repo (only `per_turn_check_mech_raider` exists at line 470), so the mech_raider per-turn check would silently never run even if the syntax were fixed. Rewrite the switch with proper `case X:` labels and a single `mech_bionics` branch dispatching on `stage_id`, mapping `mech_raider` to `per_turn_check_mech_raider`.</violation>
<violation number="6" location="scripts/MissionHelper/MissionHelper.gml:36">
P1: The raider per-turn case references an undefined handler, so raider progress cannot run. Assign `per_turn_check_mech_raider` instead.</violation>
<violation number="7" location="scripts/MissionHelper/MissionHelper.gml:42">
P0: Custom agent: **Code Quality Review**
The added `basic_turn_end` switch cannot compile: it duplicates `case "mech_bionics"` and writes the nested switch as `switch(stage_id):` instead of `switch (stage_id) {`. Replace the nested switch with valid brace syntax, remove the duplicate outer case, and place `break;` only within a case.</violation>
<violation number="8" location="scripts/MissionHelper/MissionHelper.gml:61">
P1: This zero-timer branch invokes one-shot resolvers without retiring the problem. Penalties, popups, and rewards repeat every turn while `timer == 0`; disable the checks or remove the problem after each resolver.</violation>
<violation number="9" location="scripts/MissionHelper/MissionHelper.gml:65">
P1: The `hunt_beast` expiry case references an undefined callback, so the mission cannot complete at expiry. Dispatch to `complete_beast_hunt_mission`.</violation>
<violation number="10" location="scripts/MissionHelper/MissionHelper.gml:158">
P1: This call uses `man_conditions` before its declaration and initialization. Move the conditions struct before `collect_role_group`, otherwise beast-hunt completion receives undefined filters.</violation>
<violation number="11" location="scripts/MissionHelper/MissionHelper.gml:325">
P1: When no trainer remains on the planet, `_trainer` is still an array but this line accesses `.job`. Guard the cleanup with a unit check.</violation>
<violation number="12" location="scripts/MissionHelper/MissionHelper.gml:479">
P1: When the raider mission completes, this passes `id` instead of the stored star object. `scr_mission_reward` reads `star.name`, so completion fails; pass `system` or the correct star instance.</violation>
<violation number="13" location="scripts/MissionHelper/MissionHelper.gml:599">
P0: Custom agent: **Code Quality Review**
This added statement uses `:` outside a struct literal, so the GML script cannot compile. Assign the mission counter with `=` before `per_turn_check_mech_tomb2` increments it.</violation>
<violation number="14" location="scripts/MissionHelper/MissionHelper.gml:599">
P1: `data.turns : 0;` uses a colon instead of an assignment operator and is a syntax error that will break compilation. Replace it with `data.turns = 0;` (the intent is to reset the tomb-research turn counter when the exploring stage begins).</violation>
</file>
<file name="scripts/scr_PlanetData/scr_PlanetData.gml">
<violation number="1" location="scripts/scr_PlanetData/scr_PlanetData.gml:777">
P1: `new_problem` changes collection entries from string IDs to `PlanetProblem` structs, but this file’s lookup and removal methods still use helpers that compare entries directly with strings. After registration is fixed, mission checks and resolution cleanup will not find these objects; make those helpers compare `p_id` or use object-aware operations.</violation>
<violation number="2" location="scripts/scr_PlanetData/scr_PlanetData.gml:785">
P1: Every default `new_problem` call enters `register_problem`, but `p_problem` is not a field of this `PlanetData` struct. Push into `system.p_problem[planet]` so succession and fallen problems are actually registered.</violation>
<violation number="3" location="scripts/scr_PlanetData/scr_PlanetData.gml:785">
P1: Custom agent: **Code Quality Review**
When `new_problem()` uses its default `register = true`, `register_problem()` evaluates `p_problem`, but `PlanetData` never initializes that variable. This can fail before registering the new problem; push to `system.p_problem[planet]` (or the correctly scoped `problems` array) instead.</violation>
<violation number="4" location="scripts/scr_PlanetData/scr_PlanetData.gml:785">
P1: `register_problem` pushes to a bare `p_problem` that is not an instance variable of PlanetData (grep shows it only at lines 785-786 in this file), so it throws. It should push into `system.p_problem[planet]`. Additionally, because obj_star now initializes `p_problem` as a flat `[]`, `system.p_problem[planet]` needs per-planet array initialization (`array_create_advanced(_planet_array_size, [])`) or `array_length(problems)` in `problem_count_down` will operate on an undefined/non-array value.</violation>
<violation number="5" location="scripts/scr_PlanetData/scr_PlanetData.gml:794">
P0: Custom agent: **Code Quality Review**
When a problem expires, this function checks the undeclared `problem` instead of `_problem`, then deletes from `system.p_problem[i]` and requests deletion of zero elements. The turn loop calls this for every planet, so expired `PlanetProblem` entries either trigger an undefined-variable error or remain registered. Use `_problem.timer`, `system.p_problem[planet]`, and a deletion count of `1`.</violation>
<violation number="6" location="scripts/scr_PlanetData/scr_PlanetData.gml:794">
P1: The loop item is `_problem`, but this condition reads the undeclared `problem` value. On countdown, use `_problem.timer` or expired problems will error or fail to be removed.</violation>
<violation number="7" location="scripts/scr_PlanetData/scr_PlanetData.gml:794">
P0: `problem_count_down` reads `problem.timer`, but the loop variable is `_problem`; `problem` is undefined in this scope and throws at runtime whenever a planet has a registered problem. Also both `array_delete(..., 0)` calls pass amount 0, so expired problems are never actually removed. Use the `_problem` variable and the `remove` flag that `basic_turn_end()` maintains, deleting 1 element.</violation>
<violation number="8" location="scripts/scr_PlanetData/scr_PlanetData.gml:795">
P1: After the timer check is fixed, this cleanup still indexes `system.p_problem` with the problem slot and requests deletion of zero entries. Delete one entry from the current planet collection once.</violation>
</file>
<file name="scripts/scr_inquisition_mission/scr_inquisition_mission.gml">
<violation number="1" location="scripts/scr_inquisition_mission/scr_inquisition_mission.gml:229">
P1: When the saved target system no longer exists, this line calls `get_planet_data` on `noone` before the existing guard can close the popup. Move the planet lookup below the `mission_star == noone` check.</violation>
<violation number="2" location="scripts/scr_inquisition_mission/scr_inquisition_mission.gml:251">
P1: For these structured popups, `estimate` is the popup field initialized to `0`, not `pop_data.estimate`. Passing it registers the Necron problem as due immediately; pass `pop_data.estimate` so the mission uses its advertised ETA.</violation>
</file>
<file name="objects/obj_popup/Step_0.gml">
<violation number="1" location="objects/obj_popup/Step_0.gml:215">
P1: `new_problem` exists only as a static method on PlanetData; there is no global `new_problem` function, so this bare call crashes and the recon mission is never registered. Call it on the planet-data struct: `_pdata.new_problem("recon", estimate)`. The assigned `_problem` is unused and can be dropped.</violation>
<violation number="2" location="objects/obj_popup/Step_0.gml:237">
P1: For every valid Inquisition planet mission, this call enters `PlanetData.register_problem`, which currently appends to the nonexistent `p_problem` field and throws. Fix the registration method to append the object to the target planet's problem collection before relying on this call; otherwise missions are never accepted.</violation>
</file>
<file name="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml">
<violation number="1" location="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml:146">
P1: A newly accepted tomb mission never enters `per_turn_check_mech_tomb1` because the OOP dispatcher has no `mech_tomb` case. It therefore expires into the failure handler after 17 months instead of waiting for the required Astartes and starting the research; add the tomb dispatch case before using this new problem ID.</violation>
<violation number="2" location="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml:146">
P1: Every mission acceptance path now fails while registering the new `PlanetProblem`: `register_problem` pushes to an undefined `p_problem` instead of this planet's problem array. Register the object in `system.p_problem[planet]` (or the `problems` alias) before switching these callers to `new_problem`.</violation>
<violation number="3" location="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml:171">
P1: After registration is fixed, these object-backed missions still cannot advance because `problem_count_down` dereferences the undefined `problem` name after running `_problem.basic_turn_end()`. Fix the shared loop to use the same `_problem` object before enabling these new callers.</violation>
</file>
<file name="scripts/scr_destroy_planet/scr_destroy_planet.gml">
<violation number="1" location="scripts/scr_destroy_planet/scr_destroy_planet.gml:167">
P1: Custom agent: **Code Quality Review**
When a planet is created or destroyed, `p_timer` and `p_problem_other_data` are no longer initialized, but mission and planet-data code still indexes both arrays. Restore their initialization (or migrate every remaining access) before resetting `p_problem`, otherwise those paths can read or write undefined arrays.</violation>
</file>
<file name="scripts/scr_crusade/scr_crusade.gml">
<violation number="1" location="scripts/scr_crusade/scr_crusade.gml:236">
P1: This invokes the default registration path, which references undefined `p_problem` in `PlanetData.register_problem`, so the crusade cannot be registered. Fix `register_problem` to append to `system.p_problem[planet]` before using this call.</violation>
<violation number="2" location="scripts/scr_crusade/scr_crusade.gml:236">
P1: This passes the star instance as `PlanetProblem.data`, but the new API expects a struct payload and immediately inspects it as one. Omit the third argument; `_p_data` already identifies the crusade location.</violation>
</file>
<file name="scripts/scr_chaos_alliance_test/scr_chaos_alliance_test.gml">
<violation number="1" location="scripts/scr_chaos_alliance_test/scr_chaos_alliance_test.gml:98">
P1: When the alliance roll produces `success_trap`, this call creates `meeting_trap` but no longer emits the response, event log, or purple marker. Handle `meeting_trap` in the problem initialization or restore the shared side effects for both outcomes.</violation>
</file>
<file name="objects/obj_star/Create_0.gml">
<violation number="1" location="objects/obj_star/Create_0.gml:70">
P1: Removing `p_problem_other_data` and `p_timer` from obj_star while scr_PlanetData's `refresh_data()` (lines 75-76) and the mission-display code (line 1429) still read `system.p_problem_other_data` / `system.p_timer` will throw on every planet refresh. The refactor is incomplete: scr_mission_functions.gml, scr_inquisition_mission.gml, scr_random_event.gml, and scr_enemy_ai_d.gml still reference the removed arrays and the old 2D layout, so the declarations should not be dropped until all consumers are migrated.</violation>
</file>
<file name="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml">
<violation number="1" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:268">
P1: `mission_name_key(problems[p])` now receives a PlanetProblem object instead of the mission key string, so `struct_exists(mission_key, object)` never matches and it always returns "none". Every mission is silently filtered out and the mission log renders empty. Pass the object's key instead: `mission_name_key(_problem.p_id)`.</violation>
<violation number="2" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:269">
P2: When `p_problem` contains an empty slot or a legacy string, `_problem.stage_id` throws before the mission log is rebuilt. Retain a type/empty guard here or migrate every writer before dereferencing OOP fields.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "succession": function(problem_index) { | ||
| if (problem_timers[problem_index] > 0) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
P0: Custom agent: Code Quality Review
Deleting the closing brace leaves problem_end_turn_checks unclosed, so the script is malformed and the following function declarations are not reliably parsed. Restore the closing brace after the function, or remove the obsolete declaration entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_mission_functions/scr_mission_functions.gml, line 91:
<comment>Deleting the closing brace leaves `problem_end_turn_checks` unclosed, so the script is malformed and the following function declarations are not reliably parsed. Restore the closing brace after the function, or remove the obsolete declaration entirely.</comment>
<file context>
@@ -17,7 +17,7 @@ global.planet_problem_keys = [
"mech_raider",
"mech_bionics",
"mech_mars",
- "mech_tomb1",
+ "mech_tomb",
"fallen",
"great_crusade",
"harlequins",
@@ -42,14 +42,14 @@ global.planet_problem_keys = [
</file context>
| _func = per_turn_check_mech_bionics; | ||
| break; | ||
| case "mech_bionics": | ||
| switch(stage_id): |
There was a problem hiding this comment.
P0: Custom agent: Code Quality Review
The added basic_turn_end switch cannot compile: it duplicates case "mech_bionics" and writes the nested switch as switch(stage_id): instead of switch (stage_id) {. Replace the nested switch with valid brace syntax, remove the duplicate outer case, and place break; only within a case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 42:
<comment>The added `basic_turn_end` switch cannot compile: it duplicates `case "mech_bionics"` and writes the nested switch as `switch(stage_id):` instead of `switch (stage_id) {`. Replace the nested switch with valid brace syntax, remove the duplicate outer case, and place `break;` only within a case.</comment>
<file context>
@@ -0,0 +1,617 @@
+ _func = per_turn_check_mech_bionics;
+ break;
+ case "mech_bionics":
+ switch(stage_id):
+ break;
+ case "exploring":
</file context>
| if (array_length(_marines) >= 20) { | ||
| stage_id = "exploring"; | ||
| timer = 999; | ||
| data.turns : 0; |
There was a problem hiding this comment.
P0: Custom agent: Code Quality Review
This added statement uses : outside a struct literal, so the GML script cannot compile. Assign the mission counter with = before per_turn_check_mech_tomb2 increments it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 599:
<comment>This added statement uses `:` outside a struct literal, so the GML script cannot compile. Assign the mission counter with `=` before `per_turn_check_mech_tomb2` increments it.</comment>
<file context>
@@ -0,0 +1,617 @@
+ if (array_length(_marines) >= 20) {
+ stage_id = "exploring";
+ timer = 999;
+ data.turns : 0;
+ scr_popup("Mechanicus Research", $"The Mechanicus Research team on planet {p_data.name()} has taken note of your Astartes and are now prepared to begin their research. Your marines are to stay on the planet until further notice.", "necron_cave", "");
+ }
</file context>
| data.turns : 0; | |
| data.turns = 0; |
| function SystemProblem(name, timer, data){} | ||
|
|
||
| function PlanetProblem(name, timer, data, planet) constructor{ | ||
| timer = timer; |
There was a problem hiding this comment.
P0: The constructor shadows its instance fields, so timer = timer and data = data assign the arguments to themselves. Every problem then lacks the fields used by countdown and mission callbacks; rename the parameters or assign through self.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 8:
<comment>The constructor shadows its instance fields, so `timer = timer` and `data = data` assign the arguments to themselves. Every problem then lacks the fields used by countdown and mission callbacks; rename the parameters or assign through `self`.</comment>
<file context>
@@ -0,0 +1,617 @@
+function SystemProblem(name, timer, data){}
+
+function PlanetProblem(name, timer, data, planet) constructor{
+timer = timer;
+uid = scr_uuid_generate();
+p_id = name;
</file context>
|
|
||
| static complete_beast_hunt_mission = function() { | ||
| refresh_p_data(); | ||
| var _hunters = collect_role_group("all", [system.name, planet, 0], false, man_conditions); |
There was a problem hiding this comment.
P1: This call uses man_conditions before its declaration and initialization. Move the conditions struct before collect_role_group, otherwise beast-hunt completion receives undefined filters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 158:
<comment>This call uses `man_conditions` before its declaration and initialization. Move the conditions struct before `collect_role_group`, otherwise beast-hunt completion receives undefined filters.</comment>
<file context>
@@ -0,0 +1,617 @@
+
+static complete_beast_hunt_mission = function() {
+ refresh_p_data();
+ var _hunters = collect_role_group("all", [system.name, planet, 0], false, man_conditions);
+ if (stage_id == "active") {
+ var _mission_string = "";
</file context>
| } | ||
|
|
||
| static register_problem = function(problem){ | ||
| array_push(p_problem, problem); |
There was a problem hiding this comment.
P1: register_problem pushes to a bare p_problem that is not an instance variable of PlanetData (grep shows it only at lines 785-786 in this file), so it throws. It should push into system.p_problem[planet]. Additionally, because obj_star now initializes p_problem as a flat [], system.p_problem[planet] needs per-planet array initialization (array_create_advanced(_planet_array_size, [])) or array_length(problems) in problem_count_down will operate on an undefined/non-array value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_PlanetData/scr_PlanetData.gml, line 785:
<comment>`register_problem` pushes to a bare `p_problem` that is not an instance variable of PlanetData (grep shows it only at lines 785-786 in this file), so it throws. It should push into `system.p_problem[planet]`. Additionally, because obj_star now initializes `p_problem` as a flat `[]`, `system.p_problem[planet]` needs per-planet array initialization (`array_create_advanced(_planet_array_size, [])`) or `array_length(problems)` in `problem_count_down` will operate on an undefined/non-array value.</comment>
<file context>
@@ -775,9 +773,30 @@ function PlanetData(_planet, _system) constructor {
+ }
+
+ static register_problem = function(problem){
+ array_push(p_problem, problem);
+ problems = system.p_problem[planet];
+ }
</file context>
| @@ -0,0 +1,617 @@ | |||
| function MissionHelper() constructor{ | |||
There was a problem hiding this comment.
P2: Custom agent: Code Quality Review
This new GML file uses tab indentation in many blocks, violating CODE_STYLE.md's requirement to use 4 spaces and avoid tabs. Replace leading tabs throughout the file with four spaces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 16:
<comment>This new GML file uses tab indentation in many blocks, violating CODE_STYLE.md's requirement to use 4 spaces and avoid tabs. Replace leading tabs throughout the file with four spaces.</comment>
<file context>
@@ -0,0 +1,617 @@
+planet = p_data.planet;
+system = p_data.system;
+static refresh_p_data = function(){
+ p_data = system.get_planet_data(planet);
+}
+stage_id = "";
</file context>
| } | ||
| if (problem_has_key_and_value(i, p, "stage", "preliminary")) { | ||
| var _problem = problems[p]; | ||
| if (_problem.stage_id == "preliminary") { |
There was a problem hiding this comment.
P2: When p_problem contains an empty slot or a legacy string, _problem.stage_id throws before the mission log is rebuilt. Retain a type/empty guard here or migrate every writer before dereferencing OOP fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml, line 269:
<comment>When `p_problem` contains an empty slot or a legacy string, `_problem.stage_id` throws before the mission log is rebuilt. Retain a type/empty guard here or migrate every writer before dereferencing OOP fields.</comment>
<file context>
@@ -265,20 +265,19 @@ function UnitQuickFindPanel() constructor {
- }
- if (problem_has_key_and_value(i, p, "stage", "preliminary")) {
+ var _problem = problems[p];
+ if (_problem.stage_id == "preliminary") {
continue;
}
</file context>
| if (_problem.stage_id == "preliminary") { | |
| if (!is_struct(_problem) || _problem.stage_id == "preliminary") { |
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Migrates mission initialization and completion logic into MissionHelper as methods, encapsulating mission state and behavior directly within mission objects. This change removes the reliance on parallel data arrays within PlanetData and scr_mission_functions, improving modularity and simplifying mission management by allowing direct method calls on mission instances.
Removes the global `add_new_problem` function, delegating problem addition to `PlanetData` objects via a new `add_problem` method. This change further encapsulates planet-specific state and behavior, reducing reliance on global functions and parallel data arrays. Updates `scr_inquisition_mission`, `scr_new_governor_mission`, and `scr_random_event` (Harlequins) to use the new `PlanetData` method. Additionally, centralizes Harlequin event popup and star marker logic within `MissionHelper`.
Enhances the encapsulation of planet problem logic by implementing direct management of problem objects within PlanetData methods. Removes obsolete global helper functions (`find_problem_planet`, `remove_planet_problem`, `open_problem_slot`, etc.) and updates call sites to utilize the PlanetData object's dedicated methods. This change further solidifies the object-oriented approach for handling planet problems, moving away from parallel arrays and external helper functions.
Introduces `new_end_turn_battle` in `MissionHelper` to abstract and centralize battle queuing. This new helper enables passing richer, structured data (via `special_feature`) to combat encounters. The `battle_mission` variable is deprecated and consolidated into `battle_special` to streamline battle-related context. Migrates per-turn battle logic from `scr_enemy_ai_e` into `MissionHelper`, enhancing modularity and aligning with the ongoing refactoring towards object-oriented design and better data encapsulation.
Moves pre-battle squad selection, in-battle special effects, and post-battle resolution logic from global scripts and `obj_ncombat` alarms into dedicated methods on `PlanetProblem` instances within `MissionHelper`. This significantly improves encapsulation by associating battle-related mission behavior directly with the `PlanetProblem` object (via `special_feature`), rather than relying on `battle_special` string parsing or scattered global functions. It also simplifies `obj_ncombat` by delegating special mission aftermath handling.
Moves hardcoded enemy definitions from `obj_ncombat` to a structured `battle_enemy_data` object, defined in `MissionHelper`. This allows mission-specific enemy forces to be defined declaratively, increasing flexibility and reducing reliance on conditional logic within the combat system. Introduces the `add_enemies` function in `obj_enunit` to dynamically populate units from this structured data. This continues the refactoring towards a more data-driven and encapsulated battle system.
Moves specific mission-based enemy definitions and combat flow logic from `obj_ncombat` and global scripts into `MissionHelper`. This includes the Necron Tomb, Fallen, and Spyrer encounters. Combat encounters now receive detailed enemy setups via the `battle_enemy_data` object, and pre/post-battle narratives are driven by `special_feature.data` flags. This greatly reduces reliance on `battle_special` string parsing, improving encapsulation and enabling a more flexible, data-driven approach to mission design.
There was a problem hiding this comment.
14 issues found across 28 files
Confidence score: 1/5
scripts/MissionHelper/MissionHelper.gmlhas malformed syntax, including invalid declaration keywords and assignment syntax, so the script cannot compile and the new mission objects cannot run — correct the syntax throughout the file.scripts/scr_enemy_ai_d/scr_enemy_ai_d.gmlcallsproblem_count_downas a global function even though it is only an instance method; separately,scripts/scr_enemy_ai_e/scr_enemy_ai_e.gmlomits enemy-turn and Necron tomb-raid dispatch, so enemy mission behavior can fail or be skipped — wire these calls to their valid implementations.objects/obj_star/Create_0.gmlandscripts/scr_destroy_planet/scr_destroy_planet.gmlno longer initializep_timerandp_problem_other_data, while live consumers still access them, risking undefined-variable failures — migrate those consumers toPlanetProblemor preserve the arrays.objects/obj_ncombat/Alarm_0.gmlcalls an undefined helper, andobjects/obj_enunit/Create_0.gmlstarts enemy insertion at index 1001 beyond the initialized range; custom enemy battles can abort or process no enemies — use the defined helper and scan for the first empty slot.
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_star/Create_0.gml">
<violation number="1" location="objects/obj_star/Create_0.gml:70">
P1: Custom agent: **Code Quality Review**
`p_timer` and `p_problem_other_data` are removed here while live code still reads and writes them, causing undefined-variable failures during enemy turns and inquisitor mission resolution. Migrate those consumers to the new mission objects and add a legacy save migration before removing these fields.</violation>
</file>
<file name="objects/obj_ncombat/Alarm_0.gml">
<violation number="1" location="objects/obj_ncombat/Alarm_0.gml:136">
P1: This misspelled helper is undefined, so custom enemy-data battles abort before creating their forces. Call `move_data_to_current_scope` instead.</violation>
</file>
<file name="scripts/scr_enemy_ai_e/scr_enemy_ai_e.gml">
<violation number="1" location="scripts/scr_enemy_ai_e/scr_enemy_ai_e.gml:531">
P1: Custom agent: **Code Quality Review**
The deleted enemy-turn path is not fully migrated: `per_turn_check_fallen` is never dispatched, and no replacement calls `setup_necron_tomb_raid` for `necron` missions. Add the missing `PlanetProblem` turn-end integrations so Fallen battles and Necron tomb raids still trigger.</violation>
</file>
<file name="scripts/scr_destroy_planet/scr_destroy_planet.gml">
<violation number="1" location="scripts/scr_destroy_planet/scr_destroy_planet.gml:167">
P1: Custom agent: **Code Quality Review**
Legacy mission consumers still read `p_timer` and `p_problem_other_data`, but this path no longer initializes either array. Migrate those consumers to `PlanetProblem` or preserve the required state before destroying the planet.</violation>
</file>
<file name="scripts/scr_manage_task_selector/scr_manage_task_selector.gml">
<violation number="1" location="scripts/scr_manage_task_selector/scr_manage_task_selector.gml:51">
P1: Squad selection receives a `FeatureSelected` wrapper, not a `PlanetProblem`, so this type check rejects it and skips `on_squad_selection()`. Dereference the wrapped feature before testing it.</violation>
</file>
<file name="objects/obj_enunit/Create_0.gml">
<violation number="1" location="objects/obj_enunit/Create_0.gml:80">
P1: `add_enemies` selects index 1001 on a fresh unit, while `Alarm_1` only initializes indices 1–700, so data-driven battles contain no processed enemies. Find the first empty slot and stop the scan there.</violation>
</file>
<file name="scripts/scr_inquisition_mission/scr_inquisition_mission.gml">
<violation number="1" location="scripts/scr_inquisition_mission/scr_inquisition_mission.gml:227">
P1: This dereferences `mission_star` before the missing-star guard, so an invalid or deleted target crashes mission acceptance instead of closing the popup. Move `get_planet_data` after the guard.</violation>
</file>
<file name="scripts/MissionHelper/MissionHelper.gml">
<violation number="1" location="scripts/MissionHelper/MissionHelper.gml:36">
P0: This script cannot be parsed, so none of the new mission objects can run. Correct the malformed switch syntax, declarations, and assignments throughout the file before merging.</violation>
<violation number="2" location="scripts/MissionHelper/MissionHelper.gml:37">
P1: These dispatch entries point to nonexistent callbacks, so Mechanicus raider progress and beast-hunt completion cannot execute. Bind each mission ID to the implementation that actually exists.</violation>
<violation number="3" location="scripts/MissionHelper/MissionHelper.gml:338">
P0: Custom agent: **Code Quality Review**
These declarations use the invalid keywords `stati` and `fuction`, so the new script cannot compile. Replace them with `static` and `function` (including the three other `fuction` declarations at lines 1355, 1372, and 1397).</violation>
<violation number="4" location="scripts/MissionHelper/MissionHelper.gml:836">
P0: Custom agent: **Code Quality Review**
`data.turns : 0;` is invalid GML assignment syntax and prevents this script from compiling. Change `:` to `=`.</violation>
<violation number="5" location="scripts/MissionHelper/MissionHelper.gml:1274">
P1: Necron excursion battles pass their enemy roster to a parameter the helper does not accept or store, so the encounter either errors on argument count or starts without its generated enemies. Create the battle with two arguments and assign `_battle.battle_enemy_data = _battle_data`.</violation>
</file>
<file name="scripts/scr_PlanetData/scr_PlanetData.gml">
<violation number="1" location="scripts/scr_PlanetData/scr_PlanetData.gml:802">
P1: `problem_count_down` is broken: `problem.timer` is an undefined variable (should be `_problem.timer`), `array_delete(system.p_problem[i], i, 0)` indexes `p_problem` by loop index instead of planet and deletes 0 elements, so the timer==-1 cleanup never removes anything. The only caller (scr_enemy_ai_d.gml:87) also runs in obj_star scope where this method no longer exists, so the call errors. Rewrite the loop to delete from `system.p_problem[planet]` at index `i` (count 1) and route the caller through `get_planet_data(i).problem_count_down(...)`.</violation>
</file>
<file name="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml">
<violation number="1" location="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml:87">
P0: `problem_count_down(i)` is not a global function. The only definition is an instance method declared inside the `PlanetData` constructor (scr_PlanetData.gml:798), so this bare call from `scr_enemy_ai_d()` is an unknown-function compile error and per-mission turn countdown never runs. Call it on the planet data instance instead; also fix the method itself, which currently references the undeclared `problem` (should be `_problem`) and deletes via `system.p_problem[i]` (should be `system.p_problem[planet]`).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
|
|
||
| stati init_train_forces_mission = fuction(marine) { |
There was a problem hiding this comment.
P0: Custom agent: Code Quality Review
These declarations use the invalid keywords stati and fuction, so the new script cannot compile. Replace them with static and function (including the three other fuction declarations at lines 1355, 1372, and 1397).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 338:
<comment>These declarations use the invalid keywords `stati` and `fuction`, so the new script cannot compile. Replace them with `static` and `function` (including the three other `fuction` declarations at lines 1355, 1372, and 1397).</comment>
<file context>
@@ -0,0 +1,1405 @@
+}
+
+
+stati init_train_forces_mission = fuction(marine) {
+ if (stage_id != "preliminary") {
+ exit;
</file context>
| if ((timer > -1) && per_turn_checks) { | ||
| var _func = undefined; | ||
| switch(p_id){ | ||
| case "mech_raider"; |
There was a problem hiding this comment.
P0: This script cannot be parsed, so none of the new mission objects can run. Correct the malformed switch syntax, declarations, and assignments throughout the file before merging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 36:
<comment>This script cannot be parsed, so none of the new mission objects can run. Correct the malformed switch syntax, declarations, and assignments throughout the file before merging.</comment>
<file context>
@@ -0,0 +1,1405 @@
+ if ((timer > -1) && per_turn_checks) {
+ var _func = undefined;
+ switch(p_id){
+ case "mech_raider";
+ _func = per_turn_check_raider_failed;
+ break;
</file context>
| /// @self Asset.GMObject.obj_popup | ||
| function init_mission_inquisition_tomb_world() { | ||
| mission_star = find_star_by_name(pop_data.system); | ||
| var _p_data = mission_star.get_planet_data(pop_data.planet); |
There was a problem hiding this comment.
P1: This dereferences mission_star before the missing-star guard, so an invalid or deleted target crashes mission acceptance instead of closing the popup. Move get_planet_data after the guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_inquisition_mission/scr_inquisition_mission.gml, line 227:
<comment>This dereferences `mission_star` before the missing-star guard, so an invalid or deleted target crashes mission acceptance instead of closing the popup. Move `get_planet_data` after the guard.</comment>
<file context>
@@ -226,11 +224,12 @@ function mission_inquisition_tomb_world(tomb_worlds) {
/// @self Asset.GMObject.obj_popup
function init_mission_inquisition_tomb_world() {
mission_star = find_star_by_name(pop_data.system);
+ var _p_data = mission_star.get_planet_data(pop_data.planet);
if (mission_star == noone) {
popup_default_close();
</file context>
| var _p_data = mission_star.get_planet_data(pop_data.planet); | |
| var _p_data = mission_star == noone ? noone : mission_star.get_planet_data(pop_data.planet); |
| for (var i = array_length(problems) -1; i >= 0; i--) { | ||
| var _problem = problems[i]; | ||
| _problem.basic_turn_end(); | ||
| if (problem.timer == -1){ |
There was a problem hiding this comment.
P1: problem_count_down is broken: problem.timer is an undefined variable (should be _problem.timer), array_delete(system.p_problem[i], i, 0) indexes p_problem by loop index instead of planet and deletes 0 elements, so the timer==-1 cleanup never removes anything. The only caller (scr_enemy_ai_d.gml:87) also runs in obj_star scope where this method no longer exists, so the call errors. Rewrite the loop to delete from system.p_problem[planet] at index i (count 1) and route the caller through get_planet_data(i).problem_count_down(...).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_PlanetData/scr_PlanetData.gml, line 802:
<comment>`problem_count_down` is broken: `problem.timer` is an undefined variable (should be `_problem.timer`), `array_delete(system.p_problem[i], i, 0)` indexes `p_problem` by loop index instead of planet and deletes 0 elements, so the timer==-1 cleanup never removes anything. The only caller (scr_enemy_ai_d.gml:87) also runs in obj_star scope where this method no longer exists, so the call errors. Rewrite the loop to delete from `system.p_problem[planet]` at index `i` (count 1) and route the caller through `get_planet_data(i).problem_count_down(...)`.</comment>
<file context>
@@ -760,24 +758,53 @@ function PlanetData(_planet, _system) constructor {
+ for (var i = array_length(problems) -1; i >= 0; i--) {
+ var _problem = problems[i];
+ _problem.basic_turn_end();
+ if (problem.timer == -1){
+ array_delete(system.p_problem[i], i,0);
+ array_delete(problems, i,0);
</file context>
Moves the Demon World problem creation logic directly into `scr_inquisition_mission`, eliminating a redundant helper function. Enhances `MissionHelper` to use dynamic function calls for mission acceptance, provide context-sensitive dialogue for demanded missions, and ensure popups close correctly.
There was a problem hiding this comment.
2 existing issues remain and 3 new issues found across 10 files (changes from recent commits).
Confidence score: 3/5
- In
scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml, dropping themission_name_key(...) != "none"guard can show unsupported problem types as mission rows; restore the filter. - In
scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml, the tomb-mission path leaves popup options active, which may allow another action from stale options; callreset_popup_options()after creating the mission. - In
scripts/scr_PlanetData/scr_PlanetData.gml, repeating the raw"preliminary"stage identifier risks inconsistent lifecycle checks; define a shared macro or enum and use it consistently. - The additions in
scripts/PlanetProblem/PlanetProblem.gmllack the required JSDoc, and_p_datainscripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gmlconflicts with the naming rule; add parameter/return docs and rename it to_planet_data.
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_mechanicus_missions/scr_mechanicus_missions.gml">
<violation number="1" location="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml:147">
P2: After a tomb mission is accepted, the popup options remain active because this new path never calls `reset_popup_options()`. Clear the options after creating the mission to prevent the stale popup from allowing another selection.</violation>
</file>
<file name="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml">
<violation number="1" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:267">
P3: Custom agent: **Code Quality Review**
`_p_data` abbreviates `planet_data`, conflicting with the rule requiring clear, unabbreviated variable names. Rename it to `_planet_data` and update the following call.</violation>
<violation number="2" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:268">
P2: This delegation drops the old `mission_name_key(...) != "none"` guard, so unsupported problem types now show up as mission rows. Keep filtering out `</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
/review |
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of ccb4b1c..1a445b6. Good progress, verified in workspace: PLANET_ARRAY_SIZE macro + deserialize guard, view_on_planet_screen/problems_to_mission_log split, select_units/squads _self capture, hunt-beast select_units arg fix, mech __*_init port started, garrison/tyranid renames holding, spelling cleanups. Still REQUEST_CHANGES: this increment adds new blocking runtime bugs (bad array_length paren, mech-raider timer never set, collect_role_group arg order, bare garrisons/has_feature/_star/title, button closures still throwing at click time, data.target vs data.enemy mismatch, for (i leak). Fix inline blockers first.
Carry-overs NOT in this diff (still blocking, no inline slot): (1) PlanetProblem:1885 scr_popup("Inquisition Mission", text, ...) uses bare text, local is _text. (2) PlanetProblem:1824-1825 hive init + x/+ y uses bare x/y in struct scope; use system.x/system.y. (3) Same closure-scope bug as inline :438/:1274 exists out-of-diff in __train_forces_feature_selected (:543-544, bare system/select_units with self=draw_data at click time); apply the same method(self, ...) fix. (4) PlanetProblem:999 scr_mission_reward("mech_bionics", id, planet) uses bare id (mission struct has no id); use system like :974. (5) scr_mechanicus_missions:139 for (var i = 1; i < _star.planets; i++) off-by-one skips last planet (1-indexed); use <= . (6) scr_mechanicus_missions:139 loop uses i < _star.planets, skipping the last planet (1-indexed); use <= (context line, noted here).
| draw_data.button_text = "Send Hunters"; | ||
| draw_data.button_function = function() { | ||
| var _dudes = collect_role_group("all", system.name); | ||
| select_units(_dudes, "Beast Hunt", 3); |
There was a problem hiding this comment.
Closure runs with self=draw_data at click time (called as draw_data.button_function() in __feature_selected_draw), so bare system (:437) and select_units here resolve on draw_data -> undefined/error on click. Fix: draw_data.button_function = method(self, function() { ... }) so self stays the mission. Same bug at :1274 (and out-of-diff :543-544).
There was a problem hiding this comment.
the calling of button_function occurs in the PlanetProblem scope so this is not a problem
| _mission_time += _vacation_time; | ||
| var _techs = collect_role_group([SPECIALISTS_TECHMARINES, false, true]); | ||
|
|
||
| var _techs_required = min(array_length(_techs) - 2, 6); |
There was a problem hiding this comment.
min(array_length(_techs) - 2, 6) goes negative when fewer than 2 techs exist, and then __mech_raider_per_turn (>= negative) is trivially true. Clamp: max(min(array_length(_techs) - 2, 6), 0).
There was a problem hiding this comment.
a tech couont is calculated earlier called mission is not called if less than three techs
| draw_data.button_text = "Send Squad"; | ||
| draw_data.button_tooltip = "mileage may vary on playability of this mission progress at your own risk"; | ||
| draw_data.button_function = function() { | ||
| var _dudes = collect_role_group("all", system.name); |
There was a problem hiding this comment.
Same closure bug as :438: at click time self=draw_data, so bare system and select_squads are undefined. Fix: draw_data.button_function = method(self, function() { ... });.
There was a problem hiding this comment.
same fix i stated above
| } | ||
|
|
||
| static __governor_purge_enemies_feature_selected = function(){ | ||
| draw_data.mission_description = $"The governor of {p_data.name()} has expressed his distaste of the neighbouring governance of {system.name} {data.target} he has expressed his views that they engage in heretical ways and harbor xenos enemies though in truth it is more likely that he simply wishes his political enemies disposed of, whatever the case his planet has great economic means and he has made bare his plans to compensate the emperors angels for their aid"; |
There was a problem hiding this comment.
data.target is never set; __governor_purge_enemies_init stores data.enemy (:1954). Interpolating a missing struct key throws. Fix: {system.name} {data.enemy}.
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 86 unresolved issues from previous reviews.
Re-trigger cubic
|
/review |
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of 1a445b6..ba5424b. Good progress, verified in workspace: mech_raider timer assignment/system.name fix, per-turn collect_role_group arg-order + _techs.number() fix, for (var i leak fix, garrison _garrison local/p_data.has_feature/get_features fixes, data.target now consistent with feature_selected.
Still REQUEST_CHANGES: this increment adds one blocking bug (inline), and prior carry-overs outside this diff remain unfixed:
Carry-overs NOT in this diff (still blocking, no inline slot): (1) PlanetProblem:1886 scr_popup("Inquisition Mission", text, ...) uses bare text, local is _text. (2) PlanetProblem:1825-1826 hive init + x/+ y uses bare x/y in struct scope; use system.x/system.y. (3) Same closure-scope bug exists out-of-diff in __train_forces_feature_selected (:542-545, bare system/select_units with self=draw_data at click time); apply the same method(self, ...) capture fix used for hunt-beast/protect-raiders. (4) PlanetProblem:999 scr_mission_reward("mech_bionics", id, planet) uses bare id (mission struct has no id); use system. (5) scr_mechanicus_missions:139 for (var i = 1; i < _star.planets; i++) off-by-one skips last planet (1-indexed); use <=.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 83 unresolved issues from previous reviews.
Re-trigger cubic
move rewards from mech missions into correct sequence position in PlanetProblem
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of ba5424b..5de9e66. Good progress, verified in workspace: hive init now uses system.x/system.y, purge init _text fix, _techs.number() call fix, tomb loop <= off-by-one fix, mech reward migration out of scr_mission_reward.
Still REQUEST_CHANGES: the newly inlined mech reward blocks introduce blocking runtime bugs (undefined star, undeclared struct fields leaking into saves, dead reward call/cleanup). Fix inline blockers first.
Carry-overs NOT in this diff (still blocking, no inline slot): (1) Same closure-scope bug exists out-of-diff in __hunt_beast_feature_selected (:436-439), __train_forces_feature_selected (:542-545) and __protect_raiders_feature_selected (:1398-1401) - bare system/select_units/select_squads with self=draw_data at click time; bind with method(self, ...) capturing mission state. (2) __mech_raider_per_turn:971 redeclares var _percent_complete inside the if - harmless in GML (function scope) but redundant; assign without var like the bionics version (:1062) does.
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of 5de9e66..708045f. Good progress, verified in workspace: __mech_raider_per_turn var redeclare fix (:971), _cleanup bool array + _cleanup[com] = true + if (_cleanup[i]) (:977/:1009/:1039), star.name -> system.name in both vehicle loops (:1007/:1025), bionics _mech_disp_change/_req_gain locals + system.name (:1085/:1087/:1090/:1094/:1101-1103), SystemProblem _name/_data/_system rename (:3-9).
Still REQUEST_CHANGES: one in-diff popup bug below, plus unfixed carry-overs outside this diff (verified at HEAD) that still block merge:
Carry-overs NOT in this diff (still blocking, no inline slot): (1) objects/obj_star/Create_0.gml:124 still new SystemProblem(name, data, system) - name is the star name not p_id, bare system is undefined in star scope (should be p_id, data, id/self); no SystemProblem can be created via add_problem. (2) Same closure-scope bug in __hunt_beast_feature_selected (:436-439), __train_forces_feature_selected (:542-545), __protect_raiders_feature_selected (:1397-1400) - draw_data.button_function = function() { collect_role_group(..., system.name); select_units/select_squads(...); } reads bare system with self == draw_data at click time; GML methods do not capture outer self/locals, so this throws undefined on click. Bind with method(self, ...) capturing mission state (e.g. store system.name in a captured struct). (3) Same alter_disposition return-string bug as inline :1125 exists out-of-diff at PlanetProblem:839 (var _disp_gain_string = alter_disposition(...) interpolated into event log without true).
There was a problem hiding this comment.
6 issues found across 5 files (changes from recent commits).
Confidence score: 3/5
- In
scripts/PlanetProblem/PlanetProblem.gml, the Requisition reward can skip bionic upgrades when the armoury lacks stock while still applying its side effects, leaving the outcome incomplete — supply the implants explicitly or calladd_bionicswithout consuming armoury stock. - In
scripts/PlanetProblem/PlanetProblem.gml, the “Land Raider” reward only repairs the mission’s existing vehicle instead of granting a replacement, so players may not receive the promised reward — create a replacement vehicle and stop scanning after it is added. - The remaining
scripts/PlanetProblem/PlanetProblem.gmlandscripts/SystemProblem/SystemProblem.gmlfindings are maintainability and documentation follow-ups: replace duplicated magic values, fix the assignment and variable naming conventions, and correct the parameter docblock.
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/SystemProblem/SystemProblem.gml">
<violation number="1" location="scripts/SystemProblem/SystemProblem.gml:3">
P3: The docblock above now contradicts the reordered signature: it documents `name`/`data`, omits the new `_system` param entirely, and still has the `sring` typo. Update to `/// @param {string} _name`, `/// @param {constructor obj_star} _system`, `/// @param {struct} _data` so the JSDoc matches the renamed/reordered parameters.</violation>
</file>
<file name="scripts/PlanetProblem/PlanetProblem.gml">
<violation number="1" location="scripts/PlanetProblem/PlanetProblem.gml:977">
P2: Custom agent: **Code Quality Review**
The new mission paths duplicate the magic company-array size `11` and dice-roll mode `"low"`; define named `#macro` or `enum` constants and reuse them.</violation>
<violation number="2" location="scripts/PlanetProblem/PlanetProblem.gml:979">
P3: Custom agent: **Code Quality Review**
Add trailing semicolons to the new `timer = -1` assignments in both mission handlers.</violation>
<violation number="3" location="scripts/PlanetProblem/PlanetProblem.gml:984">
P3: Custom agent: **Code Quality Review**
This local variable violates the required `_` prefix convention. Rename `result` to `_result` throughout this reward branch.</violation>
<violation number="4" location="scripts/PlanetProblem/PlanetProblem.gml:1027">
P2: The `"Land Raider"` reward does not provide a new Land Raider; it only repairs the one already used for the mission. Create the replacement vehicle and stop scanning after the replacement is added.</violation>
<violation number="5" location="scripts/PlanetProblem/PlanetProblem.gml:1116">
P2: The Requisition outcome silently fails its bionic upgrades when the armoury lacks stock, while still applying the side effects. Supply those implants explicitly or call `add_bionics` without consuming armoury stock for that outcome.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
|
|
||
| repeat (choose(2, 3, 4)) { | ||
| _unit.add_bionics(); |
There was a problem hiding this comment.
P2: The Requisition outcome silently fails its bionic upgrades when the armoury lacks stock, while still applying the side effects. Supply those implants explicitly or call add_bionics without consuming armoury stock for that outcome.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/PlanetProblem/PlanetProblem.gml, line 1116:
<comment>The Requisition outcome silently fails its bionic upgrades when the armoury lacks stock, while still applying the side effects. Supply those implants explicitly or call `add_bionics` without consuming armoury stock for that outcome.</comment>
<file context>
@@ -991,17 +1057,74 @@ static __mech_raider_resolve = function() {
+ }
+
+ repeat (choose(2, 3, 4)) {
+ _unit.add_bionics();
+ }
+ _limit++;
</file context>
| for (var i = 1; i <= 100; i++) { | ||
| if ((obj_ini.veh_role[com][i] == "Land Raider") && (obj_ini.veh_loc[com][i] == system.name) && (obj_ini.veh_wid[com][i] == planet)) { | ||
| _found = true; | ||
| obj_ini.veh_hp[com][i] = 100; |
There was a problem hiding this comment.
P2: The "Land Raider" reward does not provide a new Land Raider; it only repairs the one already used for the mission. Create the replacement vehicle and stop scanning after the replacement is added.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/PlanetProblem/PlanetProblem.gml, line 1027:
<comment>The `"Land Raider"` reward does not provide a new Land Raider; it only repairs the one already used for the mission. Create the replacement vehicle and stop scanning after the replacement is added.</comment>
<file context>
@@ -966,17 +966,83 @@ static __mech_raider_init = function(){
+ for (var i = 1; i <= 100; i++) {
+ if ((obj_ini.veh_role[com][i] == "Land Raider") && (obj_ini.veh_loc[com][i] == system.name) && (obj_ini.veh_wid[com][i] == planet)) {
+ _found = true;
+ obj_ini.veh_hp[com][i] = 100;
+ }
+ }
</file context>
| if (_percent_complete < 100){ | ||
| exit; | ||
| } | ||
| var _cleanup = array_create(11, false); |
There was a problem hiding this comment.
P2: Custom agent: Code Quality Review
The new mission paths duplicate the magic company-array size 11 and dice-roll mode "low"; define named #macro or enum constants and reuse them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/PlanetProblem/PlanetProblem.gml, line 977:
<comment>The new mission paths duplicate the magic company-array size `11` and dice-roll mode `"low"`; define named `#macro` or `enum` constants and reuse them.</comment>
<file context>
@@ -966,17 +966,83 @@ static __mech_raider_init = function(){
+ if (_percent_complete < 100){
+ exit;
+ }
+ var _cleanup = array_create(11, false);
+ delete_mission = true;
+ timer = -1
</file context>
| @@ -0,0 +1,128 @@ | |||
| /// @param {sring} name | |||
| /// @param {struct} data | |||
| function SystemProblem(_name, _system, _data = {}) constructor{ | |||
There was a problem hiding this comment.
P3: The docblock above now contradicts the reordered signature: it documents name/data, omits the new _system param entirely, and still has the sring typo. Update to /// @param {string} _name, /// @param {constructor obj_star} _system, /// @param {struct} _data so the JSDoc matches the renamed/reordered parameters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/SystemProblem/SystemProblem.gml, line 3:
<comment>The docblock above now contradicts the reordered signature: it documents `name`/`data`, omits the new `_system` param entirely, and still has the `sring` typo. Update to `/// @param {string} _name`, `/// @param {constructor obj_star} _system`, `/// @param {struct} _data` so the JSDoc matches the renamed/reordered parameters.</comment>
<file context>
@@ -1,12 +1,12 @@
/// @param {sring} name
/// @param {struct} data
-function SystemProblem(name, data = {}, system) constructor{
+function SystemProblem(_name, _system, _data = {}) constructor{
timer = -1;
f_type = eP_FEATURES.MISSION;
</file context>
| } | ||
| var _cleanup = array_create(11, false); | ||
| delete_mission = true; | ||
| timer = -1 |
There was a problem hiding this comment.
P3: Custom agent: Code Quality Review
Add trailing semicolons to the new timer = -1 assignments in both mission handlers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/PlanetProblem/PlanetProblem.gml, line 979:
<comment>Add trailing semicolons to the new `timer = -1` assignments in both mission handlers.</comment>
<file context>
@@ -966,17 +966,83 @@ static __mech_raider_init = function(){
+ }
+ var _cleanup = array_create(11, false);
+ delete_mission = true;
+ timer = -1
+ per_turn_checks = false;
+ zero_timer_checks = false;
</file context>
| timer = -1 | |
| timer = -1; |
| zero_timer_checks = false; | ||
|
|
||
| var _roll1 = roll_dice_chapter(1, 100, "low"); | ||
| var result = ""; |
There was a problem hiding this comment.
P3: Custom agent: Code Quality Review
This local variable violates the required _ prefix convention. Rename result to _result throughout this reward branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/PlanetProblem/PlanetProblem.gml, line 984:
<comment>This local variable violates the required `_` prefix convention. Rename `result` to `_result` throughout this reward branch.</comment>
<file context>
@@ -966,17 +966,83 @@ static __mech_raider_init = function(){
+ zero_timer_checks = false;
+
+ var _roll1 = roll_dice_chapter(1, 100, "low");
+ var result = "";
+
+ if (_roll1 <= 33) {
</file context>
Summary by cubic
Converts planets' parallel mission arrays (
p_problem,p_timer,p_problem_other_data) intoPlanetProblemmission objects that serialize throughPlanetData, so active missions persist across saves. System-wide problems (the Great Crusade) are nowSystemProblemobjects tracked insystem_problemswith turn-end countdown inbasic_end_turn.Mission lifecycle (turn-end checks, resolution, pre-battle effects, battle outcomes, completion/refusal, rewards) and unit assignment run through
PlanetProblemmethods. Battles get mission context via thespecial_featurestruct, and enemy forces come frombattle_enemy_datacreated through the newadd_enemiesmethod onobj_enunit. The Inquisition recon, hunt-fallen, purge, and necron missions are fully on the new system — recon triggers at turn end, popups invoke missions via thepopup_callhelper, purge completes through anon_purgehook, andbattle_specialmay now be a struct carryingspecial_idandspecial_feature. Rewards fire from the mission's lifecycle methods instead of a separate reward script.Adds a Tyranid "Hive Fleet to Cult" mission simulating fleet approach and cult growth, with fleet creation and 'Shadow in the Warp' notifications. Removes the legacy
obj_temp7object;squad.membersaccess now goes throughsquad.get_members(), which prunes stale marine entries viaclean_unit_array. ThePLANET_ARRAY_SIZEmacro replaces the hard-coded planet count, thePlanetProblemscript documents lifecycle entry points for future mission authors, and errored missions auto-delete at the end of their lifecycle step.Migration
mech_tomb1/mech_tomb2→mech_tomb,fallen→hunt_fallen,spyrer→inquisition_spyrer,recon→inquisition_recon,necron/inquisition_tomb→inquisition_necron;mech_bionicsis removed.add_new_problemwithPlanetData.new_problem; missions signal completion by setting adelete_missionflag instead of callingremove_planet_problem.obj_starand thefallencheat command are removed; legacy variables likebattle_mission,battle_data, andcaptured_gauntare gone.Written for commit dac29f7. Summary will update on new commits.