fix: migrate marine specials from the legacy spe array

[The-Real-Nyx]

Loading a save made before the obj_ini.spe parallel array refactor crashed as soon as you opened a Librarian's panel in the company management view. specials used to be a method reading obj_ini.spe[company][marine_number], and jsonify_marine_struct() skips methods so old saves carry no specials key at all, while powers_known was a plain field and round-tripped fine. psy_discipline() then reads _specials_array[0] on an empty array throwing an error. Fresh saves post refactor do not create the issue and legacy saves will be non-breaking with the refactor after first load and stored non-breaking after first fresh save.

Changes

• Add migrate_legacy_marine_specials() to obj_ini deserialize, restoring specials from the legacy spe array when present. Skips marines that already carry their own string, and is bounded by TTRPG so the padding rows past company 10 never reach fetch_unit() and thus can't create cascading errors if any legacy save put data in them.
• Guard psy_discipline() against an empty specials, throw an error when a unit has powers but no discipline. This pairing cannot legitimately occur, so the guard acts as catch in case the migration shim creates weird broken marines in some circumstance or else a marine somehow ends up with specials but no discipline. Marines with no discipline but no specials will just get gracefully ejected instead as the most likely case is an unguarded call to psy_discipline() created them and they present no issue regardless.

Testing

• Loaded old and new saves side by side. The crash only reproduces on legacy saves without the migration shim, and the migration causes no observable issues.

---

Summary by cubic

Fixes a crash when opening the Librarian panel on legacy saves by restoring marine specials from the old spe array. Also prevents errors when a unit has no specials.

• Bug Fixes
  • Migrate legacy spe data during deserialize to repopulate each marine’s specials; skip marines that already have a value and bound iteration to TTRPG.
  • Guard psy_discipline to handle empty specials and log when powers exist without a discipline, preventing out-of-bounds errors on old saves.

^{Written for commit 09fe0c4. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 2 files

Confidence score: 3/5

• In objects/obj_ini/Create_0.gml, legacy save deserialization can iterate over out-of-range spe entries and repeatedly call fetch_unit(), causing exception-driven error popup spam and potentially disrupting load flow for affected users; clamp/filter spe values to the valid marine range before lookup (or skip invalid entries) to de-risk this path.

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_ini/Create_0.gml">

<violation number="1" location="objects/obj_ini/Create_0.gml:339">
P2: Loading a legacy save with a `spe` row longer than the valid marine range can call `fetch_unit()` for every out-of-range entry, triggering its exception handler and repeated error popups during deserialization. The migration currently bounds companies but not marine indices; bounding `_mar` to the actual `TTRPG[_coy]` row length (or the known valid maximum) would prevent this cascading load failure.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Loading a legacy save with a spe row longer than the valid marine range can call fetch_unit() for every out-of-range entry, triggering its exception handler and repeated error popups during deserialization. The migration currently bounds companies but not marine indices; bounding _mar to the actual TTRPG[_coy] row length (or the known valid maximum) would prevent this cascading load failure.

Prompt for AI agents

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

<comment>Loading a legacy save with a `spe` row longer than the valid marine range can call `fetch_unit()` for every out-of-range entry, triggering its exception handler and repeated error popups during deserialization. The migration currently bounds companies but not marine indices; bounding `_mar` to the actual `TTRPG[_coy]` row length (or the known valid maximum) would prevent this cascading load failure.</comment>

<file context>
@@ -319,6 +319,48 @@ deserialize = function(save_data) {
+                continue; // the legacy array is ragged; stray non-array rows carry nothing
+            }
+            var _row_length = array_length(_company_row);
+            for (var _mar = 0; _mar < _row_length; _mar++) {
+                var _legacy_string = _company_row[_mar];
+                if (!is_string(_legacy_string) || _legacy_string == "") {
</file context>

Suggested change

	for (var _mar = 0; _mar < _row_length; _mar++) {
	var _marine_count = min(_row_length, array_length(TTRPG[_coy]));
	for (var _mar = 0; _mar < _marine_count; _mar++) {

[OH296]

ok so when i merge the full deletion of all parallel arrays what ii will do is augment this function to iterate through all the destroyed arrays and fill all of the new variables in the struct, this way this will allow for a full migration

[github-actions]

This PR is marked as stale, because it has been open for 7 days with no activity.

[OH296]

The changes are so braod and varied i'm afriaid i just don't think it's going to be possible to acomodate reverse compatibility however i've been having a bit of a play around and thinking of implenting a migration sequence that will work a bit like db migration so each save logs the last commit it was connected too and runs all relevant migration scripts since that point if any relevent migrations are needed.

I will outline this when i have more of a plan but it should avoid these unfortunate occurrences happening

[The-Real-Nyx]

The changes are so braod and varied i'm afriaid i just don't think it's going to be possible to acomodate reverse compatibility however i've been having a bit of a play around and thinking of implenting a migration sequence that will work a bit like db migration so each save logs the last commit it was connected too and runs all relevant migration scripts since that point if any relevent migrations are needed.

I will outline this when i have more of a plan but it should avoid these unfortunate occurrences happening

Don't use commits for it. I mean I can't stop you but it's a bad way to do it. You want a monotonic int that tracks schema version so migrations are just an ordered list of pure functions that run in order on the deserialized inert data, which means the migrations prolly want to happen before obj_ini runs too.

But also I ain't the one making so do what your heart desires or whatever.

[OH296]

Don't use commits for it. I mean I can't stop you but it's a bad way to do it. You want a monotonic int that tracks schema version so migrations are just an ordered list of pure functions that run in order on the deserialized inert data, which means the migrations prolly want to happen before obj_ini runs too.

Ah ok appreciate the advice, other than db work i've never built a migration system like this before so any thoughts are great. I think given how our serialiser works the best place to slip in migrations would be after items are read but before ay custom mutations/parse methods take place but that would probably mean distributing migration methods across the code to each position where information is parsed into each object pro rata e.g obj_ini, obj_controller, obj_star so i just need to think of an elegant way of keeping of keeping the monotonic int concurrent across each obj, or do i have a separate int for each object?