Skip to content

fix: Make game time/date have a single source of truth - #1408

Merged
OH296 merged 10 commits into
Adeptus-Dominus:mainfrom
OH296:ssingle_source_of_time
Aug 3, 2026
Merged

fix: Make game time/date have a single source of truth#1408
OH296 merged 10 commits into
Adeptus-Dominus:mainfrom
OH296:ssingle_source_of_time

Conversation

@OH296

@OH296 OH296 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Summary by cubic

Made the game time/date a single source of truth via SectorHandler, and wired it into UI, saves, turn progression, and unit systems. Reintroduced the Doomed rule; gene‑seed recovery and ages now follow a consistent timeline.

  • New Features

    • Chapter creation: new “Game Date” with ValueShifters for Millennium/Year (locked for premade); the chosen sector_handler is reused during init; intro/banner and event logs read sector_handler.date(), and end turn advances time via increment_date().
    • Units: set born/marine_ascension on spawn and for starting forces; new age() and recoverable_geneseed() power post‑combat harvest, kill flow, armour rolls, sorting, and unit details, and respect doomed and zygote mutations.
  • Bug Fixes

    • ValueShifter now has a safe default and step-aware clamping.
    • Save/load: properly serializes and restores obj_ini.sector_handler.
    • Penitent/blood debt timers use sector_handler.game_year() for consistent expiry.

Written for commit f58fb74. Summary will update on new commits.

Review in cubic

@github-actions github-actions Bot added Size: Big Type: Fix This is a fix for a bug labels Aug 1, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

24 issues found across 33 files

Confidence score: 2/5

  • objects/obj_ini/Create_0.gml initializes 2D arrays with shared row references (array_create(11, [])) and also leaves race[1] as a scalar, which can corrupt cross-company writes and break new chapter creation during company setup — initialize each row as an independent array before any race[c][i]/roster writes.
  • scripts/scr_company_view/scr_company_view.gml can write a transferred unit past the destination company’s appended slot by reusing a source index, creating out-of-bounds/undefined roster state that will cascade into later turn logic — compute and use the index returned from target_company insertion.
  • objects/obj_p_assra/Alarm_0.gml, scripts/scr_dialogue/scr_dialogue.gml, and objects/obj_controller/Alarm_5.gml each contain direct undefined dereferences (unit/age, unset o, and _unit.god_status before guard), so normal gameplay paths (death recovery, meeting dialogue, turn end) can hard-error — fix variable references and move undefined checks before field access.
  • scripts/scr_world_time/scr_world_time.gml and scripts/scr_random_event/scr_random_event.gml include call-site/schema mismatches (game_year used as a property, _unit.special vs specials, and bare TTRPG), which can silently break progression timing and event eligibility or throw identifier errors — align callers to game_year(), correct field names, and consistently reference obj_ini.TTRPG.
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_company_view/scr_company_view.gml">

<violation number="1" location="scripts/scr_company_view/scr_company_view.gml:52">
P2: Custom agent: **Code Quality Review**

The `find_company_open_slot` function mutates persistent `obj_ini` arrays when no open slot exists, but its name implies a simple read-only lookup. Hiding array-expansion side effects inside a finder makes it easy for callers to accidentally alter game state. Consider renaming this function to something like `find_or_create_company_open_slot`, or move the expansion logic out to the caller so the side effect is explicit.</violation>

<violation number="2" location="scripts/scr_company_view/scr_company_view.gml:63">
P1: Transfers into a full destination company can use an index from the source company's `TTRPG` array, then write the unit beyond the newly appended destination slot. Derive the returned index from `target_company` so `scr_move_unit_info` targets the initialized destination slot.</violation>
</file>

<file name="objects/obj_p_assra/Alarm_0.gml">

<violation number="1" location="objects/obj_p_assra/Alarm_0.gml:21">
P1: Recovering gene-seed now errors whenever an Astartes dies with an Apothecary available: `recoverable_geneseed()` reads undefined `unit`, and its time calculation reads undefined `age`. Fix the helper to use its receiver's `gene_seed_mutations` and pass/use `marine_ascension` in `get_time_from_current_year` before invoking it here.</violation>
</file>

<file name="scripts/scr_world_time/scr_world_time.gml">

<violation number="1" location="scripts/scr_world_time/scr_world_time.gml:26">
P1: Chapter initialization and marine ascension now consume the `game_year` function itself instead of a numeric year because existing callers use property access. Update those callers to invoke `game_year()` (or preserve a numeric property API) before this handler is used.</violation>

<violation number="2" location="scripts/scr_world_time/scr_world_time.gml:26">
P2: Custom agent: **Code Quality Review**

The raw constant `1000` (years per millennium) is repeated three times in two functions without a `#macro` or `enum`. Because this is newly added code and the project already uses `#macro` extensively for numeric constants, this should be named (e.g., `#macro YEARS_PER_MILLENNIUM 1000`) to ensure a single source of truth and easier maintenance.</violation>

<violation number="3" location="scripts/scr_world_time/scr_world_time.gml:34">
P2: Crossing a year boundary drops the 8-unit overflow from the twelfth turn, making the world clock lose time every year. Preserve the remainder when rolling the year.</violation>
</file>

<file name="scripts/scr_random_event/scr_random_event.gml">

<violation number="1" location="scripts/scr_random_event/scr_random_event.gml:710">
P1: This loop reads the bare identifier `TTRPG`, but the player roster is stored on the game object as `obj_ini.TTRPG` (declared as `obj_ini.TTRPG = array_create(11, [])` in obj_ini/Create_0.gml) and there is no `globalvar`/global `TTRPG` and no local `TTRPG` in this script. When the CHAOS_INVASION event fires, `TTRPG[0]` will throw an 'undefined variable' runtime error (breaking the event). The sibling changes in scr_enemy_ai_d and scr_dialogue correctly use `obj_ini.TTRPG[0]`; this one should match.</violation>

<violation number="2" location="scripts/scr_random_event/scr_random_event.gml:712">
P1: The psyker check in the Chaos Invasion event reads `_unit.special`, but the marine struct's field is `specials` (`scr_marine_struct.gml:1052`); the removed code read the parallel `obj_ini.spe[0][i]` copy. As written, `_unit.special` is undefined, so `string_count("0", _unit.special)` won't detect a psychic chaptermaster, and this event branch's special version of "The Maw of the Warp Yawns Wide" popup can't trigger (and may throw). Replace `_unit.special` with `_unit.specials`.</violation>
</file>

<file name="scripts/scr_dialogue/scr_dialogue.gml">

<violation number="1" location="scripts/scr_dialogue/scr_dialogue.gml:182">
P1: Selecting any response that enters `cs_meeting_m1` reads an unset `o` and fails before dialogue options are built. Use the HQ company index used by this loop.</violation>
</file>

<file name="objects/obj_controller/Alarm_5.gml">

<violation number="1" location="objects/obj_controller/Alarm_5.gml:293">
P1: Turn-end processing can error after a unit is removed: `_unit.god_status` dereferences the `undefined` slot before the intended struct guard runs. Validate `_unit` before reading its fields.</violation>
</file>

<file name="scripts/scr_chapter_managent_events/scr_chapter_managent_events.gml">

<violation number="1" location="scripts/scr_chapter_managent_events/scr_chapter_managent_events.gml:276">
P1: The new empty-slot scan dereferences obj_ini.TTRPG[0][i].name() on every slot, but scr_wipe_unit stores undefined in a dead/replaced unit's TTRPG slot (e.g. the previous Forge Master whose death opens this popup). Calling .name() on undefined crashes the popup, whereas the old obj_ini.name[0][i] == "" check was safe since the name array always holds a string. Guard the access with is_struct (or keep scanning the name array) before calling .name().</violation>
</file>

<file name="scripts/scr_after_combat/scr_after_combat.gml">

<violation number="1" location="scripts/scr_after_combat/scr_after_combat.gml:306">
P2: Gene-seed recovery now uses one 80% roll for the entire unit, reducing the prior 90%-per-seed recovery rate and eliminating partial recovery for two seeds. Roll once per seed with the previous threshold to preserve recovery behavior.</violation>
</file>

<file name="scripts/scr_count_forces/scr_count_forces.gml">

<violation number="1" location="scripts/scr_count_forces/scr_count_forces.gml:16">
P2: The loop bound now equals the company's marine-array length, but the same loop also counts vehicles, and vehicle slots can extend beyond that length (veh_* arrays hold up to 205 slots per company and aren't resized with the marine count). When a company has vehicles occupying indexes >= its TTRPG array length, those vehicles silently drop out of the count, misreporting forces to the AI (scr_enemy_ai_e.gml:519) and the turn-end UI. Restore scanning up to the max of marine and vehicle lengths, guarding each sub-check by its own array length.</violation>
</file>

<file name="objects/obj_ini/Create_0.gml">

<violation number="1" location="objects/obj_ini/Create_0.gml:121">
P2: Custom agent: **Code Quality Review**

The marine array initializations repeat the raw dimension constant `11` across eight consecutive added lines instead of using the already-defined `_max_companies` constant. Using a named constant prevents drift between the array sizes and the intended capacity (e.g., vehicle arrays right above already use `_max_companies`). Consider replacing the raw `11` with `_max_companies`.</violation>

<violation number="2" location="objects/obj_ini/Create_0.gml:121">
P1: Creating a new chapter fails while initializing company 1 because `race[1]` is `501`, not an array. Initialize independent empty rows so the later `race[c][i]` assignments can grow them.</violation>

<violation number="3" location="objects/obj_ini/Create_0.gml:123">
P0: Custom agent: **Code Quality Review**

Using `array_create(11, [])` for a grid introduces shared-row references: every slot points to the same empty array. That makes two-dimensional indexing fragile and can cause writes to one company to leak into others. It also leaves each row at length 0, which breaks downstream code that indexes up to 500. Keep the explicit `array_create_2d(11, 501, "")` helper (or initialize each row independently) so each company gets its own 501-length array.</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:170">
P2: This reads `_unit.special` (singular), but the psyker-powers field on the unit struct is `specials` (plural). Since `special` is undefined, `string_count("0", undefined)` returns 0, so the 'Shadow in the Warp' popup/event-log for a psyker Chapter Master will never trigger even when the master has psyker powers. Should be `_unit.specials`.</violation>
</file>

<file name="scripts/scr_civil_roster/scr_civil_roster.gml">

<violation number="1" location="scripts/scr_civil_roster/scr_civil_roster.gml:703">
P1: `deploying_unit` is `obj_ini` (assigned at the top of this function), and `obj_ini` has its own `specials = 0` field (obj_ini/Create_0.gml:4), not a per-unit powers string. So `string_count("0", 0)` converts 0 to "0" and returns 1, meaning `chapter_master_psyker` is always set to 1 for every Chapter Master regardless of actual psyker powers. The per-unit specials now live on the fetched unit struct, so this should read `unit.specials` (as the dudes_powers line at 369 in the same change already does).</violation>

<violation number="2" location="scripts/scr_civil_roster/scr_civil_roster.gml:750">
P1: `deploying_unit` is `obj_ini`, whose `specials` field is `0` (obj_ini/Create_0.gml:4), not the unit's powers. This writes `0` to `marine_powers` for the deployed Chapter Master instead of the actual specials string. Since the per-unit powers now live on the fetched unit struct, this should read `unit.specials` to match the dudes_powers line (369) changed in the same PR.</violation>
</file>

<file name="objects/obj_controller/Mouse_50.gml">

<violation number="1" location="objects/obj_controller/Mouse_50.gml:48">
P1: The jail-detection loop can crash on dead or unassigned marines. The previous code scanned a numeric array (obj_ini.god[c]) where every index held an int, so reading god[c][e] was always safe. This version iterates obj_ini.TTRPG[c] and dereferences each entry directly (obj_ini.TTRPG[c][e].god_status) without checking that the entry is a struct. Other code paths set TTRPG slots to undefined when a marine is killed (scr_kill_unit array_set ... undefined) and when new slots are appended (scr_company_view array_push ... undefined for open slots), so the loop will hit undefined entries and raise a runtime error instead of simply skipping them. Add an is_struct guard before reading god_status, matching how scr_role_count already guards its equivalent loop.</violation>
</file>

<file name="scripts/scr_marine_struct/scr_marine_struct.gml">

<violation number="1" location="scripts/scr_marine_struct/scr_marine_struct.gml:105">
P1: Loading an existing save resets every marine's age to zero because those saves contain legacy `obj_ini.age` data, not `born`. Seed `born` from that legacy age when present before treating the new field as authoritative.</violation>

<violation number="2" location="scripts/scr_marine_struct/scr_marine_struct.gml:1052">
P2: Existing saves lose all encoded psychic-power specials after load because the replacement source is initialized to an empty string rather than migrated from legacy `obj_ini.spe`. Initialize from `spe` when it exists, then retain this per-marine field going forward.</violation>

<violation number="3" location="scripts/scr_marine_struct/scr_marine_struct.gml:2009">
P2: Existing saves clear every jailed marine's status on load because legacy `obj_ini.god` values are never copied into `god_status`. Migrate that value when the legacy array is present.</violation>
</file>

<file name="scripts/scr_controller_helpers/scr_controller_helpers.gml">

<violation number="1" location="scripts/scr_controller_helpers/scr_controller_helpers.gml:240">
P1: This jail-detection loop dereferences obj_ini.TTRPG[c][e].god_status directly and can hit undefined entries (dead marines and unassigned slots are stored as undefined in obj_ini.TTRPG), producing a runtime error instead of skipping them. The previous code read a numeric god[c][e] array where every index existed. Guard the dereference with an is_struct check, as scr_role_count does for its equivalent loop.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/scr_initialize_custom/scr_initialize_custom.gml Outdated
Comment thread objects/obj_controller/Create_0.gml Outdated
Comment thread scripts/SectorHandler/SectorHandler.gml
race = array_create(11, 501);
/// @type {Array<Array<String>>}
name = array_create_2d(11, 501, "");
name = array_create(11, []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P0: Custom agent: Code Quality Review

Using array_create(11, []) for a grid introduces shared-row references: every slot points to the same empty array. That makes two-dimensional indexing fragile and can cause writes to one company to leak into others. It also leaves each row at length 0, which breaks downstream code that indexes up to 500. Keep the explicit array_create_2d(11, 501, "") helper (or initialize each row independently) so each company gets its own 501-length array.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_ini/Create_0.gml, line 123:

<comment>Using `array_create(11, [])` for a grid introduces shared-row references: every slot points to the same empty array. That makes two-dimensional indexing fragile and can cause writes to one company to leak into others. It also leaves each row at length 0, which breaks downstream code that indexes up to 500. Keep the explicit `array_create_2d(11, 501, "")` helper (or initialize each row independently) so each company gets its own 501-length array.</comment>

<file context>
@@ -118,29 +118,23 @@ veh_acc = array_create_2d(_max_companies, _max_vehicles, "");
+race = array_create(11, 501);
 /// @type {Array<Array<String>>}
-name = array_create_2d(11, 501, "");
+name = array_create(11, []);
 /// @type {Array<Array<String>>}
-role = array_create_2d(11, 501, "");
</file context>
Suggested change
name = array_create(11, []);
name = array_create_2d(11, 501, "");

Comment thread scripts/scr_world_time/scr_world_time.gml Outdated
static specials = function() {
return obj_ini.spe[company][marine_number];
};
specials = "";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Existing saves lose all encoded psychic-power specials after load because the replacement source is initialized to an empty string rather than migrated from legacy obj_ini.spe. Initialize from spe when it exists, then retain this per-marine field going forward.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_marine_struct/scr_marine_struct.gml, line 1052:

<comment>Existing saves lose all encoded psychic-power specials after load because the replacement source is initialized to an empty string rather than migrated from legacy `obj_ini.spe`. Initialize from `spe` when it exists, then retain this per-marine field going forward.</comment>

<file context>
@@ -1048,12 +1049,10 @@ function TTRPG_stats(faction, comp, mar, class = "marine", other_spawn_data = {}
-    static specials = function() {
-        return obj_ini.spe[company][marine_number];
-    };
+    specials = "";
 
     static specials_array = function() {
</file context>
Suggested change
specials = "";
specials = variable_instance_exists(obj_ini, "spe") ? obj_ini.spe[company][marine_number] : "";

for (var q = 0; q < array_length(obj_ini.TTRPG[0]); q++) {
var _unit = fetch_unit([0, q]);
if (_unit.role() == obj_ini.role[100][eROLE.CHAPTERMASTER]) {
if (string_count("0", _unit.special) > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This reads _unit.special (singular), but the psyker-powers field on the unit struct is specials (plural). Since special is undefined, string_count("0", undefined) returns 0, so the 'Shadow in the Warp' popup/event-log for a psyker Chapter Master will never trigger even when the master has psyker powers. Should be _unit.specials.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml, line 170:

<comment>This reads `_unit.special` (singular), but the psyker-powers field on the unit struct is `specials` (plural). Since `special` is undefined, `string_count("0", undefined)` returns 0, so the 'Shadow in the Warp' popup/event-log for a psyker Chapter Master will never trigger even when the master has psyker powers. Should be `_unit.specials`.</comment>

<file context>
@@ -164,9 +164,10 @@ function scr_enemy_ai_d() {
+                for (var q = 0; q < array_length(obj_ini.TTRPG[0]); q++) {
+                    var _unit = fetch_unit([0, q]);
+                    if (_unit.role() == obj_ini.role[100][eROLE.CHAPTERMASTER]) {
+                        if (string_count("0", _unit.special) > 0) {
                             scr_popup("Shadow in the Warp", "You are distracted and bothered by a nagging sensation in the warp.  It feels as though a shadow descends upon your sector.", "shadow", "");
                             scr_event_log("red", "You sense a disturbance in the warp.  It feels something like a massive shadow.");
</file context>

break;
}
}
if (good == -1) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Custom agent: Code Quality Review

The find_company_open_slot function mutates persistent obj_ini arrays when no open slot exists, but its name implies a simple read-only lookup. Hiding array-expansion side effects inside a finder makes it easy for callers to accidentally alter game state. Consider renaming this function to something like find_or_create_company_open_slot, or move the expansion logic out to the caller so the side effect is explicit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_company_view/scr_company_view.gml, line 52:

<comment>The `find_company_open_slot` function mutates persistent `obj_ini` arrays when no open slot exists, but its name implies a simple read-only lookup. Hiding array-expansion side effects inside a finder makes it easy for callers to accidentally alter game state. Consider renaming this function to something like `find_or_create_company_open_slot`, or move the expansion logic out to the caller so the side effect is explicit.</comment>

<file context>
@@ -49,6 +49,19 @@ function find_company_open_slot(target_company) {
             break;
         }
     }
+    if (good == -1) {
+        good = array_length(obj_ini.name[target_company]);
+        array_push(obj_ini.race[target_company], 0);
</file context>

return $"{check_number} {yf} {year}.M{millenium}";
}

static game_year = function(){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Custom agent: Code Quality Review

The raw constant 1000 (years per millennium) is repeated three times in two functions without a #macro or enum. Because this is newly added code and the project already uses #macro extensively for numeric constants, this should be named (e.g., #macro YEARS_PER_MILLENNIUM 1000) to ensure a single source of truth and easier maintenance.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_world_time/scr_world_time.gml, line 26:

<comment>The raw constant `1000` (years per millennium) is repeated three times in two functions without a `#macro` or `enum`. Because this is newly added code and the project already uses `#macro` extensively for numeric constants, this should be named (e.g., `#macro YEARS_PER_MILLENNIUM 1000`) to ensure a single source of truth and easier maintenance.</comment>

<file context>
@@ -0,0 +1,45 @@
+		return $"{check_number} {yf} {year}.M{millenium}";
+	}
+
+	static game_year = function(){
+		return (millenium * 1000) + year;
+	}
</file context>

Comment thread objects/obj_ini/Create_0.gml Outdated

/// @type {Array<Array<Real>>}
race = array_create_2d(11, 501, 0);
race = array_create(11, 501);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Custom agent: Code Quality Review

The marine array initializations repeat the raw dimension constant 11 across eight consecutive added lines instead of using the already-defined _max_companies constant. Using a named constant prevents drift between the array sizes and the intended capacity (e.g., vehicle arrays right above already use _max_companies). Consider replacing the raw 11 with _max_companies.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_ini/Create_0.gml, line 121:

<comment>The marine array initializations repeat the raw dimension constant `11` across eight consecutive added lines instead of using the already-defined `_max_companies` constant. Using a named constant prevents drift between the array sizes and the intended capacity (e.g., vehicle arrays right above already use `_max_companies`). Consider replacing the raw `11` with `_max_companies`.</comment>

<file context>
@@ -118,29 +118,23 @@ veh_acc = array_create_2d(_max_companies, _max_vehicles, "");
 
 /// @type {Array<Array<Real>>}
-race = array_create_2d(11, 501, 0);
+race = array_create(11, 501);
 /// @type {Array<Array<String>>}
-name = array_create_2d(11, 501, "");
</file context>

@OH296
OH296 force-pushed the ssingle_source_of_time branch from 631cba7 to e873e26 Compare August 2, 2026 18:01
@OH296

OH296 commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

/review

@opencode-agent

opencode-agent Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Reviewed the actual diff (e873e26 vs merge-base c1630ffcd). All findings below are verified against the code. Note: most of cubic's 32 findings reference files not in this PR (scr_kill_unit, scr_random_event, scr_dialogue, obj_ini/Create_0, Mouse_50, scr_company_view, scr_count_forces, scr_civil_roster, scr_enemy_ai_d, etc. — all from already-merged #1405); those are out of scope here.

Blockers

  • scr_marine_struct.gml:105 — new game crashes before it starts. born = obj_controller.sector_handler.game_year(); is unguarded, but TTRPG_stats is first constructed inside scr_initialize_custom() from obj_ini's Create in rm_creation, where obj_controller does not exist (it's only in rm_game). Unset-variable runtime error on every chapter creation.
  • scr_world_time.gml:44get_time_from_current_year uses built-in age (step counter) instead of age_from_year. Silent wrong math, and it underpins both age() and recoverable_geneseed(). (This is the one cubic got right.)
  • scr_world_time.gml:15date() reads obj_controller.year_fraction, a field this PR deleted. Crashes whenever year_fraction < 10 (i.e. the first 10 turns, including the intro banner at Create_0.gml:1384).
  • Load is completely broken. SectorHandler is constructed at obj_controller/Create_0.gml:78, before the load branch (line 1172) destroys/recreates obj_ini with millenium = 0 (and from main menu, obj_ini doesn't exist at all). The constructor's obj_ini.millenium != 0 guard then never passes → check_number/year_fraction/year/millenium undefined → date(), game_year(), increment_date() all error on every load. There are no defaults (scr_world_time.gml:1-9), unlike the removed controller code which always assigned year = 0; millenium = 0 first. Also: increment_date() never writes back to obj_ini, so saves persist the stale creation date and loaded games revert to M41.735 forever.

High

  • scr_marine_struct.gml:933recoverable_geneseed() reads unit.gene_seed_mutations, but unit is unset in the method scope → crash on any Astartes death with an apothecary (obj_p_assra/Alarm_0.gml:21, scr_after_combat.gml:306). Should be gene_seed_mutations.
  • scr_marine_struct.gml:260marine_ascension = obj_controller.sector_handler.game_year; missing () → stores the method, not a year; every consumer gets NaN.
  • scr_initialize_custom.gml:2000var _game_year = obj_controller.sector_handler.game_year; same missing ()born/marine_ascension of all 5501 blank marine slots become NaN.
  • obj_controller/Create_0.gml:576,579penitent_end = millenium + year + ... still reads the removed fields → crash when the Blood Debt disadvantage is selected (untouched by the PR, broken by it).
  • scr_add_man.gml:124_unit.age = (obj_controller.millenium * 1000) + obj_controller.year; → crash on adding non-marine-role units, and even if guarded, assigns a number over the new age method, clobbering it (cubic missed both this and the load breakage).

Minor

  • scr_after_combat.gml:306 — one 80% roll for the whole bundle vs. prior 90%-per-seed (E[harvest] 1.8 → 1.6, all-or-nothing). If not intentional, roll per seed.
  • Save migration: old saves lack born/marine_ascension/specials/god_status on TTRPG structs; age()/recoverable_geneseed() will break on them (moot until load is fixed).
  • scr_world_time.gml:34 year-boundary overflow drop is identical to the old code — pre-existing, not this PR (cubic flagged it as new).
  • Cubic's two "✅ Addressed in e873e26" markers are false — that's the PR commit itself and the code is unfixed.
  • Style: new SectorHandler(); trailing space (Create_0.gml:78), mixed tabs/spaces and missing trailing newline in scr_world_time.gml (GoboCat will churn); 1000 repeated 3× — worth a #macro YEARS_PER_MILLENNIUM.

The centralization idea is sound, but the handler is wired to obj_ini state that (a) isn't ready at construction on load, (b) is never updated after increment_date(), and (c) doesn't exist when TTRPG_stats is constructed in rm_creation. I'd block until A, B, C, and the load path are fixed.

github run

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 6 files (changes from recent commits).

Confidence score: 4/5

  • In scripts/scr_marine_struct/scr_marine_struct.gml, the fallback born = 4000 introduces a conflicting timeline baseline, so units created before init can get incorrect age/year data and downstream logic drift despite the “single source of truth” intent—replace the magic number with the canonical timeline source (or defer until init is available) and document the fallback behavior.
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_marine_struct/scr_marine_struct.gml">

<violation number="1" location="scripts/scr_marine_struct/scr_marine_struct.gml:110">
P2: The fallback `born = 4000` is an unexplained magic number that contradicts the game's real timeline (obj_ini.millenium = 41 → game_year() ≈ 41000) and the PR's "single source of truth" goal. Any unit constructed before obj_controller exists that doesn't get born overridden (e.g. the blank + load path in obj_ini/Create_0) would carry a birth year ~37,000 years in the past, so age() (get_time_from_current_year(born)) returns an implausible value. Consider deriving the fallback from a named start-of-game constant (M41.000 = 41000) or documenting 4000, so it can't silently drift from the sector_handler's canonical time.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread objects/obj_ini/Create_0.gml Outdated
if (instance_exists(obj_controller)){
born = obj_controller.sector_handler.game_year();
} else {
born = 4000;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The fallback born = 4000 is an unexplained magic number that contradicts the game's real timeline (obj_ini.millenium = 41 → game_year() ≈ 41000) and the PR's "single source of truth" goal. Any unit constructed before obj_controller exists that doesn't get born overridden (e.g. the blank + load path in obj_ini/Create_0) would carry a birth year ~37,000 years in the past, so age() (get_time_from_current_year(born)) returns an implausible value. Consider deriving the fallback from a named start-of-game constant (M41.000 = 41000) or documenting 4000, so it can't silently drift from the sector_handler's canonical time.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_marine_struct/scr_marine_struct.gml, line 110:

<comment>The fallback `born = 4000` is an unexplained magic number that contradicts the game's real timeline (obj_ini.millenium = 41 → game_year() ≈ 41000) and the PR's "single source of truth" goal. Any unit constructed before obj_controller exists that doesn't get born overridden (e.g. the blank + load path in obj_ini/Create_0) would carry a birth year ~37,000 years in the past, so age() (get_time_from_current_year(born)) returns an implausible value. Consider deriving the fallback from a named start-of-game constant (M41.000 = 41000) or documenting 4000, so it can't silently drift from the sector_handler's canonical time.</comment>

<file context>
@@ -102,7 +102,13 @@ function TTRPG_stats(faction, comp, mar, class = "marine", other_spawn_data = {}
+    if (instance_exists(obj_controller)){
+        born = obj_controller.sector_handler.game_year();
+    } else { 
+        born = 4000;
+    }
 
</file context>

@OH296

OH296 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

/review

@opencode-agent

opencode-agent Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Reviewed the PR diff (1f8fc3bc9..HEAD) directly. The current HEAD still carries the two gene-seed blockers and happens to lose the save/load date — several "✅ addressed" marks from the earlier cubic run were reverted by the latest merge.

Blockers

  • scripts/scr_marine_struct/scr_marine_struct.gml:266marine_ascension = obj_ini.sector_handler.game_year; missing (). The fixed version (game_year()) from commit e873e26 got lost in the re-merge. Stored as a method:
    • recoverable_geneseed() (line 939) computes game_year() - <method> → type error.
    • scr_kill_unit.gml:62,68 gates (method > 30) now throw instead of the numeric compare it had at the base.
    • scr_unit_detail_text renders the method as the ascension date.
  • scr_marine_struct.gml:941recoverable_geneseed() reads unit.gene_seed_mutations; unit is undefined in the method scope. Should be gene_seed_mutations. Every marine death with an Apothecary available crashes (obj_p_assra/Alarm_0.gml:21, scr_after_combat).
  • Save/load date is never persistedsector_handler is a struct, so copy_serializable_fields skips it (logs a warning), serialize() never adds it to save_data, and the struct_exists(save_data,"sector_handler") branch in deserialize() can never be true (obj_ini/Create_0.gml:352). Loaded games always revert to the M41.735 constructor default. Worse, base-merge loads correctly restored obj_ini.millenium/year; those fields no longer exist, so old saves also lose their date. Real regression for this feature's own goal (single source of truth that survives load).
  • born = 4000 fallback (line 110) — magic number contradicting the M41 timeline. Any unit constructed before obj_controller (Chapter Master etc. in rm_creation) gets age() ≈ 37,000 years. Guard reads obj_ini.sector_handler but checks instance_exists(obj_controller); use a named start-of-game constant or defer.

High

  • _init_marine_row deleted, rows left as array_create(11, []) (the refactor: Remove static company sizes and KILL parallel arrays #1405 change, but this PR removed the only code that created proper rows). The HQ loop then auto-grows one shared array from all 11 companies → company rosters are aliases of one row. role[101] = role[100] likewise aliases. Re-create per-company rows at the top of the init.
  • Gene-seed recovery behavior change (scr_after_combat): one 80% roll for the whole package vs. 90% per seed; all-or-nothing. seed_lost now also always counted even on no recovery. Confirm this is intended.
  • New-game cramp — blood starter rows read obj_ini.race[co][i] == 1 for all slots; with the shared/grown rows that's a new set of pre-1970 offsets. Covered by the row fix above.

Minor

  • obj_creation shifter clamps: current_value - shift_value >= min_clamp with shift_value:100 means the year shifter can't reach 0 or 900 (default 735 floors at 35, caps at 835). Clamp check should be on current_value, not current_value ± shift_size.
  • scr_world_time.gml: tab/space indentation, #macro YEARS_PER_MILLENNIUM for the triple 1000, add trailing newline. new SectorHandler(); has a trailing space (.gml:143).
  • The born guard reading obj_ini under an instance_exists(obj_controller) test — guard the actual dependency (obj_ini), marine_ascension (line 259) has the same mismatch.
  • date() now correctly uses year_fraction (the obj_controller.year_fraction crash from the first diff is fixed), but get_time_from_current_year(age_from_year) also got fixed and then not regressed — it now reads age_from_year correctly.

Recommendation: rebase/cherry-pick the lost fixes (missing () at line 266, unitgene_seed_mutations at line 941) and decide the date-persistence path (manual sector_handler serialization) before merge. The shared-row init and the recovery/review are behavioral decisions you may want to confirm.

Notably, several of cubic's findings target files not in this PR (scr_kill_unit, scr_dialogue, scr_random_event, scr_civil_roster, controller helpers) — those are breaks that this PR causes through changes in marine_ascension/specials/god_status and are worth migrating after the load path is fixed.

github run

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 17 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread objects/obj_ini/Create_0.gml
Comment thread scripts/scr_marine_struct/scr_marine_struct.gml
Comment thread objects/obj_creation/Create_0.gml

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread scripts/scr_kill_unit/scr_kill_unit.gml
Comment thread scripts/scr_unit_detail_text/scr_unit_detail_text.gml Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/scr_marine_struct/scr_marine_struct.gml Outdated
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

0 issues found across 1 file (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 25 unresolved issues from previous reviews.

Re-trigger cubic

@OH296
OH296 merged commit 065dde3 into Adeptus-Dominus:main Aug 3, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: Medium Type: Fix This is a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant