Stop stealing/reaping cutscene JSON docs - parse in place instead

cutsceneSystemLoad, cutsceneCutsceneResolve and
cutsceneInsertResolveAndSplice all used to steal ownership of the
asset entry's parsed yyjson_doc (nulling entry->data.json) and force
an immediate assetReapUnused() to avoid handing back a corrupted
cache entry on a later load of the same file - which doubled as a
latent crash if that reap was ever skipped (as it briefly was),
since a stale cache hit would then have a NULL doc.

Turns out none of that was needed: every cutsceneitem_t field that
could reference the doc's memory already copies its string data by
value (cutsceneJsonCopyString into fixed buffers), so the parsed
items don't depend on the doc surviving past cutsceneParseDoc. All
three call sites now parse straight out of entry->data.json while
still locked, then unlock normally - the asset entry stays a normal,
correctly-cached resource, and CUTSCENE_LOADED_DOC/cutsceneLoadedSetDoc
are gone entirely.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

src/dusk/rpg/cutscene/cutscenesystem.c

@@ -16,8 +16,6 @@
 
 cutscenesystem_t CUTSCENE_SYSTEM;
 
-static yyjson_doc *CUTSCENE_LOADED_DOC = NULL;
-
 void cutsceneSystemInit() {
   memoryZero(&CUTSCENE_SYSTEM, sizeof(cutscenesystem_t));
   CUTSCENE_SYSTEM.loadedScene.items = CUTSCENE_SYSTEM.loadedItems;
@@ -37,8 +35,6 @@ void cutsceneSystemDispose() {
   CUTSCENE_SYSTEM.textCache[0] = '\0';
   CUTSCENE_SYSTEM.onComplete = NULL;
   CUTSCENE_SYSTEM.loadedFile[0] = '\0';
-
-  cutsceneLoadedSetDoc(NULL);
 }
 
 void cutsceneSystemPrepare(
@@ -120,41 +116,26 @@ void cutsceneSystemLoad(const char_t *file) {
     return;
   }
 
-  // Steal the parsed doc before unlocking - assetJsonDispose would
+  // Parse straight out of the entry's own doc while still locked - every
-  // otherwise free it out before it's been parsed below.
+  // cutsceneitem_t field that could reference it owns its string data by
-  yyjson_doc *doc = entry->data.json;
+  // value (see cutscenecutsceneref_t/cutscenemarker_t's doc comments), so
-  entry->data.json = NULL;
+  // nothing needs to survive past this call and the entry can just be
-  assetUnlockEntry(entry);
+  // unlocked like any other asset once parsing finishes.
-  // Force this now-zero-ref entry to actually go away right away, rather
-  // than leaving it languishing as LOADED until something else happens to
-  // trigger a reap - assetRequireLoaded treats an already-LOADED entry as
-  // an instant cache hit with no reparsing, so a stale leftover entry for
-  // this same path would otherwise make a later load of it wrongly skip
-  // reloading, still returning CUTSCENE_SYSTEM.loadedScene even after
-  // some other cutscene has since overwritten it.
-  assetReapUnused();
-
   errorret_t parseResult = cutsceneParseDoc(
-    doc, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems,
+    entry->data.json, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems,
     CUTSCENE_LOADED_ITEMS_MAX
   );
+  assetUnlockEntry(entry);
   if(errorIsNotOk(parseResult)) {
-    yyjson_doc_free(doc);
     errorCatch(errorPrint(parseResult));
     uiFatalErrorOpen(NULL);
     return;
   }
 
-  cutsceneLoadedSetDoc(doc);
   cutsceneSystemStartCutscene(&CUTSCENE_SYSTEM.loadedScene);
   stringCopy(CUTSCENE_SYSTEM.loadedFile, file, ASSET_FILE_NAME_MAX - 1);
 }
 
-void cutsceneLoadedSetDoc(yyjson_doc *doc) {
-  if(CUTSCENE_LOADED_DOC != NULL) yyjson_doc_free(CUTSCENE_LOADED_DOC);
-  CUTSCENE_LOADED_DOC = doc;
-}
-
 void cutsceneRestart(void) {
   assertNotNull(
     CUTSCENE_SYSTEM.scene, "cutsceneRestart called with no cutscene running"

src/dusk/rpg/cutscene/cutscenesystem.h

@@ -158,41 +158,32 @@ void cutsceneSystemInsertCutscene(const cutscene_t *cutscene);
  * Loads and immediately starts a cutscene asset by file name, e.g.
  * cutsceneSystemLoad("main_menu.jsonc") loads and starts
  * assets/cutscenes/main_menu.jsonc, parsing it into
- * CUTSCENE_SYSTEM.loadedScene/loadedItems - the underlying JSON asset
+ * CUTSCENE_SYSTEM.loadedScene/loadedItems. Locks the underlying JSON
- * entry is unlocked and reaped immediately once that's done (see
+ * asset entry just long enough to parse it, then unlocks it like any
- * CUTSCENE_LOADED_ITEMS_MAX's doc comment for why), so it's always
+ * other asset - every cutsceneitem_t field that could reference the
- * re-read+re-parsed fresh, never assumed still resident from a previous
+ * parsed doc owns its string data by value (see cutscenecutsceneref_t/
- * call. Opens the fatal error overlay (see uiFatalErrorOpen) instead of
+ * cutscenemarker_t's doc comments), so nothing needs to keep it resident
- * starting anything if the asset fails to load. Records file into
+ * past this call, and a later load of the same file is a normal cache
- * CUTSCENE_SYSTEM.loadedFile once it starts running.
+ * hit (or a fresh re-read, if the entry was reaped meanwhile) rather than
+ * something this function has to force either way. Opens the fatal error
+ * overlay (see uiFatalErrorOpen) instead of starting anything if the
+ * asset fails to load. Records file into CUTSCENE_SYSTEM.loadedFile once
+ * it starts running.
  *
  * @param file Cutscene file name (with .jsonc extension), relative to
  * assets/cutscenes/.
  */
 void cutsceneSystemLoad(const char_t *file);
 
-/**
- * Frees whatever yyjson_doc was parsed into CUTSCENE_SYSTEM.loadedItems
- * last and takes ownership of doc instead. Called by
- * cutsceneSystemLoad/cutsceneCutsceneResolve once a fresh parse into
- * loadedItems completes successfully, and from cutsceneSystemDispose to
- * release the last one at shutdown.
- *
- * @param doc The new doc to take ownership of (NULL just frees/clears the
- * current one).
- */
-void cutsceneLoadedSetDoc(yyjson_doc *doc);
-
 /**
  * Restarts the currently running cutscene from its first item,
  * preserving whatever interact/interacted entities triggered it and
  * whatever completion callback was armed. If CUTSCENE_SYSTEM.loadedFile
  * is set (i.e. the running cutscene came from cutsceneSystemLoad), this
- * re-reads and re-parses that file via cutsceneSystemLoad rather than
+ * re-invokes cutsceneSystemLoad on that same filename rather than just
- * just rerunning whatever's still resident in loadedScene/loadedItems, so
+ * rerunning whatever's still resident in loadedScene/loadedItems -
- * a restart always reflects the file's current contents - otherwise it's
+ * otherwise it's just cutsceneSystemStartCutsceneWith on the same
- * just cutsceneSystemStartCutsceneWith on the same cutscene_t. Asserts if
+ * cutscene_t. Asserts if no cutscene is running.
- * no cutscene is running.
  */
 void cutsceneRestart(void);
 

src/dusk/rpg/cutscene/item/cutsceneitem.c

@@ -410,32 +410,22 @@ cutscene_t * cutsceneCutsceneResolve(const char_t *name) {
     return NULL;
   }
 
-  // Steal the parsed doc before unlocking - assetJsonDispose would
+  // Parse straight out of the entry's own doc while still locked - every
-  // otherwise free it out before it's been parsed below.
+  // cutsceneitem_t field that could reference it owns its string data by
-  yyjson_doc *doc = entry->data.json;
+  // value (see cutscenecutsceneref_t/cutscenemarker_t's doc comments), so
-  entry->data.json = NULL;
+  // nothing needs to survive past this call and the entry can just be
-  assetUnlockEntry(entry);
+  // unlocked like any other asset once parsing finishes.
-  // Force this now-zero-ref entry to actually go away right away, rather
-  // than leaving it languishing as LOADED until something else happens to
-  // trigger a reap - assetRequireLoaded treats an already-LOADED entry as
-  // an instant cache hit with no reparsing, so a stale leftover entry for
-  // this same path would otherwise make a later resolve of it wrongly
-  // skip loading, still returning CUTSCENE_SYSTEM.loadedScene even after
-  // some other cutscene has since overwritten it.
-  assetReapUnused();
-
   errorret_t parseResult = cutsceneParseDoc(
-    doc, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems,
+    entry->data.json, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems,
     CUTSCENE_LOADED_ITEMS_MAX
   );
+  assetUnlockEntry(entry);
   if(errorIsNotOk(parseResult)) {
-    yyjson_doc_free(doc);
     errorCatch(errorPrint(parseResult));
     uiFatalErrorOpen(NULL);
     return NULL;
   }
 
-  cutsceneLoadedSetDoc(doc);
   return &CUTSCENE_SYSTEM.loadedScene;
 }
 
@@ -501,25 +491,17 @@ void cutsceneInsertResolveAndSplice(const char_t *name) {
     return;
   }
 
-  // Steal the parsed doc before unlocking - assetJsonDispose would
-  // otherwise free it out before it's been parsed below.
-  yyjson_doc *doc = entry->data.json;
-  entry->data.json = NULL;
-  assetUnlockEntry(entry);
-  // See cutsceneCutsceneResolve's matching comment - forces a stale
-  // leftover entry for this same path out immediately, so a later resolve
-  // never wrongly skips reloading it.
-  assetReapUnused();
-
   // Local scratch, not a shared buffer - this target is spliced straight
   // into the running scene below and never referenced again afterward, so
-  // it only needs to live for the rest of this call (see this function's
+  // it only needs to live for the rest of this call. Parsed straight out
-  // doc comment for why the doc can be freed immediately too).
+  // of the entry's own doc while still locked, then unlocked like any
+  // other asset - see cutsceneCutsceneResolve's matching comment for why
+  // nothing needs to keep the doc itself resident past the parse.
   cutsceneitem_t items[CUTSCENE_INSERT_ITEMS_MAX];
   cutscene_t scene = { .items = items };
   errorret_t parseResult =
-    cutsceneParseDoc(doc, &scene, items, CUTSCENE_INSERT_ITEMS_MAX);
+    cutsceneParseDoc(entry->data.json, &scene, items, CUTSCENE_INSERT_ITEMS_MAX);
-  yyjson_doc_free(doc);
+  assetUnlockEntry(entry);
   if(errorIsNotOk(parseResult)) {
     errorCatch(errorPrint(parseResult));
     uiFatalErrorOpen(NULL);

src/dusk/rpg/cutscene/item/cutsceneitem.h

@@ -227,8 +227,9 @@ void cutsceneCutsceneStart(
 /**
  * Resolves a JSON-authored CUTSCENE item's referenced cutscene by name:
  * locks+loads assets/cutscenes/<name>.jsonc, parses it into
- * CUTSCENE_SYSTEM.loadedItems/loadedScene, then unlocks and reaps the asset entry
+ * CUTSCENE_SYSTEM.loadedItems/loadedScene, then unlocks the asset entry
- * immediately (see CUTSCENE_LOADED_ITEMS_MAX's doc comment for why).
+ * like any other asset - the parsed items don't reference the doc past
+ * that (see cutsceneInsertResolveAndSplice's matching comment for why).
  * Called lazily from cutsceneCutsceneStart the first time the item
  * actually runs, not at parse time, so a cutscene referencing many others
  * doesn't load every one of them up front. On failure, opens the fatal
@@ -315,14 +316,14 @@ void cutsceneInsertStart(
  * splices it into the running cutscene (CUTSCENE_SYSTEM.scene): locks+
  * loads assets/cutscenes/<name>.jsonc as a plain ASSET_LOADER_TYPE_JSON
  * asset, parses it into a small scratch buffer local to this call
- * (capacity CUTSCENE_INSERT_ITEMS_MAX), then immediately frees the parsed
+ * (capacity CUTSCENE_INSERT_ITEMS_MAX) straight out of the asset entry's
- * doc - unlike cutsceneCutsceneResolve's CUTSCENE_SYSTEM.loadedScene, nothing
+ * own doc, then unlocks the entry like any other asset - nothing needs
- * needs to persist past this call: cutsceneSystemInsertCutscene copies the
+ * to keep the doc itself resident past the parse: every cutscene item
- * scratch items into the running scene's own array, and every cutscene
+ * field that could reference it owns its string data by value (see
- * item field that could reference the doc's memory owns its string data
+ * cutscenecutsceneref_t/cutscenemarker_t's doc comments), and
- * by value (see cutscenecutsceneref_t/cutscenemarker_t's doc comments), so
+ * cutsceneSystemInsertCutscene copies the scratch items into the running
- * the doc is disposable the moment parsing finishes. On failure, opens the
+ * scene's own array right after. On failure, opens the fatal error
- * fatal error overlay, matching cutsceneCutsceneResolve.
+ * overlay, matching cutsceneCutsceneResolve.
  *
  * @param name Bare cutscene name (without "cutscenes/" prefix or
  * ".jsonc" suffix).