refactor: Artifact API, kill parallel arrays

[EttyKitty]

Summary by cubic

Refactors the artifact system to a struct-based API centered on ArtifactStruct, replacing parallel arrays, removing magic ship IDs, and tightening UI/events/equip flows to prevent crashes and bad state. Adds JSDoc annotations for clearer types and safer calls. Also removes legacy save migration.

• Refactors

  • Introduced ArtifactStruct with getters/setters; artifacts now live in obj_ini.artifact_map and are read via fetch_artifact(id). Reset ArtifactStruct.__next_id on custom init; added JSDoc types across scripts.
  • Moved scr_add_artifact into scripts/artifact_functions (returns new artifact ID) and added helpers like build_sorted_artifact_ids(); Librarium derives counts and initial selection from sorted IDs.
  • Normalized ship assignment: artifacts store raw ship indices; removed +500 offsets; updated fleet moves, ship removal (uses stored index for shifts), and turn init to relocate artifacts and ensure location names.
  • Updated UI/events/quests/missions/popups to use .get_type_name(), .get_description(), .get_bearer_text(), .has_tag(), .is_equipped(), .is_heretical(); dropdown/tooltips append bearer info and event-display options derive from NOT_EQUIPPABLE; corruption checks read artifact tags via struct.
  • Hardened equip/data checks: avoid “no change” when re-selecting the same artifact, mass-equip aggregates only non-real slot items, struct/null guards in dropdowns/events/popups, units drop equipped artifacts on death, and combat sizing/specialist checks use unit getters.

• Migration

  • Iterate with struct_get_names(obj_ini.artifact_map) and fetch via obj_ini.artifact_map[$ key] or fetch_artifact(id); removed legacy artifact* arrays and fest_display_tags; fest_display and menu_artifact now use -1.
  • Pass raw ship index to scr_add_artifact(..., ship_id) and manage location via struct setters.
  • Removed save migration for old artifacts; existing saves may be incompatible.

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

[EttyKitty]

/review

[EttyKitty]

/review

[cubic-dev-ai]

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

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

Re-trigger cubic</sub>

[cubic-dev-ai]

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

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

Re-trigger cubic</sub>

[EttyKitty]

/review Last time. I want to merge this already. Please god, help me. I don't want to fix anything else.

[opencode-agent]

Review complete. Verified against HEAD (ce725e1), including all prior review claims. Most round-1/2 issues are genuinely fixed (sid shifting in scr_kill_ship uses get_stored_ship_id(), Alarm_7 excludes equipped artifacts, scr_add_artifact invalidates the sorted cache via set_chapter_arti_data(), delete_artifact clears fest_display, unequip matches by artifact_id, > -1 sentinel guards everywhere fest_display is fetched, no leftover artifact* array references).

High

1. Unguarded deref still in inquisitor_inspect_units — scr_inquisition_inspection.gml:136. fetch_artifact asserts (non-halting popup) and returns undefined; artifact.is_heretical() on undefined throws. Flagged by three separate reviews; the is_struct guard was never restored. Add if (is_struct(artifact) && artifact.is_heretical()).

2. Legacy saves now lose artifacts silently — obj_ini/Create_0.gml:345-352. Old parallel arrays are excluded from auto-load (lines 248-256) and never migrated; only artifact_list is consumed. Intentional ("Saves will break anyway"), but there's no LOGGER.error() when struct_exists(save_data, "artifact") and no artifact_list — the last cubic review flagged exactly this. Fail loud or the next "why did my artifacts vanish" bug report is a no-repro.

3. scr_turn_first overwrites star location with ship name — scr_turn_first.gml:25. Old code only set the ship id; new code also stamps set_location_name(_arti_fleet.capital[0]). Artifacts relocated onto a fleet ship stop matching star-scoped checks (get_location_name() == star.name at scr_inquisition_inspection.gml:170,286), so star inspections/arsenal undercount after any fleet-identification tick. If ship-name-is-location is the intended new model, the star-scoped checks need to resolve via obj_ini.ship_location[get_ship_id()] instead; otherwise keep the star name.

Medium

4. Mass-equip tally drops artifact gear — obj_mass_equip/Step_0.gml:217. Old code counted the slot string (artifact type name); !is_real now silently excludes artifact ids from "In total they are equipped with". Resolve fetch_artifact(id).get_type_name() before adding, or the summary undercounts artifact-equipped roles.

5. fetch_artifact assert-on-missing contract leaks into hot paths — scr_marine_struct.gml:410 (x5 accessors), equipped_artifact_tag:2086, scr_unit_equip_functions.gml:13/92/103/147/200/210/etc, obj_dropdown_sel/Draw_0.gml:33+75. Every caller must guard or a single stale id (partial load, mid-open dropdown deletion) crashes in Draw/Step after the assert popup. Cheapest fix: is_struct fallback in the 5 marine accessors (is_struct(arti) ? arti.get_type_name() : wep) — those are the highest-traffic callers.

6. Dropdown tooltip block still duplicated — obj_dropdown_sel/Draw_0.gml:31-39 and 73-81, unguarded fetches in both. Extract get_artifact_tooltip(id) and guard.

Minor

7. Dead legacy branch: artifact_functions.gml:311-313 sid >= 500 conversion — from_json reads data.ship_id, never the converted sid, and no released save has artifact_list with old keys. Harmless, but misleading; drop it or wire it into from_json fallbacks.
8. equip_on_unit (ArtifactStruct.gml:340) silently returns on _result != "complete" — fine now that mobility returns "no change", but a LOGGER line would make equip failures diagnosable.
9. JSDoc still incomplete on most new ArtifactStruct methods (is_identifiable, has_tag, destroy_artifact, unequip_from_unit, equip_on_unit, is_equipped, get_type, setters, privates) — CODE_STYLE requires @desc/@param/@returns.
10. set_chapter_arti_data resets menu_artifact to the first artifact on every acquisition — mid-librarium acquisitions jump the selection.

Direction is sound and the mechanical conversion is thorough; #1 (one-line guard) and a fail-loud legacy-save notice are the only merge blockers.

github run

[cubic-dev-ai]

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.

<sub>Requires human review: Auto-approval blocked by 30 unresolved issues from previous reviews.

Re-trigger cubic</sub>