From b3623c84a547a4a2e7de010c06e28296f712dbf7 Mon Sep 17 00:00:00 2001 From: Dominic Masters Date: Mon, 7 Sep 2026 10:12:37 -0500 Subject: [PATCH] Fix entity chunk-tracking leaks, add saveCanSave, use chunkindex_t Entities weren't being detached from their chunk's entities[] slot in two despawn paths (CUTSCENE_ENTITY_REMOVE, item pickup collection) - the slot leaked forever until the whole chunk unloaded. Both now call entitySetChunk(entity, CHUNK_INDEX_INVALID) before nulling the type. entity_t.chunkIndex and entitySetChunk/mapGetChunk now use chunkindex_t instead of uint8_t, matching mapGetChunkIndexAt's own -1-is-invalid convention - added CHUNK_INDEX_INVALID next to the typedef in worldpos.h rather than reusing the old 0xFF/uint8_t sentinel, which would silently mean +255 on a signed 16-bit field instead of -1. mapPositionSet now also recomputes every live entity's cached chunkIndex after mapRebuildChunkOrder() reshuffles which chunk_t sits at each chunkOrder slot - done as a direct field recompute rather than through entitySetChunk, which would've looked up each entity's now-stale old index, failed to find/clear its real registration, and inserted a duplicate entry into the correct chunk on every map position change. Also adds saveCanSave() (false while a cutscene is running) and shows the player's live chunkIndex in the ui/debug/uiplayerpos overlay. Co-Authored-By: Claude Sonnet 5 --- .../item/entity/cutsceneentityremove.c | 6 ++- src/dusk/rpg/entity/entity.c | 52 +++++++++---------- src/dusk/rpg/entity/entity.h | 16 +++--- src/dusk/rpg/entity/item/entityitem.c | 1 + src/dusk/rpg/overworld/map.c | 35 ++++++++++--- src/dusk/rpg/overworld/map.h | 10 ++-- src/dusk/rpg/overworld/worldpos.h | 5 ++ src/dusk/save/save.c | 5 ++ src/dusk/save/save.h | 8 +++ src/dusk/ui/debug/uiplayerpos.c | 5 +- src/dusk/ui/debug/uiplayerpos.h | 6 +-- 11 files changed, 97 insertions(+), 52 deletions(-) diff --git a/src/dusk/rpg/cutscene/item/entity/cutsceneentityremove.c b/src/dusk/rpg/cutscene/item/entity/cutsceneentityremove.c index ba0cce85..10b77b4a 100644 --- a/src/dusk/rpg/cutscene/item/entity/cutsceneentityremove.c +++ b/src/dusk/rpg/cutscene/item/entity/cutsceneentityremove.c @@ -14,8 +14,10 @@ void cutsceneEntityRemoveStart( const cutsceneitem_t *item, cutsceneitemdata_t *data ) { - cutsceneSystemGetEntity(item->entityRemove.entityIndex)->type = \ - ENTITY_TYPE_NULL; + entity_t *entity = + cutsceneSystemGetEntity(item->entityRemove.entityIndex); + entitySetChunk(entity, CHUNK_INDEX_INVALID); + entity->type = ENTITY_TYPE_NULL; } bool_t cutsceneEntityRemoveUpdate( diff --git a/src/dusk/rpg/entity/entity.c b/src/dusk/rpg/entity/entity.c index c51582b5..dabe2f67 100644 --- a/src/dusk/rpg/entity/entity.c +++ b/src/dusk/rpg/entity/entity.c @@ -31,7 +31,7 @@ void entityInit(entity_t *entity, const entitytype_t type) { entity->id = (uint8_t)(entity - ENTITIES); entity->globalId = ENTITY_GLOBAL_ID_NULL; entity->type = type; - entity->chunkIndex = 0xFF; + entity->chunkIndex = -1; if(ENTITY_CALLBACKS[type].init != NULL) ENTITY_CALLBACKS[type].init(entity); } @@ -288,41 +288,37 @@ void entityPositionSet(entity_t *entity, const worldpos_t pos) { entityUpdateChunk(entity); } -void entitySetChunk(entity_t *entity, const uint8_t chunkIndex) { +void entitySetChunk(entity_t *entity, const chunkindex_t chunkIndex) { assertNotNull(entity, "Entity pointer cannot be NULL"); - if(entity->chunkIndex != 0xFF) { - chunk_t *old = mapGetChunk(entity->chunkIndex); - if(old != NULL) { - for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) { - if(old->entities[i] != entity->id) continue; - old->entities[i] = 0xFF; - break; - } + chunk_t *old = mapGetChunk(entity->chunkIndex); + if(old != NULL) { + for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) { + if(old->entities[i] != entity->id) continue; + old->entities[i] = 0xFF; } } // Only claim the new chunk once actually inserted into one of its slots - // otherwise entity->chunkIndex would point at a chunk that doesn't know // about this entity, so it would never be torn down on unload. - entity->chunkIndex = 0xFF; + entity->chunkIndex = CHUNK_INDEX_INVALID; - if(chunkIndex != 0xFF) { - chunk_t *next = mapGetChunk(chunkIndex); - if(next != NULL) { - for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) { - if(next->entities[i] != 0xFF) continue; - next->entities[i] = entity->id; - entity->chunkIndex = chunkIndex; - break; - } - if(entity->chunkIndex != chunkIndex) { - consolePrint( - "entitySetChunk: chunk %u has no free entity slots, entity %u " - "left untracked", - chunkIndex, entity->id - ); - } + chunk_t *next = mapGetChunk(chunkIndex); + if(next != NULL) { + for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) { + if(next->entities[i] != 0xFF) continue; + next->entities[i] = entity->id; + entity->chunkIndex = chunkIndex; + break; + } + + if(entity->chunkIndex != chunkIndex) { + consolePrint( + "entitySetChunk: chunk %d has no free entity slots, entity %u " + "left untracked", + chunkIndex, entity->id + ); } } } @@ -333,5 +329,5 @@ void entityUpdateChunk(entity_t *entity) { chunkpos_t cp; worldPosToChunkPos(&entity->position, &cp); chunkindex_t ci = mapGetChunkIndexAt(cp); - if(ci != -1) entitySetChunk(entity, (uint8_t)ci); + entitySetChunk(entity, ci); } \ No newline at end of file diff --git a/src/dusk/rpg/entity/entity.h b/src/dusk/rpg/entity/entity.h index b676cf7e..2ef8b64c 100644 --- a/src/dusk/rpg/entity/entity.h +++ b/src/dusk/rpg/entity/entity.h @@ -39,7 +39,7 @@ typedef struct entity_s { entityinteract_t interact; - uint8_t chunkIndex; + chunkindex_t chunkIndex; } entity_t; extern entity_t ENTITIES[ENTITY_COUNT]; @@ -185,15 +185,17 @@ uint8_t entityGetAvailable(); /** * Assigns an entity to a chunk, removing it from its current chunk first. - * Pass 0xFF as chunkIndex to detach the entity from any chunk. If the - * target chunk has no free entity slots, the entity is left detached - * (chunkIndex 0xFF) rather than assigned to a chunk that isn't actually - * tracking it - entityUpdateChunk will keep retrying on subsequent moves. + * Pass CHUNK_INDEX_INVALID as chunkIndex to detach the entity from any + * chunk. If the target chunk has no free entity slots, the entity is + * left detached (chunkIndex CHUNK_INDEX_INVALID) rather than assigned to + * a chunk that isn't actually tracking it - entityUpdateChunk will keep + * retrying on subsequent moves. * * @param entity Pointer to the entity. - * @param chunkIndex Index of the chunk to assign to, or 0xFF for none. + * @param chunkIndex Index of the chunk to assign to, or + * CHUNK_INDEX_INVALID for none. */ -void entitySetChunk(entity_t *entity, const uint8_t chunkIndex); +void entitySetChunk(entity_t *entity, const chunkindex_t chunkIndex); /** * Resolves the chunk that an entity's current position falls into and diff --git a/src/dusk/rpg/entity/item/entityitem.c b/src/dusk/rpg/entity/item/entityitem.c index eccfd0f6..5d39ce09 100644 --- a/src/dusk/rpg/entity/item/entityitem.c +++ b/src/dusk/rpg/entity/item/entityitem.c @@ -40,5 +40,6 @@ void entityItemMovement(entity_t *entity) { if(!entity->data.item.collected) return; if(uiTextboxMainIsActive()) return; + entitySetChunk(entity, CHUNK_INDEX_INVALID); entity->type = ENTITY_TYPE_NULL; } diff --git a/src/dusk/rpg/overworld/map.c b/src/dusk/rpg/overworld/map.c index 337b946c..01aa8944 100644 --- a/src/dusk/rpg/overworld/map.c +++ b/src/dusk/rpg/overworld/map.c @@ -98,6 +98,29 @@ errorret_t mapPositionSet(const chunkpos_t newPos) { MAP.chunkPosition = newPos; mapRebuildChunkOrder(); + + // chunkOrder was just reshuffled, so entity->chunkIndex (an index into + // chunkOrder, not a direct chunk_t pointer) may now resolve to a + // completely different chunk than the one that actually still tracks + // this entity in its entities[] slot - a chunk that stays loaded keeps + // its entities[] contents untouched by the reorder, only its slot + // number within chunkOrder moves. Recompute the cached index directly + // from each entity's (unmoved) position rather than going through + // entitySetChunk, which would look up the stale old index, fail to + // find/clear the entity's real registration, and insert a duplicate + // entry alongside it. Entities already handled by mapChunkUnload above + // (detached to CHUNK_INDEX_INVALID, or despawned to ENTITY_TYPE_NULL) + // are skipped. + for(uint8_t i = 0; i < ENTITY_COUNT; i++) { + entity_t *entity = &ENTITIES[i]; + if(entity->type == ENTITY_TYPE_NULL) continue; + if(entity->chunkIndex == CHUNK_INDEX_INVALID) continue; + + chunkpos_t cp; + worldPosToChunkPos(&entity->position, &cp); + entity->chunkIndex = mapGetChunkIndexAt(cp); + } + errorOk(); } @@ -120,7 +143,7 @@ void mapChunkUnload(chunk_t *chunk) { if(chunk->entities[i] == 0xFF) continue; entity_t *entity = &ENTITIES[chunk->entities[i]]; if(!entityCanUnload(entity)) { - entitySetChunk(entity, 0xFF); + entitySetChunk(entity, CHUNK_INDEX_INVALID); } else { entity->type = ENTITY_TYPE_NULL; } @@ -268,7 +291,7 @@ void mapChunkLoadQueueRemove(chunk_t *chunk) { chunkindex_t mapGetChunkIndexAt(const chunkpos_t position) { - if(!mapIsLoaded()) return -1; + if(!mapIsLoaded()) return CHUNK_INDEX_INVALID; chunkpos_t relPos = { position.x - MAP.chunkPosition.x, @@ -282,14 +305,14 @@ chunkindex_t mapGetChunkIndexAt(const chunkpos_t position) { relPos.y >= MAP_CHUNK_HEIGHT || relPos.z >= MAP_CHUNK_DEPTH ) { - return -1; + return CHUNK_INDEX_INVALID; } return chunkPosToIndex(&relPos); } -chunk_t *mapGetChunk(const uint8_t index) { - if(index >= MAP_CHUNK_COUNT) return NULL; +chunk_t *mapGetChunk(const chunkindex_t index) { + if(index == CHUNK_INDEX_INVALID || index >= MAP_CHUNK_COUNT) return NULL; if(!mapIsLoaded()) return NULL; return MAP.chunkOrder[index]; } @@ -300,7 +323,7 @@ tile_t mapGetTile(const worldpos_t position) { chunkpos_t chunkPos; worldPosToChunkPos(&position, &chunkPos); chunkindex_t chunkIndex = mapGetChunkIndexAt(chunkPos); - if(chunkIndex == -1) return TILE_NULL; + if(chunkIndex == CHUNK_INDEX_INVALID) return TILE_NULL; chunk_t *chunk = mapGetChunk(chunkIndex); assertNotNull(chunk, "Chunk pointer cannot be NULL"); diff --git a/src/dusk/rpg/overworld/map.h b/src/dusk/rpg/overworld/map.h index 29be32f6..b9c48ae7 100644 --- a/src/dusk/rpg/overworld/map.h +++ b/src/dusk/rpg/overworld/map.h @@ -132,17 +132,19 @@ void mapRebuildChunkOrder(); * Gets the index of a chunk, within the world, at the given position. * * @param position The chunk position. - * @return The index of the chunk, or -1 if out of bounds. + * @return The index of the chunk, or CHUNK_INDEX_INVALID if out of + * bounds. */ chunkindex_t mapGetChunkIndexAt(const chunkpos_t position); /** * Gets a chunk by its index. - * + * * @param chunkIndex The index of the chunk. - * @return A pointer to the chunk. + * @return A pointer to the chunk, or NULL if chunkIndex is + * CHUNK_INDEX_INVALID, out of range, or no map is currently loaded. */ -chunk_t * mapGetChunk(const uint8_t chunkIndex); +chunk_t * mapGetChunk(const chunkindex_t chunkIndex); /** * Gets the tile at the given world position. diff --git a/src/dusk/rpg/overworld/worldpos.h b/src/dusk/rpg/overworld/worldpos.h index b6235150..3169839c 100644 --- a/src/dusk/rpg/overworld/worldpos.h +++ b/src/dusk/rpg/overworld/worldpos.h @@ -37,6 +37,11 @@ typedef int16_t chunkunit_t; typedef int16_t chunkindex_t; typedef uint32_t chunktileindex_t; +// Sentinel chunkindex_t value meaning "no chunk" - returned by +// mapGetChunkIndexAt when a position falls outside every loaded chunk, +// and used throughout as the "not assigned to any chunk" state. +#define CHUNK_INDEX_INVALID ((chunkindex_t)-1) + typedef int32_t worldunits_t; typedef int32_t chunkunits_t; diff --git a/src/dusk/save/save.c b/src/dusk/save/save.c index 9874680e..4586b0af 100644 --- a/src/dusk/save/save.c +++ b/src/dusk/save/save.c @@ -8,6 +8,7 @@ #include "save.h" #include "util/memory.h" #include "assert/assert.h" +#include "rpg/cutscene/cutscenesystem.h" save_t SAVE; @@ -164,6 +165,10 @@ void saveOnDeviceAvailabilityChecked(savedevice_t *device, void *user) { SAVE.deviceCurrent = (uint8_t)(device - &SAVE.devices[0]); } +bool_t saveCanSave() { + return CUTSCENE_SYSTEM.scene == NULL; +} + errorret_t saveSaveSettings() { assertTrue(SAVE.deviceCurrent != 0xFF, "No current device"); diff --git a/src/dusk/save/save.h b/src/dusk/save/save.h index 5618f581..88d86f38 100644 --- a/src/dusk/save/save.h +++ b/src/dusk/save/save.h @@ -67,6 +67,14 @@ void saveFindAvailableDevice( */ void saveOnDeviceAvailabilityChecked(savedevice_t *device, void *user); +/** + * Whether the game is in a state where saving is currently allowed. + * Currently just checks that no cutscene is running. + * + * @return True if saving is currently allowed, false otherwise. + */ +bool_t saveCanSave(); + /** * Saves the current settings to the current device. * diff --git a/src/dusk/ui/debug/uiplayerpos.c b/src/dusk/ui/debug/uiplayerpos.c index 8e7e2e65..ad430075 100644 --- a/src/dusk/ui/debug/uiplayerpos.c +++ b/src/dusk/ui/debug/uiplayerpos.c @@ -46,13 +46,14 @@ errorret_t uiPlayerPosDraw() { stringFormat( UIPLAYERPOS.text, UI_PLAYERPOS_TEXT_MAX - 1, - "%d,%d,%d[%d,%d,%d]", + "%d,%d,%d[%d,%d,%d]#%d", (int_t)player->position.x, (int_t)player->position.y, (int_t)player->position.z, (int_t)chunkPos.x, (int_t)chunkPos.y, - (int_t)chunkPos.z + (int_t)chunkPos.z, + (int_t)player->chunkIndex ); UIPLAYERPOS.label.dirty = true; diff --git a/src/dusk/ui/debug/uiplayerpos.h b/src/dusk/ui/debug/uiplayerpos.h index 0c53a289..ed84ea6b 100644 --- a/src/dusk/ui/debug/uiplayerpos.h +++ b/src/dusk/ui/debug/uiplayerpos.h @@ -28,9 +28,9 @@ extern uiplayerpos_t UIPLAYERPOS; errorret_t uiPlayerPosInit(); /** - * Draws the player world and chunk position on screen. - * Searches for the first ENTITY_TYPE_PLAYER and renders: - * WORLDX,WORLDY,WORLDZ[chunkx,chunky,chunkz] + * Draws the player world position, chunk position, and chunk index on + * screen. Searches for the first ENTITY_TYPE_PLAYER and renders: + * WORLDX,WORLDY,WORLDZ[chunkx,chunky,chunkz]#chunkIndex * * @return Any error that occurs. */