From c9f098693c6f41f683171e4b586368dd0cf2ff28 Mon Sep 17 00:00:00 2001 From: Dominic Masters Date: Mon, 21 Sep 2026 15:04:41 -0500 Subject: [PATCH] Wire up battle AI/scene-return, reclaim cutscene item slots, fix GOTO - battleStateSelectionAiDecide: AI-controlled fighters now actually queue a basic attack against the first living enemy instead of doing nothing every round. - battleStateEndingInit: disposes the battle and switches back to SCENE_TYPE_OVERWORLD instead of leaving the game stuck in the battle scene forever. - cutsceneSystemNext now pops each finished item off the front of the running cutscene (cutsceneRemoveFront) instead of leaving it in place forever, so a long-running cutscene (many battle rounds splicing items in repeatedly) no longer grows scene->items unboundedly. - cutsceneGoTo reworked to match: since consumed items are evicted, a marker behind the current position can't be found in the live queue anymore, so it now re-parses the cutscene fresh from its source file (cutsceneLoadParse, into a persistent scratch buffer, not the stack) and replaces the live queue with marker-onward from that fresh parse. Only supported when the running cutscene is the one cutsceneSystemLoad/cutsceneCutsceneResolve populated. - Fixed a real bug this surfaced: cutsceneSystemPrepare unconditionally cleared loadedFile, which ran *after* cutsceneCutsceneResolve had just stamped it but *before* the cutscene started - wiping it out immediately and breaking cutsceneGoTo for any cutscene entered via a CUTSCENE item or cutsceneCutsceneResolve (e.g. the main menu's NEW_GAME/OPTIONS/QUIT navigation). Now only cleared when switching away from loadedScene entirely. - Added cutsceneLoadParse, the shared "lock asset, require loaded, parse items, unlock" sequence, replacing four near-identical copies across cutsceneSystemLoad/cutsceneCutsceneResolve/ cutsceneInsertResolveAndSplice/battleStateExecutingInit. Each call site handles its own fatal-error display; cutsceneLoadParse itself just propagates. - Added test_cutscene.c (cutsceneRemoveFront) and a regression test for the loadedFile bug. Writing the zero-count RemoveFront test caught a real self-move bug (count == 0 passed dest == src into memoryMove). Co-Authored-By: Claude Sonnet 5 --- src/dusk/rpg/battle/state/battlestateending.c | 10 +- src/dusk/rpg/battle/state/battlestateending.h | 3 +- .../rpg/battle/state/battlestateexecuting.c | 29 ++--- .../rpg/battle/state/battlestateselection.c | 22 +++- .../rpg/battle/state/battlestateselection.h | 23 +++- src/dusk/rpg/cutscene/cutscene.c | 50 ++++++++ src/dusk/rpg/cutscene/cutscene.h | 42 +++++++ src/dusk/rpg/cutscene/cutscenesystem.c | 115 ++++++++++++------ src/dusk/rpg/cutscene/cutscenesystem.h | 30 ++++- src/dusk/rpg/cutscene/item/cutsceneitem.c | 53 ++------ test/rpg/cutscene/CMakeLists.txt | 1 + test/rpg/cutscene/test_cutscene.c | 84 +++++++++++++ test/rpg/cutscene/test_cutscenesystem.c | 91 ++++++++++++-- 13 files changed, 430 insertions(+), 123 deletions(-) create mode 100644 test/rpg/cutscene/test_cutscene.c diff --git a/src/dusk/rpg/battle/state/battlestateending.c b/src/dusk/rpg/battle/state/battlestateending.c index 448171d4..2a195cbc 100644 --- a/src/dusk/rpg/battle/state/battlestateending.c +++ b/src/dusk/rpg/battle/state/battlestateending.c @@ -6,9 +6,17 @@ */ #include "battlestateending.h" +#include "rpg/battle/battle.h" +#include "scene/scene.h" void battleStateEndingInit(void) { - + // Tear down the battle and hand control back to the overworld. The + // overworld scene keeps no state of its own in scenedata_t (player/map + // state lives in the separate global entity/map systems, untouched by + // switching to/from SCENE_TYPE_BATTLE), so there's nothing else to + // restore here. + battleDispose(); + sceneSet(SCENE_TYPE_OVERWORLD); } void battleStateEndingUpdate(void) { diff --git a/src/dusk/rpg/battle/state/battlestateending.h b/src/dusk/rpg/battle/state/battlestateending.h index 56721c24..7b4fc581 100644 --- a/src/dusk/rpg/battle/state/battlestateending.h +++ b/src/dusk/rpg/battle/state/battlestateending.h @@ -13,7 +13,8 @@ typedef struct { } battlestateending_t; /** - * Called when the battle enters BATTLE_STATE_ENDING. + * Called when the battle enters BATTLE_STATE_ENDING. Disposes the battle + * (see battleDispose) and switches back to SCENE_TYPE_OVERWORLD. */ void battleStateEndingInit(void); diff --git a/src/dusk/rpg/battle/state/battlestateexecuting.c b/src/dusk/rpg/battle/state/battlestateexecuting.c index ce8bf499..d0045a38 100644 --- a/src/dusk/rpg/battle/state/battlestateexecuting.c +++ b/src/dusk/rpg/battle/state/battlestateexecuting.c @@ -10,8 +10,6 @@ #include "rpg/cutscene/item/cutsceneitem.h" #include "rpg/cutscene/cutscenesystem.h" #include "rpg/cutscene/cutscene.h" -#include "asset/asset.h" -#include "asset/loader/assetloader.h" #include "asset/assetfile.h" #include "ui/overlay/uifatalerror.h" #include "assert/assert.h" @@ -61,33 +59,20 @@ void battleStateExecutingInit(void) { "cutscenes/battle/moves/%s.jsonc", ability->name ); - // Load cutscene asset. - assetentry_t *entry = assetLock(path, ASSET_LOADER_TYPE_JSON, NULL); - errorret_t result = assetRequireLoaded(entry); + // Load and parse the move's cutscene straight into the tail of items. + const uint8_t moveItemsStart = itemsCount; + cutscene_t scene; + errorret_t result = cutsceneLoadParse( + path, &scene, &items[itemsCount], + BATTLE_STATE_EXECUTING_ITEMS_MAX - itemsCount + ); if(errorIsNotOk(result)) { - assetUnlockEntry(entry); errorCatch(errorPrint(result)); uiFatalErrorOpen(NULL); continue; } - - // Parse out cutscene items and append to the list. - const uint8_t moveItemsStart = itemsCount; - cutscene_t scene; - errorret_t parseResult = cutsceneParseDoc( - entry->data.json, &scene, &items[itemsCount], - BATTLE_STATE_EXECUTING_ITEMS_MAX - itemsCount - ); itemsCount += scene.itemCount; - // Release asset - assetUnlockEntry(entry); - if(errorIsNotOk(parseResult)) { - errorCatch(errorPrint(parseResult)); - uiFatalErrorOpen(NULL); - continue; - } - // Resolve BATTLE_FIGHTER_POS_TARGET/USER sentinels against this // action, now that its user/target fighters are known. for(uint8_t j = moveItemsStart; j < itemsCount; j++) { diff --git a/src/dusk/rpg/battle/state/battlestateselection.c b/src/dusk/rpg/battle/state/battlestateselection.c index 80ce4a31..22d08c55 100644 --- a/src/dusk/rpg/battle/state/battlestateselection.c +++ b/src/dusk/rpg/battle/state/battlestateselection.c @@ -32,6 +32,25 @@ bool_t battleStateSelectionIsComplete(void) { return BATTLE.stateData.selection.selectionIndex >= BATTLE.fighterOrderCount; } +void battleStateSelectionAiDecide(const uint8_t fighterIndex) { + battlefighter_t *fighter = BATTLE.fighters[fighterIndex]; + const battlefighterteam_t enemyTeam = + fighter->team == BATTLE_FIGHTER_TEAM_ALLY ? + BATTLE_FIGHTER_TEAM_ENEMY : BATTLE_FIGHTER_TEAM_ALLY; + + // Simple placeholder AI: attack the first living fighter on the + // opposing team. No move variety or target scoring yet. + for(uint8_t i = 0; i < BATTLE_FIGHTER_POS_COUNT; i++) { + battlefighter_t *candidate = BATTLE.fighters[i]; + if(candidate == NULL) continue; + if(candidate->team != enemyTeam) continue; + if(!battleFighterIsAlive(candidate)) continue; + + battleStateSelectionQueueAttack(fighterIndex, i); + return; + } +} + void battleStateSelectionUpdate(void) { battlestateselection_t *state = &BATTLE.stateData.selection; @@ -49,8 +68,7 @@ void battleStateSelectionUpdate(void) { battlefighter_t *fighter = BATTLE.fighters[fighterIndex]; if(fighter->controller != BATTLE_FIGHTER_CONTROLLER_AI) return; - // This is AI, in future I'll have proper AI here, for now I'm actually not - // going to do anything. + battleStateSelectionAiDecide(fighterIndex); state->selectionIndex++; } } diff --git a/src/dusk/rpg/battle/state/battlestateselection.h b/src/dusk/rpg/battle/state/battlestateselection.h index 71d8ee69..c4a7d1ba 100644 --- a/src/dusk/rpg/battle/state/battlestateselection.h +++ b/src/dusk/rpg/battle/state/battlestateselection.h @@ -20,11 +20,13 @@ typedef struct { void battleStateSelectionInit(void); /** - * Updates BATTLE_STATE_SELECTION for one frame: auto-decides for every - * AI-controlled fighter the cursor passes, stopping once it reaches a - * player-controlled fighter awaiting input (see - * battleStateSelectionGetCurrentFighter), or transitions to - * BATTLE_STATE_EXECUTING once every position has decided. + * Updates BATTLE_STATE_SELECTION for one frame: auto-decides (see + * battleStateSelectionAiDecide) for every AI-controlled fighter the + * cursor passes, stopping once it reaches a player-controlled fighter + * awaiting input (see battleStateSelectionGetCurrentFighter). Once every + * position has decided, BATTLE_WAIT_SELECTION (see + * cutsceneBattleWaitSelectionUpdate) queues the move to + * BATTLE_STATE_EXECUTING. */ void battleStateSelectionUpdate(void); @@ -52,6 +54,17 @@ bool_t battleStateSelectionFighterHasDecided(const uint8_t fighterIndex); */ bool_t battleStateSelectionFighterNeedsDecision(const uint8_t fighterIndex); +/** + * Decides and queues a move for an AI-controlled fighter: currently + * always a basic attack against the first living fighter on the + * opposing team (see battleStateSelectionQueueAttack). No-ops if there's + * no living target (the battle should already be over by then via + * BATTLE_POST_MOVE). + * + * @param fighterIndex Index into BATTLE.fighters of the deciding fighter. + */ +void battleStateSelectionAiDecide(const uint8_t fighterIndex); + /** * Checks whether every fighter in BATTLE.fighterOrder has decided a move * this round. diff --git a/src/dusk/rpg/cutscene/cutscene.c b/src/dusk/rpg/cutscene/cutscene.c index 710d188e..7e5c16bf 100644 --- a/src/dusk/rpg/cutscene/cutscene.c +++ b/src/dusk/rpg/cutscene/cutscene.c @@ -9,6 +9,8 @@ #include "item/json/cutscenejsonpauseflags.h" #include "util/memory.h" #include "assert/assert.h" +#include "asset/asset.h" +#include "asset/loader/assetloader.h" errorret_t cutsceneParseItem( yyjson_val *itemObj, @@ -145,3 +147,51 @@ void cutsceneAppendNext( ) { cutsceneAppendAt(scene, (uint8_t)(currentIndex + 1), items, count); } + +void cutsceneRemoveFront( + cutscene_t *scene, + const uint8_t count +) { + assertNotNull(scene, "Scene cannot be NULL"); + assertTrue(count <= scene->itemCount, "count exceeds itemCount"); + + const size_t tailCount = (size_t)scene->itemCount - count; + // count == 0 has nothing to shift (and would pass dest == src into + // memoryMove, which asserts against that as a no-op call). + if(count > 0 && tailCount > 0) { + memoryMove( + &scene->items[0], + &scene->items[count], + tailCount * sizeof(cutsceneitem_t) + ); + } + scene->itemCount -= count; +} + +errorret_t cutsceneLoadParse( + const char_t *path, + cutscene_t *scene, + cutsceneitem_t *items, + const size_t itemsMax +) { + assertNotNull(path, "Path cannot be NULL"); + assertNotNull(scene, "Scene cannot be NULL"); + assertNotNull(items, "Items cannot be NULL"); + + assetentry_t *entry = assetLock(path, ASSET_LOADER_TYPE_JSON, NULL); + errorret_t result = assetRequireLoaded(entry); + if(errorIsNotOk(result)) { + assetUnlockEntry(entry); + errorChain(result); + } + + // Parse straight out of the entry's own doc while still locked - every + // cutsceneitem_t field that could reference it owns its string data by + // value, so nothing needs to survive past this call and the entry can + // just be unlocked like any other asset once parsing finishes. + errorret_t parseResult = cutsceneParseDoc(entry->data.json, scene, items, itemsMax); + assetUnlockEntry(entry); + errorChain(parseResult); + + errorOk(); +} diff --git a/src/dusk/rpg/cutscene/cutscene.h b/src/dusk/rpg/cutscene/cutscene.h index 2da53b0c..b6263a9d 100644 --- a/src/dusk/rpg/cutscene/cutscene.h +++ b/src/dusk/rpg/cutscene/cutscene.h @@ -127,3 +127,45 @@ void cutsceneAppendNext( const cutsceneitem_t *items, const uint8_t count ); + +/** + * Removes count items from the front of scene->items, shifting the + * remainder down to index 0 and shrinking scene->itemCount to match. Used + * by cutsceneSystemNext/cutsceneGoTo to reclaim already-run items' slots + * instead of leaving them as dead weight forever - see + * CUTSCENE_SYSTEM.currentItem's doc comment, since the currently-running + * item is always scene->items[0] once a cutscene has started. + * + * @param scene Cutscene to remove from. + * @param count Number of items to remove from the front - must be + * <= scene->itemCount. + */ +void cutsceneRemoveFront( + cutscene_t *scene, + const uint8_t count +); + +/** + * Loads a cutscene JSON asset by path and parses it into scene/items (see + * cutsceneParseDoc) - the common "lock the asset, require it loaded, + * parse its 'items' array, unlock it again" sequence shared by every + * place that resolves a cutscene by file path (cutsceneSystemLoad, + * cutsceneCutsceneResolve, cutsceneInsertResolveAndSplice, + * battleStateExecutingInit, cutsceneGoTo's reload). Purely propagates the + * error on failure (like cutsceneParseDoc) - this is core cutscene logic, + * not UI, so it doesn't reach for uiFatalErrorOpen itself; each caller + * decides how to surface the failure (typically errorCatch(errorPrint()) + * + uiFatalErrorOpen(NULL), same as any other asset load failure). + * + * @param path Asset path of the cutscene JSON file to load. + * @param scene Destination scene - itemCount/itemsMax/pause overwritten. + * @param items Destination item array, capacity itemsMax. + * @param itemsMax Capacity of items. + * @return Error code indicating success or failure of the load/parse. + */ +errorret_t cutsceneLoadParse( + const char_t *path, + cutscene_t *scene, + cutsceneitem_t *items, + const size_t itemsMax +); diff --git a/src/dusk/rpg/cutscene/cutscenesystem.c b/src/dusk/rpg/cutscene/cutscenesystem.c index fe9a48c9..415e917d 100644 --- a/src/dusk/rpg/cutscene/cutscenesystem.c +++ b/src/dusk/rpg/cutscene/cutscenesystem.c @@ -10,8 +10,6 @@ #include "util/memory.h" #include "util/string.h" #include "assert/assert.h" -#include "asset/asset.h" -#include "asset/loader/assetloader.h" #include "ui/overlay/uifatalerror.h" cutscenesystem_t CUTSCENE_SYSTEM; @@ -55,7 +53,19 @@ void cutsceneSystemPrepare( CUTSCENE_SYSTEM.textCache[0] = '\0'; CUTSCENE_SYSTEM.currentItem = 0xFF;// Set to 0xFF so Next wraps to 0. CUTSCENE_SYSTEM.onComplete = NULL; - CUTSCENE_SYSTEM.loadedFile[0] = '\0'; + + // Only invalidate the known source file when switching away from + // loadedScene entirely. Starting loadedScene itself means + // cutsceneSystemLoad/cutsceneCutsceneResolve just populated it - for + // cutsceneCutsceneResolve (see cutsceneCutsceneStart), that already + // happened before this call, so clearing loadedFile here would + // immediately undo it. cutsceneGoTo needs loadedFile intact to reload + // a marker behind the current position; cutsceneSystemLoad re-stamps + // it itself right after this call anyway, so it's unaffected either + // way. + if(cutscene != &CUTSCENE_SYSTEM.loadedScene) { + CUTSCENE_SYSTEM.loadedFile[0] = '\0'; + } } void cutsceneSystemStartCutscene(cutscene_t *cutscene) { @@ -118,27 +128,12 @@ void cutsceneSystemInsertCutscene(const cutscene_t *cutscene) { void cutsceneSystemLoad(const char_t *file) { assertNotNull(file, "File cannot be NULL"); - assetentry_t *entry = assetLock(file, ASSET_LOADER_TYPE_JSON, NULL); - errorret_t result = assetRequireLoaded(entry); - if(errorIsNotOk(result)) { - assetUnlockEntry(entry); - errorCatch(errorPrint(result)); - uiFatalErrorOpen(NULL); - return; - } - - // Parse straight out of the entry's own doc while still locked - every - // cutsceneitem_t field that could reference it owns its string data by - // value (see cutscenecutsceneref_t/cutscenemarker_t's doc comments), so - // nothing needs to survive past this call and the entry can just be - // unlocked like any other asset once parsing finishes. - errorret_t parseResult = cutsceneParseDoc( - entry->data.json, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems, + errorret_t result = cutsceneLoadParse( + file, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems, CUTSCENE_LOADED_ITEMS_MAX ); - assetUnlockEntry(entry); - if(errorIsNotOk(parseResult)) { - errorCatch(errorPrint(parseResult)); + if(errorIsNotOk(result)) { + errorCatch(errorPrint(result)); uiFatalErrorOpen(NULL); return; } @@ -256,15 +251,21 @@ void cutsceneSystemSetTextCache(const char_t *text) { void cutsceneSystemNext() { if(CUTSCENE_SYSTEM.scene == NULL) return; - CUTSCENE_SYSTEM.currentItem++; + // Pop the just-finished item off the front rather than advancing an + // index further into the array - see CUTSCENE_SYSTEM.currentItem's doc + // comment. The very first call (currentItem still 0xFF) has nothing to + // pop yet; it just marks the cutscene as started. + if(CUTSCENE_SYSTEM.currentItem == 0xFF) { + CUTSCENE_SYSTEM.currentItem = 0; + } else { + cutsceneRemoveFront(CUTSCENE_SYSTEM.scene, 1); + } // End of the cutscene? Note that a CUTSCENE_ITEM_TYPE_INSERT item's // spliced-in items are physically part of this same array (see // cutsceneSystemInsertCutscene), so there's no separate "parent scene" // to fall back into here - itemCount already accounts for them. - if( - CUTSCENE_SYSTEM.currentItem >= CUTSCENE_SYSTEM.scene->itemCount - ) { + if(CUTSCENE_SYSTEM.scene->itemCount == 0) { // Saved and cleared before firing so a callback that immediately // starts another cutscene (or sets its own onComplete) isn't clobbered // by this function's own cleanup running after it - same reentrancy @@ -298,18 +299,57 @@ void cutsceneGoTo(const char_t *name) { assertNotNull( CUTSCENE_SYSTEM.scene, "cutsceneGoTo called with no cutscene running" ); + // A marker behind the current position can't be found in the live + // queue - already-run items are evicted from its front as the + // cutscene advances (see cutsceneSystemNext/cutsceneRemoveFront), so + // the untouched original has to be re-parsed from source instead. Only + // supported when the running cutscene is the one cutsceneSystemLoad/ + // cutsceneCutsceneResolve populated - a C-authored cutscene_t, or + // content spliced in via INSERT/battleStateExecutingInit, has no + // source to reload from, so a backward jump into those isn't supported + // yet. + assertTrue( + CUTSCENE_SYSTEM.scene == &CUTSCENE_SYSTEM.loadedScene && + CUTSCENE_SYSTEM.loadedFile[0] != '\0', + "cutsceneGoTo requires the running cutscene to have come from " + "cutsceneSystemLoad/cutsceneCutsceneResolve" + ); - for(uint8_t i = 0; i < CUTSCENE_SYSTEM.scene->itemCount; i++) { - const cutsceneitem_t *item = &CUTSCENE_SYSTEM.scene->items[i]; + cutscene_t freshScene = { .items = CUTSCENE_SYSTEM.gotoScratchItems }; + errorret_t result = cutsceneLoadParse( + CUTSCENE_SYSTEM.loadedFile, &freshScene, CUTSCENE_SYSTEM.gotoScratchItems, + CUTSCENE_LOADED_ITEMS_MAX + ); + if(errorIsNotOk(result)) { + errorCatch(errorPrint(result)); + uiFatalErrorOpen(NULL); + return; + } + + for(uint8_t i = 0; i < freshScene.itemCount; i++) { + const cutsceneitem_t *item = &freshScene.items[i]; if( - item->type == CUTSCENE_ITEM_TYPE_MARKER && - stringEquals(item->marker.name, name) - ) { - CUTSCENE_SYSTEM.currentItem = i; - memoryZero(&CUTSCENE_SYSTEM.data, sizeof(CUTSCENE_SYSTEM.data)); - cutsceneItemStart(item, &CUTSCENE_SYSTEM.data); - return; - } + item->type != CUTSCENE_ITEM_TYPE_MARKER || + !stringEquals(item->marker.name, name) + ) continue; + + // Commit: replace the live queue outright with marker-onward from + // the fresh parse. Nothing before the marker (including whatever was + // running) gets Started - only the marker item itself does, same as + // the old in-place jump's semantics. + const uint8_t remaining = freshScene.itemCount - i; + memoryCopy( + CUTSCENE_SYSTEM.loadedItems, &freshScene.items[i], + remaining * sizeof(cutsceneitem_t) + ); + CUTSCENE_SYSTEM.loadedScene.itemCount = remaining; + CUTSCENE_SYSTEM.currentItem = 0; + + memoryZero(&CUTSCENE_SYSTEM.data, sizeof(CUTSCENE_SYSTEM.data)); + cutsceneItemStart( + &CUTSCENE_SYSTEM.loadedItems[0], &CUTSCENE_SYSTEM.data + ); + return; } assertTrue(false, "cutsceneGoTo: no marker found with that name"); @@ -325,5 +365,6 @@ void cutsceneSystemUpdate() { const cutsceneitem_t * cutsceneSystemGetCurrentItem() { if(CUTSCENE_SYSTEM.scene == NULL) return NULL; - return &CUTSCENE_SYSTEM.scene->items[CUTSCENE_SYSTEM.currentItem]; + // Always index 0 - see CUTSCENE_SYSTEM.currentItem's doc comment. + return &CUTSCENE_SYSTEM.scene->items[0]; } diff --git a/src/dusk/rpg/cutscene/cutscenesystem.h b/src/dusk/rpg/cutscene/cutscenesystem.h index eb988a49..3ccdf3fd 100644 --- a/src/dusk/rpg/cutscene/cutscenesystem.h +++ b/src/dusk/rpg/cutscene/cutscenesystem.h @@ -23,6 +23,13 @@ typedef struct entity_s entity_t; typedef struct { cutscene_t *scene; + + // 0xFF if nothing has started running yet, otherwise always 0 - the + // currently-running item is always scene->items[0]; cutsceneSystemNext + // removes it from the front (see cutsceneRemoveFront) rather than + // advancing an index further into the array, so an item's slot is + // reclaimed the moment it finishes instead of sitting there as dead + // weight for the rest of the cutscene's run. uint8_t currentItem; cutscenepause_t pause; entity_t *entityInteract; @@ -47,6 +54,13 @@ typedef struct { // Filename last passed to cutsceneSystemLoad - see cutsceneRestart. char_t loadedFile[ASSET_FILE_NAME_MAX]; + + // Scratch space for cutsceneGoTo's re-parse of loadedFile - a fixed, + // persistent buffer rather than a local one since cutsceneitem_t is + // large (e.g. cutscenemodal_t alone carries a 256-byte message buffer), + // making a CUTSCENE_LOADED_ITEMS_MAX-sized array too big to put on the + // stack on constrained platforms (PSP/GameCube/Wii). + cutsceneitem_t gotoScratchItems[CUTSCENE_LOADED_ITEMS_MAX]; } cutscenesystem_t; extern cutscenesystem_t CUTSCENE_SYSTEM; @@ -211,8 +225,20 @@ void cutsceneSystemNext(); /** * Jumps the running cutscene directly to the marker with the given name - * and starts it immediately. Asserts if no cutscene is running or no - * marker with that name exists. + * and starts it immediately. Re-parses the cutscene fresh from its + * source file (see cutsceneLoadParse) and replaces the live queue with + * marker-onward from that fresh parse - items already run may have been + * evicted from the front of the live queue (see + * cutsceneSystemNext/cutsceneRemoveFront), so a marker behind the + * current position can only be found in the untouched original. Nothing + * before the marker gets Started, only the marker item itself. + * + * Requires the running cutscene to be the one cutsceneSystemLoad/ + * cutsceneCutsceneResolve populated (i.e. CUTSCENE_SYSTEM.scene == + * &CUTSCENE_SYSTEM.loadedScene) - asserts otherwise. A C-authored + * cutscene_t, or content spliced in via INSERT/battleStateExecutingInit, + * has no source file to reload from, so backward jumps into those + * aren't supported yet. * * @param name Marker name to search for. */ diff --git a/src/dusk/rpg/cutscene/item/cutsceneitem.c b/src/dusk/rpg/cutscene/item/cutsceneitem.c index 3f10f891..53b39fa3 100644 --- a/src/dusk/rpg/cutscene/item/cutsceneitem.c +++ b/src/dusk/rpg/cutscene/item/cutsceneitem.c @@ -7,9 +7,6 @@ #include "rpg/cutscene/cutscenesystem.h" #include "rpg/cutscene/item/cutsceneitembase.h" -#include "asset/asset.h" -#include "asset/assetfile.h" -#include "asset/loader/assetloader.h" #include "util/memory.h" #include "util/string.h" #include "ui/overlay/uifatalerror.h" @@ -433,30 +430,21 @@ bool_t cutsceneCutsceneUpdate( } cutscene_t * cutsceneCutsceneResolve(const char_t *name) { - assetentry_t *entry = assetLock(name, ASSET_LOADER_TYPE_JSON, NULL); - errorret_t result = assetRequireLoaded(entry); + errorret_t result = cutsceneLoadParse( + name, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems, + CUTSCENE_LOADED_ITEMS_MAX + ); if(errorIsNotOk(result)) { - assetUnlockEntry(entry); errorCatch(errorPrint(result)); uiFatalErrorOpen(NULL); return NULL; } - // Parse straight out of the entry's own doc while still locked - every - // cutsceneitem_t field that could reference it owns its string data by - // value (see cutscenecutsceneref_t/cutscenemarker_t's doc comments), so - // nothing needs to survive past this call and the entry can just be - // unlocked like any other asset once parsing finishes. - errorret_t parseResult = cutsceneParseDoc( - entry->data.json, &CUTSCENE_SYSTEM.loadedScene, CUTSCENE_SYSTEM.loadedItems, - CUTSCENE_LOADED_ITEMS_MAX - ); - assetUnlockEntry(entry); - if(errorIsNotOk(parseResult)) { - errorCatch(errorPrint(parseResult)); - uiFatalErrorOpen(NULL); - return NULL; - } + // Shares loadedScene/loadedItems with cutsceneSystemLoad, so track the + // source the same way it does - cutsceneGoTo needs this to reload the + // untouched original when jumping to a marker behind the current + // position (see cutsceneGoTo's doc comment). + stringCopy(CUTSCENE_SYSTEM.loadedFile, name, ASSET_FILE_NAME_MAX - 1); return &CUTSCENE_SYSTEM.loadedScene; } @@ -511,29 +499,14 @@ bool_t cutsceneInsertUpdate( } void cutsceneInsertResolveAndSplice(const char_t *name) { - assetentry_t *entry = assetLock(name, ASSET_LOADER_TYPE_JSON, NULL); - errorret_t result = assetRequireLoaded(entry); - if(errorIsNotOk(result)) { - assetUnlockEntry(entry); - errorCatch(errorPrint(result)); - uiFatalErrorOpen(NULL); - return; - } - // 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. Parsed straight out - // 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. + // it only needs to live for the rest of this call. cutsceneitem_t items[CUTSCENE_INSERT_ITEMS_MAX]; cutscene_t scene = { .items = items }; - errorret_t parseResult = cutsceneParseDoc( - entry->data.json, &scene, items, CUTSCENE_INSERT_ITEMS_MAX - ); - assetUnlockEntry(entry); - if(errorIsNotOk(parseResult)) { - errorCatch(errorPrint(parseResult)); + errorret_t result = cutsceneLoadParse(name, &scene, items, CUTSCENE_INSERT_ITEMS_MAX); + if(errorIsNotOk(result)) { + errorCatch(errorPrint(result)); uiFatalErrorOpen(NULL); return; } diff --git a/test/rpg/cutscene/CMakeLists.txt b/test/rpg/cutscene/CMakeLists.txt index d6f0804e..52464928 100644 --- a/test/rpg/cutscene/CMakeLists.txt +++ b/test/rpg/cutscene/CMakeLists.txt @@ -6,6 +6,7 @@ include(dusktest) # Tests +dusktest(test_cutscene.c) dusktest(test_cutscenesystem.c) dusktest(test_cutscenecontrol.c) dusktest(test_cutscenemaparea.c) diff --git a/test/rpg/cutscene/test_cutscene.c b/test/rpg/cutscene/test_cutscene.c new file mode 100644 index 00000000..2f0457cf --- /dev/null +++ b/test/rpg/cutscene/test_cutscene.c @@ -0,0 +1,84 @@ +/** + * Copyright (c) 2026 Dominic Masters + * + * This software is released under the MIT License. + * https://opensource.org/licenses/MIT + */ + +#include "dusktest.h" +#include "rpg/cutscene/cutscene.h" +#include "util/memory.h" +#include "util/string.h" + +// ============================================================ +// cutsceneRemoveFront +// ============================================================ + +static cutsceneitem_t MARKER_ITEM(const char_t *name) { + cutsceneitem_t item; + memoryZero(&item, sizeof(item)); + item.type = CUTSCENE_ITEM_TYPE_MARKER; + stringCopy(item.marker.name, name, CUTSCENE_MARKER_NAME_MAX - 1); + return item; +} + +static void test_cutsceneRemoveFrontShiftsRemainderToIndexZero( + void **state +) { + + cutsceneitem_t items[4] = { + MARKER_ITEM("a"), MARKER_ITEM("b"), MARKER_ITEM("c"), MARKER_ITEM("d") + }; + cutscene_t scene = { .items = items, .itemCount = 4, .itemsMax = 4 }; + + cutsceneRemoveFront(&scene, 2); + + assert_int_equal(scene.itemCount, 2); + assert_string_equal(scene.items[0].marker.name, "c"); + assert_string_equal(scene.items[1].marker.name, "d"); +} + +static void test_cutsceneRemoveFrontAllItemsLeavesEmptyScene(void **state) { + + cutsceneitem_t items[3] = { + MARKER_ITEM("a"), MARKER_ITEM("b"), MARKER_ITEM("c") + }; + cutscene_t scene = { .items = items, .itemCount = 3, .itemsMax = 3 }; + + cutsceneRemoveFront(&scene, 3); + + assert_int_equal(scene.itemCount, 0); +} + +static void test_cutsceneRemoveFrontZeroIsNoop(void **state) { + + cutsceneitem_t items[2] = { MARKER_ITEM("a"), MARKER_ITEM("b") }; + cutscene_t scene = { .items = items, .itemCount = 2, .itemsMax = 2 }; + + cutsceneRemoveFront(&scene, 0); + + assert_int_equal(scene.itemCount, 2); + assert_string_equal(scene.items[0].marker.name, "a"); + assert_string_equal(scene.items[1].marker.name, "b"); +} + +static void test_cutsceneRemoveFrontAssertsWhenCountExceedsItemCount( + void **state +) { + + cutsceneitem_t items[2] = { MARKER_ITEM("a"), MARKER_ITEM("b") }; + cutscene_t scene = { .items = items, .itemCount = 2, .itemsMax = 2 }; + + expect_assert_failure(cutsceneRemoveFront(&scene, 3)); +} + +int main(int argc, char** argv) { + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_cutsceneRemoveFrontShiftsRemainderToIndexZero), + cmocka_unit_test(test_cutsceneRemoveFrontAllItemsLeavesEmptyScene), + cmocka_unit_test(test_cutsceneRemoveFrontZeroIsNoop), + cmocka_unit_test(test_cutsceneRemoveFrontAssertsWhenCountExceedsItemCount), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/test/rpg/cutscene/test_cutscenesystem.c b/test/rpg/cutscene/test_cutscenesystem.c index eaf02f32..c8dbf5ea 100644 --- a/test/rpg/cutscene/test_cutscenesystem.c +++ b/test/rpg/cutscene/test_cutscenesystem.c @@ -9,6 +9,7 @@ #include "rpg/cutscene/cutscenesystem.h" #include "rpg/entity/entity.h" #include "time/time.h" +#include "util/string.h" static uint8_t callbackFired; @@ -198,18 +199,72 @@ static void test_cutsceneSystemDisposeResetsState(void **state) { assert_null(CUTSCENE_SYSTEM.entityInteracted); } +// --- loadedFile survival across cutsceneSystemPrepare ------------------- +// +// cutsceneCutsceneResolve (and cutsceneSystemLoad) stamp +// CUTSCENE_SYSTEM.loadedFile with what they just parsed loadedScene from +// - cutsceneGoTo needs that to survive so it can reload the same source +// later. cutsceneCutsceneResolve's caller stamps it BEFORE calling +// cutsceneSystemStartCutscene(&CUTSCENE_SYSTEM.loadedScene), so +// cutsceneSystemPrepare must not blindly clear loadedFile on every +// start - only when switching away from loadedScene to some other +// cutscene_t entirely (see cutsceneSystemPrepare's doc comment). This +// regression-tests the exact bug hit via initial.jsonc's CUTSCENE item +// into main_menu.jsonc: cutsceneCutsceneResolve stamped loadedFile, then +// the immediately following cutsceneSystemStartCutscene call wiped it +// straight back out, so a later cutsceneGoTo on the still-running +// cutscene had nothing to reload from. +static void test_cutsceneSystemPrepareKeepsLoadedFileWhenReenteringLoadedScene( + void **state +) { + + cutsceneSystemInit(); + + // Simulate cutsceneCutsceneResolve: populate loadedScene/loadedItems + // and stamp loadedFile with its source, before the cutscene has + // actually started running. + CUTSCENE_SYSTEM.loadedItems[0] = + (cutsceneitem_t){ .type = CUTSCENE_ITEM_TYPE_WAIT, .wait = 1.0f }; + CUTSCENE_SYSTEM.loadedScene = (cutscene_t){ + .items = CUTSCENE_SYSTEM.loadedItems, + .itemCount = 1, + .itemsMax = CUTSCENE_LOADED_ITEMS_MAX, + .pause = CUTSCENE_PAUSE_NONE + }; + stringCopy( + CUTSCENE_SYSTEM.loadedFile, "cutscenes/main_menu.jsonc", + ASSET_FILE_NAME_MAX - 1 + ); + + cutsceneSystemStartCutscene(&CUTSCENE_SYSTEM.loadedScene); + + assert_string_equal( + CUTSCENE_SYSTEM.loadedFile, "cutscenes/main_menu.jsonc" + ); + + // Starting a genuinely different cutscene, though, must still + // invalidate it - there's nothing left to reload it from. + cutsceneSystemStartCutscene(&CUTSCENE_TEST_SINGLE_WAIT); + + assert_string_equal(CUTSCENE_SYSTEM.loadedFile, ""); +} + // --- CUTSCENE_INSERT --------------------------------------------------- // // INSERT now splices its target's items directly into the running // scene's own array (see cutsceneSystemInsertCutscene) rather than // swapping CUTSCENE_SYSTEM.scene to a different cutscene_t and pushing a // return frame - CUTSCENE_SYSTEM.scene therefore stays the same outer -// cutscene throughout every test below; itemCount growing is what shows -// the splice actually happened. Since the running scene's own array is -// what gets appended into now, any outer cutscene used with INSERT must -// declare its items array (and .itemsMax) with enough spare capacity for -// whatever ends up spliced into it, including transitively through any -// nested INSERT. +// cutscene throughout every test below. cutsceneSystemNext pops each +// already-run item off the front of the array as it advances (see +// CUTSCENE_SYSTEM.currentItem's doc comment), so itemCount reflects only +// what's left to run at any given point, not a running total of +// everything ever spliced in - the itemCount assertions below are sized +// accordingly. Since the running scene's own array is what gets appended +// into, any outer cutscene used with INSERT must still declare its items +// array (and .itemsMax) with enough spare capacity for whatever ends up +// spliced into it at once, including transitively through any nested +// INSERT. static uint8_t insertOrderLog[8]; static uint8_t insertOrderLogCount; @@ -263,10 +318,12 @@ static void test_cutsceneInsertSplicesItemsInPlaceThenResumes(void **state) { cutsceneSystemUpdate();// the WAIT elapses -> starts order1 cutsceneSystemUpdate();// order1 done -> starts the INSERT, cascading into orderA - // The splice happened in place - CUTSCENE_SYSTEM.scene never changed, - // but it grew by the snippet's 3 items. + // The splice happened in place - CUTSCENE_SYSTEM.scene never changed. + // order0/the WAIT/order1/the INSERT item itself have all since been + // popped off the front, leaving the snippet's 3 items (orderA now + // running, its WAIT, orderB) plus order2. assert_ptr_equal(CUTSCENE_SYSTEM.scene, &CUTSCENE_TEST_INSERT_OUTER); - assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 8); + assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 4); TIME.delta = 2.5f;// elapses the snippet's WAIT(2.0) cutsceneSystemUpdate();// orderA done -> starts snippet's WAIT @@ -323,7 +380,9 @@ static void test_cutsceneInsertAsLastItemEndsCutsceneNaturally( cutsceneSystemUpdate();// order0 done -> starts INSERT, cascades to orderA assert_ptr_equal(CUTSCENE_SYSTEM.scene, &CUTSCENE_TEST_INSERT_AS_LAST_ITEM_OUTER); - assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 3); + // order0 and the INSERT item itself have both been popped off the + // front by now, leaving just orderA (currently running). + assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 1); // orderA is the snippet's only item, and the snippet was spliced in as // the outer's last item -- finishing it should reach the natural end @@ -398,7 +457,9 @@ static void test_cutsceneInsertNestsThroughMultipleLevels(void **state) { // land in OUTER's own array. cutsceneSystemUpdate(); assert_ptr_equal(CUTSCENE_SYSTEM.scene, &CUTSCENE_TEST_INSERT_NESTED_OUTER); - assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 6); + // order0 and both INSERT items themselves have all been popped off the + // front by now, leaving orderA (currently running), orderB, order2. + assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 3); cutsceneSystemUpdate();// orderA done -> continues on to orderB cutsceneSystemUpdate();// orderB done -> continues on to order2 @@ -486,9 +547,10 @@ static void test_cutsceneInsertDoesNotResetPauseOrInteractEntities( ); // The INSERT item's Start already spliced the snippet into place and - // cascaded into it by now. + // cascaded into it by now - and since the INSERT item itself has since + // been popped off the front, only the snippet's one item is left. assert_ptr_equal(CUTSCENE_SYSTEM.scene, &CUTSCENE_TEST_INSERT_PERSIST_OUTER); - assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 2); + assert_int_equal(CUTSCENE_SYSTEM.scene->itemCount, 1); assert_ptr_equal(CUTSCENE_SYSTEM.entityInteract, &ENTITIES[3]); assert_ptr_equal(CUTSCENE_SYSTEM.entityInteracted, &ENTITIES[4]); // The snippet declares ALL, but INSERT doesn't apply it -- the outer @@ -507,6 +569,9 @@ int main(int argc, char** argv) { cmocka_unit_test(test_cutsceneSystemGetAreaId), cmocka_unit_test(test_cutsceneSystemGetTextMiniId), cmocka_unit_test(test_cutsceneSystemDisposeResetsState), + cmocka_unit_test( + test_cutsceneSystemPrepareKeepsLoadedFileWhenReenteringLoadedScene + ), cmocka_unit_test(test_cutsceneInsertSplicesItemsInPlaceThenResumes), cmocka_unit_test(test_cutsceneInsertAsLastItemEndsCutsceneNaturally), cmocka_unit_test(test_cutsceneInsertNestsThroughMultipleLevels),