From b8cbd8ff6a15a7e1b25477a64ee0a118ae021632 Mon Sep 17 00:00:00 2001 From: Dominic Masters Date: Sat, 12 Sep 2026 21:30:08 -0500 Subject: [PATCH] Make party and save-slot init/new-game operate on SAVE.slot directly party.c's functions no longer take a party_t* - they always operate on SAVE.slot.party, since gameplay only ever has one active party. That forced the same change on saveSlotInit/saveSlotNewGame (they called partyInit/partyAddMember internally), so those are now parameterless too, always resetting/seeding SAVE.slot. Callers building a save at a specific index (new-game creation, slot deletion) now set SAVE.slotCurrent first instead of building an independent local struct - safe since the select-save screen is only ever reached before any gameplay starts. saveSlotWriteJSON/saveSlotReadJSON deliberately keep their explicit saveslot_t* parameter - they're genuine serialization primitives used by the save-device layer and tests against arbitrary structs, unrelated to the single-current-party assumption. saveSlotReadJSON's internal reset is now a plain memset instead of delegating to the now-global-only saveSlotInit(). Updates test_save/test_savedevice/test_savedevicelinux/test_saveslot for the new signatures. Note: test_savedevice/test_savedevicelinux/test_save have pre-existing failures in this sandbox unrelated to this change - savetestfixture.c swaps $HOME, but saveDeviceLinuxGetDirectory actually derives the save path from ASSET.baseDirectory (the executable's own location), so the fixture's sandboxing never actually applies. test_ saveslot.c (which doesn't depend on that fixture) passes 15/15. Co-Authored-By: Claude Sonnet 5 --- src/dusk/rpg/battle/party.c | 25 +++--- src/dusk/rpg/battle/party.h | 38 ++++---- .../item/battle/cutscenestartbattle.c | 4 +- src/dusk/rpg/rpg.c | 2 +- src/dusk/save/save.c | 8 +- src/dusk/save/savedevice.c | 9 +- src/dusk/save/slot/saveslot.c | 35 +++++--- src/dusk/save/slot/saveslot.h | 27 +++--- src/dusk/ui/dialog/save/uiselectsave.c | 24 +++-- src/dusk/ui/screen/mainmenu/uimainmenu.c | 2 +- test/save/test_save.c | 30 ++++--- test/save/test_savedevice.c | 17 ++-- test/save/test_savedevicelinux.c | 29 +++--- test/save/test_saveslot.c | 88 +++++++++---------- 14 files changed, 180 insertions(+), 158 deletions(-) diff --git a/src/dusk/rpg/battle/party.c b/src/dusk/rpg/battle/party.c index f1ed9493..c5971f73 100644 --- a/src/dusk/rpg/battle/party.c +++ b/src/dusk/rpg/battle/party.c @@ -8,9 +8,10 @@ #include "party.h" #include "assert/assert.h" #include "util/memory.h" +#include "save/save.h" -void partyInit(party_t *party) { - assertNotNull(party, "Party cannot be null"); +void partyInit(void) { + party_t *party = &SAVE.slot.party; memoryZero(party, sizeof(party_t)); for(uint8_t i = 0; i < PARTY_MEMBER_COUNT_MAX; i++) { @@ -21,8 +22,8 @@ void partyInit(party_t *party) { } } -uint8_t partyGetAvailableMember(const party_t *party) { - assertNotNull(party, "Party cannot be null"); +uint8_t partyGetAvailableMember(void) { + const party_t *party = &SAVE.slot.party; for(uint8_t i = 0; i < PARTY_MEMBER_COUNT_MAX; i++) { if(party->members[i].status == BATTLE_FIGHTER_STATUS_NULL) return i; @@ -32,14 +33,13 @@ uint8_t partyGetAvailableMember(const party_t *party) { } battlefighter_t *partyAddMember( - party_t *party, const battlefighterstats_t stats, const uint16_t healthMax, const uint16_t mpMax ) { - assertNotNull(party, "Party cannot be null"); + party_t *party = &SAVE.slot.party; - const uint8_t index = partyGetAvailableMember(party); + const uint8_t index = partyGetAvailableMember(); if(index == 0xFF) return NULL; battlefighter_t *member = &party->members[index]; @@ -57,24 +57,21 @@ battlefighter_t *partyAddMember( return member; } -battlefighter_t *partyGetOrderMember(party_t *party, const uint8_t slot) { - assertNotNull(party, "Party cannot be null"); +battlefighter_t *partyGetOrderMember(const uint8_t slot) { assertTrue(slot < PARTY_ACTIVE_SIZE_MAX, "Invalid party order slot"); + party_t *party = &SAVE.slot.party; const uint8_t index = party->order[slot]; if(index == PARTY_ORDER_EMPTY) return NULL; return &party->members[index]; } -void partySetOrder( - party_t *party, const uint8_t slot, const uint8_t memberIndex -) { - assertNotNull(party, "Party cannot be null"); +void partySetOrder(const uint8_t slot, const uint8_t memberIndex) { assertTrue(slot < PARTY_ACTIVE_SIZE_MAX, "Invalid party order slot"); assertTrue( memberIndex == PARTY_ORDER_EMPTY || memberIndex < PARTY_MEMBER_COUNT_MAX, "Invalid party member index" ); - party->order[slot] = memberIndex; + SAVE.slot.party.order[slot] = memberIndex; } diff --git a/src/dusk/rpg/battle/party.h b/src/dusk/rpg/battle/party.h index 4485ae6f..3e8f4e8f 100644 --- a/src/dusk/rpg/battle/party.h +++ b/src/dusk/rpg/battle/party.h @@ -23,26 +23,25 @@ typedef struct { } party_t; /** - * Initializes the party with an empty roster and order. - * - * @param party The party to initialize. + * Initializes the current save slot's party (SAVE.slot.party) with an + * empty roster and order. */ -void partyInit(party_t *party); +void partyInit(void); /** - * Gets an available (unused) party member slot index. + * Gets an available (unused) member slot index in the current save slot's + * party. * - * @param party The party to query. * @return The index of an available slot, or 0xFF if the party is full. */ -uint8_t partyGetAvailableMember(const party_t *party); +uint8_t partyGetAvailableMember(void); /** - * Adds a member to the party roster in the next available slot. Party - * members are always allies controlled by the player. If there is a - * free active order slot, the new member is placed into it. + * Adds a member to the current save slot's party roster in the next + * available slot. Party members are always allies controlled by the + * player. If there is a free active order slot, the new member is placed + * into it. * - * @param party The party to add the member to. * @param stats The member's base combat stats. * @param healthMax The member's maximum health. * @param mpMax The member's maximum mp. @@ -50,31 +49,28 @@ uint8_t partyGetAvailableMember(const party_t *party); * already full. */ battlefighter_t *partyAddMember( - party_t *party, const battlefighterstats_t stats, const uint16_t healthMax, const uint16_t mpMax ); /** - * Gets the roster member currently occupying an active order slot. + * Gets the roster member currently occupying an active order slot in the + * current save slot's party. * - * @param party The party to query. * @param slot The active order slot to query. * @return Pointer to the member in that slot, or NULL if the slot is * empty. */ -battlefighter_t *partyGetOrderMember(party_t *party, const uint8_t slot); +battlefighter_t *partyGetOrderMember(const uint8_t slot); /** - * Assigns a roster member to an active order slot, replacing whatever - * was there. Use PARTY_ORDER_EMPTY to clear a slot. + * Assigns a roster member to an active order slot in the current save + * slot's party, replacing whatever was there. Use PARTY_ORDER_EMPTY to + * clear a slot. * - * @param party The party to modify. * @param slot The active order slot to assign. * @param memberIndex The roster member index to place there, or * PARTY_ORDER_EMPTY to clear the slot. */ -void partySetOrder( - party_t *party, const uint8_t slot, const uint8_t memberIndex -); +void partySetOrder(const uint8_t slot, const uint8_t memberIndex); diff --git a/src/dusk/rpg/cutscene/item/battle/cutscenestartbattle.c b/src/dusk/rpg/cutscene/item/battle/cutscenestartbattle.c index ad6e2ee2..338fd62d 100644 --- a/src/dusk/rpg/cutscene/item/battle/cutscenestartbattle.c +++ b/src/dusk/rpg/cutscene/item/battle/cutscenestartbattle.c @@ -22,7 +22,7 @@ void cutsceneStartBattleStart( for(uint8_t i = 0; i < config->playerCount; i++) { battlefighter_t *member = - partyGetOrderMember(&SAVE.slot.party, config->players[i]); + partyGetOrderMember(config->players[i]); if(member == NULL) continue; battlefighter_t *fighter = battleAddFighter( @@ -62,7 +62,7 @@ bool_t cutsceneStartBattleUpdate( uint8_t allySlot = 0; for(uint8_t i = 0; i < config->playerCount; i++) { battlefighter_t *member = - partyGetOrderMember(&SAVE.slot.party, config->players[i]); + partyGetOrderMember(config->players[i]); if(member == NULL) continue; battlefighter_t *fighter = &BATTLE.fighters[allySlot++]; diff --git a/src/dusk/rpg/rpg.c b/src/dusk/rpg/rpg.c index 17220401..9b0791d5 100644 --- a/src/dusk/rpg/rpg.c +++ b/src/dusk/rpg/rpg.c @@ -28,7 +28,7 @@ errorret_t rpgInit(void) { memoryZero(MAP_AREAS, sizeof(MAP_AREAS)); backpackInit(); - partyInit(&SAVE.slot.party); + partyInit(); cutsceneSystemInit(); errorChain(mapInit()); rpgCameraInit(); diff --git a/src/dusk/save/save.c b/src/dusk/save/save.c index 5691fa5e..afdb958d 100644 --- a/src/dusk/save/save.c +++ b/src/dusk/save/save.c @@ -21,7 +21,7 @@ errorret_t saveInit() { // Initialize the slots and settings saveSettingsInit(&SAVE.settings); - saveSlotInit(&SAVE.slot); + saveSlotInit(); // Update caches to match default data. for(uint8_t i = 0; i < SAVE_SLOT_COUNT; i++) { @@ -226,7 +226,7 @@ errorret_t saveLoadSlot() { if(SAVE.deviceCurrent == 0xFF) { // No save device available, so we assume an empty slot. - saveSlotInit(&SAVE.slot); + saveSlotInit(); SAVE.caches[SAVE.slotCurrent] = SAVE.slot.cachedData; errorOk(); } @@ -245,7 +245,7 @@ errorret_t saveLoadSlot() { "Save slot %u could not be loaded, treating as empty.\n", (uint32_t)SAVE.slotCurrent ); - saveSlotInit(&SAVE.slot); + saveSlotInit(); SAVE.slot.cachedData.corrupt = true; } @@ -260,7 +260,7 @@ errorret_t saveLoadSlot() { errorret_t saveLoadAllSlots() { for(uint8_t i = 0; i < SAVE_SLOT_COUNT; i++) { SAVE.slotCurrent = i; - saveSlotInit(&SAVE.slot); + saveSlotInit(); errorChain(saveLoadSlot());// Load slot updates the cache. } diff --git a/src/dusk/save/savedevice.c b/src/dusk/save/savedevice.c index 91f4131e..111fdb08 100644 --- a/src/dusk/save/savedevice.c +++ b/src/dusk/save/savedevice.c @@ -340,8 +340,15 @@ errorret_t saveDeviceRawStoreItem( slotItems[i].ptr = oldSpans.slots[i].ptr; slotItems[i].len = oldSpans.slots[i].len; } else { + // A raw reset, not saveSlotInit() - this fills in a slot index the + // current write isn't touching, and saveSlotInit() only ever + // operates on SAVE.slot now. A blank ("" name) slot never actually + // needs a properly-initialized party (see saveSlotReadJSON's early + // return for an unused slot), so this is equivalent for this + // filler-only purpose. saveslot_t defaultSlot; - saveSlotInit(&defaultSlot); + memorySet(&defaultSlot, 0, sizeof(defaultSlot)); + defaultSlot.version = 1; char_t *json = NULL; size_t len = 0; errorret_t result = saveDeviceRawSerializeSlot(&defaultSlot, &json, &len); diff --git a/src/dusk/save/slot/saveslot.c b/src/dusk/save/slot/saveslot.c index 5d4f5696..7c0d1b3f 100644 --- a/src/dusk/save/slot/saveslot.c +++ b/src/dusk/save/slot/saveslot.c @@ -7,35 +7,32 @@ #include "saveslot.h" #include "save/slot/saveslotcurrent.h" +#include "save/save.h" #include "assert/assert.h" #include "util/memory.h" #include "util/string.h" -void saveSlotInit(saveslot_t *slot) { - assertNotNull(slot, "Slot cannot be null"); +void saveSlotInit(void) { + memorySet(&SAVE.slot, 0, sizeof(saveslot_t)); - memorySet(slot, 0, sizeof(saveslot_t)); + SAVE.slot.version = 1; - slot->version = 1; - - partyInit(&slot->party); + partyInit(); } -void saveSlotNewGame(saveslot_t *slot) { - assertNotNull(slot, "Slot cannot be null"); - - saveSlotInit(slot); +void saveSlotNewGame(void) { + saveSlotInit(); stringCopy( - slot->cachedData.mapName, SAVE_SLOT_MAP_NAME_DEFAULT, - sizeof(slot->cachedData.mapName) + SAVE.slot.cachedData.mapName, SAVE_SLOT_MAP_NAME_DEFAULT, + sizeof(SAVE.slot.cachedData.mapName) ); // TEMPORARY: placeholder starting party stats - replace once there's a // real starting-party/character-creation flow. const battlefighterstats_t startingStats = { .attack = 10, .defense = 5, .magic = 0, .speed = 10, .luck = 0 }; - partyAddMember(&slot->party, startingStats, 30, 10); + partyAddMember(startingStats, 30, 10); } bool_t saveSlotInUse(saveslotcache_t *slot) { @@ -58,7 +55,17 @@ errorret_t saveSlotWriteJSON( } errorret_t saveSlotReadJSON(saveslot_t *slot, yyjson_val *object) { - saveSlotInit(slot); + assertNotNull(slot, "Slot cannot be null"); + assertNotNull(object, "Object cannot be null"); + + // A plain reset, not saveSlotInit() - this may be reading into a slot + // other than SAVE.slot (e.g. a save-device round trip in a test), and + // saveSlotInit() only ever operates on SAVE.slot now. saveSlotCurrent + // ReadJSON() fully overwrites every field below anyway (including + // party.members/order), so this only needs to reset the bookkeeping + // fields (version/dataType) that aren't part of the JSON wire format. + memorySet(slot, 0, sizeof(saveslot_t)); + slot->version = 1; errorChain(saveSlotCurrentReadJSON(slot, object)); errorOk(); diff --git a/src/dusk/save/slot/saveslot.h b/src/dusk/save/slot/saveslot.h index 9a8fcde3..4bbeaef1 100644 --- a/src/dusk/save/slot/saveslot.h +++ b/src/dusk/save/slot/saveslot.h @@ -35,25 +35,22 @@ typedef struct saveslot_s { } saveslot_t; /** - * Inits the save slot with the default state, this is functionally "new game" - * but will not set the player name, as that is what we use to determine if a - * save slot is "in use" or not. - * - * @param slot The save slot to init. + * Inits the current save slot (SAVE.slot) with the default state, this is + * functionally "new game" but will not set the player name, as that is + * what we use to determine if a save slot is "in use" or not. */ -void saveSlotInit(saveslot_t *slot); +void saveSlotInit(void); /** - * Sets up the save slot for a brand new game: calls saveSlotInit, then - * seeds it with a starting party. Use this (not saveSlotInit) whenever a - * new save is actually being created for a player - saveSlotInit alone - * leaves the party empty, which every other caller (resetting a slot - * before a load attempt, filling in an untouched slot during a raw - * device write, wiping a deleted slot) wants. - * - * @param slot The save slot to set up. + * Sets up the current save slot (SAVE.slot) for a brand new game: calls + * saveSlotInit, then seeds it with a starting party. Use this (not + * saveSlotInit) whenever a new save is actually being created for a + * player - saveSlotInit alone leaves the party empty, which every other + * caller (resetting the slot before a load attempt, wiping a deleted + * slot) wants. Callers creating a new save at a specific index should set + * SAVE.slotCurrent first. */ -void saveSlotNewGame(saveslot_t *slot); +void saveSlotNewGame(void); /** * Checks if the save slot is in use, this is determined by checking if the diff --git a/src/dusk/ui/dialog/save/uiselectsave.c b/src/dusk/ui/dialog/save/uiselectsave.c index 80917858..d012c689 100644 --- a/src/dusk/ui/dialog/save/uiselectsave.c +++ b/src/dusk/ui/dialog/save/uiselectsave.c @@ -103,18 +103,21 @@ void uiSelectSaveDeleteConfirmed(const bool_t result, void *user) { assertTrue(SAVE.deviceCurrent != 0xFF, "No current device"); const uint8_t index = UI_SELECT_SAVE.pendingDeleteIndex; - saveslot_t slot; - saveSlotInit(&slot); + // uiSelectSaveOpen is only ever reached before any gameplay starts (see + // uimainmenu.c), so it's safe to reuse SAVE.slot as scratch for whichever + // index is being written here - nothing else is relying on it. + SAVE.slotCurrent = index; + saveSlotInit(); errorret_t writeResult = saveDeviceSlotWrite( - &SAVE.devices[SAVE.deviceCurrent], &slot, index + &SAVE.devices[SAVE.deviceCurrent], &SAVE.slot, index ); if(errorIsNotOk(writeResult)) { errorCatch(errorPrint(writeResult)); return; } - SAVE.caches[index] = slot.cachedData; + SAVE.caches[index] = SAVE.slot.cachedData; uiSelectSaveSwitchType(UI_SELECT_SAVE_TYPE_LOAD); uiSelectSaveRefreshSlot(index); @@ -137,16 +140,19 @@ void uiSelectSaveNameEntered( const uint8_t index = UI_SELECT_SAVE.pendingNameIndex; - saveslot_t slot; - saveSlotNewGame(&slot); - stringCopy(slot.cachedData.name, text, SAVE_SLOT_NAME_LENGTH); + // uiSelectSaveOpen is only ever reached before any gameplay starts (see + // uimainmenu.c), so it's safe to reuse SAVE.slot as scratch for whichever + // index is being written here - nothing else is relying on it. + SAVE.slotCurrent = index; + saveSlotNewGame(); + stringCopy(SAVE.slot.cachedData.name, text, SAVE_SLOT_NAME_LENGTH); // No save device available - proceed with an in-memory-only slot rather // than writing to a device that doesn't exist (see saveLoadSlot()'s own // comment on the same "continue without a save device" flow). if(SAVE.deviceCurrent != 0xFF) { errorret_t writeResult = saveDeviceSlotWrite( - &SAVE.devices[SAVE.deviceCurrent], &slot, index + &SAVE.devices[SAVE.deviceCurrent], &SAVE.slot, index ); if(errorIsNotOk(writeResult)) { errorCatch(errorPrint(writeResult)); @@ -155,7 +161,7 @@ void uiSelectSaveNameEntered( } } - SAVE.caches[index] = slot.cachedData; + SAVE.caches[index] = SAVE.slot.cachedData; uiSelectSaveRefreshSlot(index); UI_SELECT_SAVE.result = index; diff --git a/src/dusk/ui/screen/mainmenu/uimainmenu.c b/src/dusk/ui/screen/mainmenu/uimainmenu.c index 7cfe5ccc..4340388b 100644 --- a/src/dusk/ui/screen/mainmenu/uimainmenu.c +++ b/src/dusk/ui/screen/mainmenu/uimainmenu.c @@ -50,7 +50,7 @@ void uiMainMenuSelectSaveResult(const uint8_t slotIndex, void *user) { } SAVE.slotCurrent = slotIndex; - saveSlotInit(&SAVE.slot); + saveSlotInit(); errorret_t result = saveLoadSlot(); if(errorIsNotOk(result)) { errorCatch(errorPrint(result)); diff --git a/test/save/test_save.c b/test/save/test_save.c index 07396e51..4628cd5c 100644 --- a/test/save/test_save.c +++ b/test/save/test_save.c @@ -298,7 +298,7 @@ static void test_saveSaveSlot_invalidSlotIndexAsserts(void **state) { static void test_saveSaveSlot_success(void **state) { makeDeviceAvailable(); SAVE.slotCurrent = 1; - saveSlotInit(&SAVE.slot); + saveSlotInit(); stringCopy( SAVE.slot.cachedData.name, "Hero", sizeof(SAVE.slot.cachedData.name) ); @@ -311,8 +311,9 @@ static void test_saveSaveSlot_success(void **state) { assert_true(stringEquals(SAVE.caches[1].name, "Hero")); assert_int_equal(SAVE.caches[1].playerLevel, 12); + // No pre-init needed - saveSlotReadJSON() (via saveDeviceSlotRead) fully + // resets/overwrites its destination struct regardless of prior content. saveslot_t onDisk; - saveSlotInit(&onDisk); errorret_t readRet = saveDeviceSlotRead(&SAVE.devices[0], &onDisk, 1); assert_true(errorIsOk(readRet)); assert_true(stringEquals(onDisk.cachedData.name, "Hero")); @@ -331,15 +332,20 @@ static void test_saveLoadSlot_invalidSlotIndexAsserts(void **state) { static void test_saveLoadSlot_success(void **state) { makeDeviceAvailable(); - saveslot_t onDisk; - saveSlotInit(&onDisk); - stringCopy(onDisk.cachedData.name, "Zelda", sizeof(onDisk.cachedData.name)); - onDisk.cachedData.playerLevel = 30; - errorret_t writeRet = saveDeviceSlotWrite(&SAVE.devices[0], &onDisk, 2); + // Write slot 2's on-disk file via SAVE.slot itself (saveSlotInit() only + // ever operates on SAVE.slot now), then reset it again below - the + // write and the load-under-test don't overlap in time. + SAVE.slotCurrent = 2; + saveSlotInit(); + stringCopy( + SAVE.slot.cachedData.name, "Zelda", sizeof(SAVE.slot.cachedData.name) + ); + SAVE.slot.cachedData.playerLevel = 30; + errorret_t writeRet = saveDeviceSlotWrite(&SAVE.devices[0], &SAVE.slot, 2); assert_true(errorIsOk(writeRet)); SAVE.slotCurrent = 2; - saveSlotInit(&SAVE.slot); + saveSlotInit(); SAVE.slotDirty = true; errorret_t ret = saveLoadSlot(); @@ -365,7 +371,7 @@ static void test_saveLoadSlot_deviceReadFails_cacheStaysStale(void **state) { fclose(file); SAVE.slotCurrent = 0; - saveSlotInit(&SAVE.slot); + saveSlotInit(); SAVE.caches[0].playerLevel = 77;// sentinel errorret_t ret = saveLoadSlot(); @@ -385,7 +391,7 @@ static void test_saveLoadAllSlots_loadsAllAndEndsAtLastIndex(void **state) { for(uint8_t i = 0; i < SAVE_SLOT_COUNT; i++) { SAVE.slotCurrent = i; - saveSlotInit(&SAVE.slot); + saveSlotInit(); SAVE.slot.cachedData.playerLevel = (int32_t)(i + 1); errorret_t saveRet = saveSaveSlot(); assert_true(errorIsOk(saveRet)); @@ -404,7 +410,7 @@ static void test_saveLoadAllSlots_missingFileLeavesInitDefaults(void **state) { makeDeviceAvailable(); SAVE.slotCurrent = 0; - saveSlotInit(&SAVE.slot); + saveSlotInit(); SAVE.slot.cachedData.playerLevel = 5; errorret_t saveRet = saveSaveSlot(); assert_true(errorIsOk(saveRet)); @@ -427,7 +433,7 @@ static void test_saveLoadAllSlots_middleSlotCorrupt_stopsAtFailingIndex( makeDeviceAvailable(); SAVE.slotCurrent = 0; - saveSlotInit(&SAVE.slot); + saveSlotInit(); errorret_t saveRet = saveSaveSlot(); assert_true(errorIsOk(saveRet)); diff --git a/test/save/test_savedevice.c b/test/save/test_savedevice.c index 909aa06c..02bc800e 100644 --- a/test/save/test_savedevice.c +++ b/test/save/test_savedevice.c @@ -10,6 +10,7 @@ #include "save/savedevice.h" #include "save/slot/saveslot.h" #include "save/settings/savesettings.h" +#include "save/save.h" #include "util/memory.h" #include "util/string.h" @@ -155,8 +156,9 @@ static void test_saveDeviceCheckAvailability_resolvesSynchronouslyOnLinux( static void test_saveDeviceSlotWrite_nullAsserts(void **state) { savedevice_t device; + // Content is never touched - both calls assert on a NULL argument + // before either device or slot would actually be read. saveslot_t slot; - saveSlotInit(&slot); expect_assert_failure(saveDeviceSlotWrite(NULL, &slot, 0)); expect_assert_failure(saveDeviceSlotWrite(&device, NULL, 0)); @@ -165,7 +167,6 @@ static void test_saveDeviceSlotWrite_nullAsserts(void **state) { static void test_saveDeviceSlotRead_nullAsserts(void **state) { savedevice_t device; saveslot_t slot; - saveSlotInit(&slot); expect_assert_failure(saveDeviceSlotRead(NULL, &slot, 0)); expect_assert_failure(saveDeviceSlotRead(&device, NULL, 0)); @@ -198,15 +199,17 @@ static void test_saveDeviceSlot_dispatchesToPlatform(void **state) { saveDeviceCheckAvailability(&device, updateCallback, NULL); saveDeviceUpdate(&device); - saveslot_t written; - saveSlotInit(&written); - written.cachedData.playerLevel = 21; + // saveSlotInit() only ever operates on SAVE.slot now, so that's the + // write source here. + saveSlotInit(); + SAVE.slot.cachedData.playerLevel = 21; - errorret_t writeRet = saveDeviceSlotWrite(&device, &written, 0); + errorret_t writeRet = saveDeviceSlotWrite(&device, &SAVE.slot, 0); assert_true(errorIsOk(writeRet)); + // No pre-init needed - saveSlotReadJSON() (via saveDeviceSlotRead) fully + // resets/overwrites its destination struct regardless of prior content. saveslot_t read; - saveSlotInit(&read); errorret_t readRet = saveDeviceSlotRead(&device, &read, 0); assert_true(errorIsOk(readRet)); diff --git a/test/save/test_savedevicelinux.c b/test/save/test_savedevicelinux.c index 10df93f5..f7c3f3f0 100644 --- a/test/save/test_savedevicelinux.c +++ b/test/save/test_savedevicelinux.c @@ -11,6 +11,7 @@ #include "save/savedevicelinux.h" #include "save/slot/saveslot.h" #include "save/settings/savesettings.h" +#include "save/save.h" #include "util/memory.h" #include "util/string.h" #include @@ -180,8 +181,9 @@ static void test_checkAvailability_mkdirpFails(void **state) { static void test_slotWrite_nullAsserts(void **state) { savedevice_t device; + // Content is never touched - both calls assert on a NULL argument + // before either device or slot would actually be read. saveslot_t slot; - saveSlotInit(&slot); expect_assert_failure(saveDeviceLinuxSlotWrite(NULL, &slot, 0)); expect_assert_failure(saveDeviceLinuxSlotWrite(&device, NULL, 0)); @@ -190,7 +192,6 @@ static void test_slotWrite_nullAsserts(void **state) { static void test_slotRead_nullAsserts(void **state) { savedevice_t device; saveslot_t slot; - saveSlotInit(&slot); expect_assert_failure(saveDeviceLinuxSlotRead(NULL, &slot, 0)); expect_assert_failure(saveDeviceLinuxSlotRead(&device, NULL, 0)); @@ -203,17 +204,21 @@ static void test_slotWriteRead_roundTrip(void **state) { // saveDeviceLinuxCheckAvailability does this via mkdirp. checkAvailabilityAndDrain(&device); - saveslot_t written; - saveSlotInit(&written); - stringCopy(written.cachedData.name, "Hero", sizeof(written.cachedData.name)); - written.cachedData.playerLevel = 9; + // saveSlotInit() only ever operates on SAVE.slot now, so that's the + // write source here. + saveSlotInit(); + stringCopy( + SAVE.slot.cachedData.name, "Hero", sizeof(SAVE.slot.cachedData.name) + ); + SAVE.slot.cachedData.playerLevel = 9; - errorret_t writeRet = saveDeviceLinuxSlotWrite(&device, &written, 1); + errorret_t writeRet = saveDeviceLinuxSlotWrite(&device, &SAVE.slot, 1); assert_true(errorIsOk(writeRet)); + // No pre-init needed - saveSlotReadJSON() (via saveDeviceLinuxSlotRead) + // fully resets/overwrites its destination struct regardless of prior + // content. saveslot_t read; - saveSlotInit(&read); - errorret_t readRet = saveDeviceLinuxSlotRead(&device, &read, 1); assert_true(errorIsOk(readRet)); @@ -225,8 +230,11 @@ static void test_slotRead_noFileYet_leavesSlotUntouched(void **state) { savedevice_t device; memoryZero(&device, sizeof(device)); + // Deliberately not saveSlotInit()'d - a missing slot file means + // saveDeviceLinuxSlotRead never even calls saveSlotReadJSON, so slot + // must come in exactly as the caller left it and go out unchanged. saveslot_t slot; - saveSlotInit(&slot); + memoryZero(&slot, sizeof(slot)); stringCopy(slot.cachedData.name, "sentinel", sizeof(slot.cachedData.name)); slot.cachedData.playerLevel = 555; @@ -260,7 +268,6 @@ static void test_slotRead_corruptFile_errors(void **state) { fclose(file); saveslot_t slot; - saveSlotInit(&slot); errorret_t ret = saveDeviceLinuxSlotRead(&device, &slot, 0); assert_true(errorIsNotOk(ret)); diff --git a/test/save/test_saveslot.c b/test/save/test_saveslot.c index bcdb10e9..8309cae3 100644 --- a/test/save/test_saveslot.c +++ b/test/save/test_saveslot.c @@ -7,6 +7,7 @@ #include "dusktest.h" #include "save/slot/saveslot.h" +#include "save/save.h" #include "save/savejson.h" #include "util/memory.h" #include "util/string.h" @@ -45,24 +46,19 @@ static errorret_t slotFromJSON(saveslot_t *slot, const char_t *json) { } // ============================================================ -// saveSlotInit +// saveSlotInit - only ever operates on SAVE.slot now. // ============================================================ static void test_saveSlotInit_defaults(void **state) { - saveslot_t slot; - memorySet(&slot, 0xFF, sizeof(slot)); + memorySet(&SAVE.slot, 0xFF, sizeof(SAVE.slot)); - saveSlotInit(&slot); + saveSlotInit(); - assert_int_equal(slot.version, 1); - assert_int_equal(slot.dataType, 0); - assert_int_equal(slot.cachedData.name[0], '\0'); - assert_true(slot.cachedData.time.time == 0.0); - assert_int_equal(slot.cachedData.playerLevel, 0); -} - -static void test_saveSlotInit_nullAsserts(void **state) { - expect_assert_failure(saveSlotInit(NULL)); + assert_int_equal(SAVE.slot.version, 1); + assert_int_equal(SAVE.slot.dataType, 0); + assert_int_equal(SAVE.slot.cachedData.name[0], '\0'); + assert_true(SAVE.slot.cachedData.time.time == 0.0); + assert_int_equal(SAVE.slot.cachedData.playerLevel, 0); } // ============================================================ @@ -70,13 +66,14 @@ static void test_saveSlotInit_nullAsserts(void **state) { // ============================================================ static void test_saveSlotInUse(void **state) { - saveslot_t slot; - saveSlotInit(&slot); + saveSlotInit(); - assert_false(saveSlotInUse(&slot.cachedData)); + assert_false(saveSlotInUse(&SAVE.slot.cachedData)); - stringCopy(slot.cachedData.name, "Hero", sizeof(slot.cachedData.name)); - assert_true(saveSlotInUse(&slot.cachedData)); + stringCopy( + SAVE.slot.cachedData.name, "Hero", sizeof(SAVE.slot.cachedData.name) + ); + assert_true(saveSlotInUse(&SAVE.slot.cachedData)); } static void test_saveSlotInUse_nullAsserts(void **state) { @@ -84,18 +81,17 @@ static void test_saveSlotInUse_nullAsserts(void **state) { } static void test_saveSlotHasSaved(void **state) { - saveslot_t slot; - saveSlotInit(&slot); + saveSlotInit(); - assert_false(saveSlotHasSaved(&slot.cachedData)); + assert_false(saveSlotHasSaved(&SAVE.slot.cachedData)); // Writing to JSON stamps the current time as a side effect, so even a // pure serialize (no device write) flips "has ever saved" to true. char_t *json; size_t len; - errorret_t ret = slotToJSON(&slot, &json, &len); + errorret_t ret = slotToJSON(&SAVE.slot, &json, &len); assert_true(errorIsOk(ret)); - assert_true(saveSlotHasSaved(&slot.cachedData)); + assert_true(saveSlotHasSaved(&SAVE.slot.cachedData)); free(json); } @@ -109,8 +105,9 @@ static void test_saveSlotHasSaved_nullAsserts(void **state) { // ============================================================ static void test_saveSlotWriteJSON_nullAsserts(void **state) { + // Content is never touched - both calls assert on a NULL doc/object + // argument before slot itself would actually be read. saveslot_t slot; - saveSlotInit(&slot); writeInit(); expect_assert_failure(saveSlotWriteJSON(NULL, doc, object)); @@ -126,7 +123,6 @@ static void test_saveSlotWriteJSON_nullAsserts(void **state) { static void test_saveSlotReadJSON_nullAsserts(void **state) { saveslot_t slot; - saveSlotInit(&slot); yyjson_doc *readDoc = yyjson_read("{}", 2, 0); yyjson_val *readObject = yyjson_doc_get_root(readDoc); @@ -138,45 +134,52 @@ static void test_saveSlotReadJSON_nullAsserts(void **state) { } static void test_saveSlotReadJSON_roundTrip(void **state) { - saveslot_t written; - saveSlotInit(&written); - written.version = 7;// deliberately non-default, not part of the JSON schema - stringCopy(written.cachedData.name, "Hero", sizeof(written.cachedData.name)); - written.cachedData.playerLevel = 42; + // saveSlotInit() only ever operates on SAVE.slot now, so that's the + // write source here. + saveSlotInit(); + SAVE.slot.version = 7;// deliberately non-default, not part of the JSON schema + stringCopy( + SAVE.slot.cachedData.name, "Hero", sizeof(SAVE.slot.cachedData.name) + ); + SAVE.slot.cachedData.playerLevel = 42; // An in-use slot (non-empty name) requires a non-empty map name too - // see saveSlotNewGame/requireStrMin("mapName", 1) in saveSlotVer1ReadJSON. stringCopy( - written.cachedData.mapName, "overworld", - sizeof(written.cachedData.mapName) + SAVE.slot.cachedData.mapName, "overworld", + sizeof(SAVE.slot.cachedData.mapName) ); char_t *json; size_t len; - errorret_t writeRet = slotToJSON(&written, &json, &len); + errorret_t writeRet = slotToJSON(&SAVE.slot, &json, &len); assert_true(errorIsOk(writeRet)); - saveslot_t read; - saveSlotInit(&read); + const dusktimeepoch_t writtenTime = SAVE.slot.cachedData.time; + // No pre-init needed - saveSlotReadJSON() fully resets/overwrites its + // destination struct regardless of prior content. + saveslot_t read; errorret_t readRet = slotFromJSON(&read, json); assert_true(errorIsOk(readRet)); assert_true(stringEquals(read.cachedData.name, "Hero")); assert_int_equal(read.cachedData.playerLevel, 42); assert_true(stringEquals(read.cachedData.mapName, "overworld")); - assert_true(read.cachedData.time.time == written.cachedData.time.time); + assert_true(read.cachedData.time.time == writtenTime.time); // version/dataType are struct-only bookkeeping, never serialized - the - // reader keeps whatever saveSlotInit gave it regardless of the writer's - // version. + // reader resets them to the default regardless of the writer's version. assert_int_equal(read.version, 1); free(json); } static void test_saveSlotReadJSON_blankSlot_shortCircuits(void **state) { + // Deliberately not saveSlotInit()'d - saveSlotReadJSON() fully resets its + // destination struct itself before parsing, so these sentinel values are + // only here to prove they get overwritten, not preserved. saveslot_t slot; - saveSlotInit(&slot); + memorySet(&slot, 0, sizeof(slot)); stringCopy(slot.cachedData.name, "sentinel", sizeof(slot.cachedData.name)); slot.cachedData.playerLevel = 999; @@ -197,7 +200,6 @@ static void test_saveSlotReadJSON_blankSlot_shortCircuits(void **state) { static void test_saveSlotReadJSON_inUseMissingMapName_errors(void **state) { saveslot_t slot; - saveSlotInit(&slot); // "name" is present (in use) but "mapName" is missing - a real save // (via saveSlotNewGame) always has both, so this is corrupt. @@ -212,7 +214,6 @@ static void test_saveSlotReadJSON_inUseMissingMapName_errors(void **state) { static void test_saveSlotReadJSON_missingTimeKey_errors(void **state) { saveslot_t slot; - saveSlotInit(&slot); errorret_t ret = slotFromJSON( &slot, "{\"version\":1,\"name\":\"Hero\",\"playerLevel\":5}" @@ -223,7 +224,6 @@ static void test_saveSlotReadJSON_missingTimeKey_errors(void **state) { static void test_saveSlotReadJSON_nameTooLong_errors(void **state) { saveslot_t slot; - saveSlotInit(&slot); errorret_t ret = slotFromJSON( &slot, @@ -236,7 +236,6 @@ static void test_saveSlotReadJSON_nameTooLong_errors(void **state) { static void test_saveSlotReadJSON_nonObjectRoot_errors(void **state) { saveslot_t slot; - saveSlotInit(&slot); // A non-object root has no keys, so every field falls back to its // default - except "version" and "time", which are required and error @@ -252,7 +251,6 @@ static void test_saveSlotReadJSON_nonObjectRoot_errors(void **state) { static void test_saveSlotReadJSON_versionMissing_errors(void **state) { saveslot_t slot; - saveSlotInit(&slot); errorret_t ret = slotFromJSON( &slot, @@ -265,7 +263,6 @@ static void test_saveSlotReadJSON_versionMissing_errors(void **state) { static void test_saveSlotReadJSON_versionMismatch_errors(void **state) { saveslot_t slot; - saveSlotInit(&slot); errorret_t ret = slotFromJSON( &slot, @@ -280,7 +277,6 @@ int main(void) { assertInit(); const struct CMUnitTest tests[] = { cmocka_unit_test(test_saveSlotInit_defaults), - cmocka_unit_test(test_saveSlotInit_nullAsserts), cmocka_unit_test(test_saveSlotInUse), cmocka_unit_test(test_saveSlotInUse_nullAsserts),