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 <[email protected]>
This commit is contained in:
2026-09-07 10:12:37 -05:00
co-authored by Claude Sonnet 5
parent 45c1f9004b
commit b3623c84a5
11 changed files with 97 additions and 52 deletions
@@ -14,8 +14,10 @@ void cutsceneEntityRemoveStart(
const cutsceneitem_t *item, const cutsceneitem_t *item,
cutsceneitemdata_t *data cutsceneitemdata_t *data
) { ) {
cutsceneSystemGetEntity(item->entityRemove.entityIndex)->type = \ entity_t *entity =
ENTITY_TYPE_NULL; cutsceneSystemGetEntity(item->entityRemove.entityIndex);
entitySetChunk(entity, CHUNK_INDEX_INVALID);
entity->type = ENTITY_TYPE_NULL;
} }
bool_t cutsceneEntityRemoveUpdate( bool_t cutsceneEntityRemoveUpdate(
+24 -28
View File
@@ -31,7 +31,7 @@ void entityInit(entity_t *entity, const entitytype_t type) {
entity->id = (uint8_t)(entity - ENTITIES); entity->id = (uint8_t)(entity - ENTITIES);
entity->globalId = ENTITY_GLOBAL_ID_NULL; entity->globalId = ENTITY_GLOBAL_ID_NULL;
entity->type = type; entity->type = type;
entity->chunkIndex = 0xFF; entity->chunkIndex = -1;
if(ENTITY_CALLBACKS[type].init != NULL) ENTITY_CALLBACKS[type].init(entity); 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); 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"); assertNotNull(entity, "Entity pointer cannot be NULL");
if(entity->chunkIndex != 0xFF) { chunk_t *old = mapGetChunk(entity->chunkIndex);
chunk_t *old = mapGetChunk(entity->chunkIndex); if(old != NULL) {
if(old != NULL) { for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) {
for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) { if(old->entities[i] != entity->id) continue;
if(old->entities[i] != entity->id) continue; old->entities[i] = 0xFF;
old->entities[i] = 0xFF;
break;
}
} }
} }
// Only claim the new chunk once actually inserted into one of its slots - // 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 // otherwise entity->chunkIndex would point at a chunk that doesn't know
// about this entity, so it would never be torn down on unload. // 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);
chunk_t *next = mapGetChunk(chunkIndex); if(next != NULL) {
if(next != NULL) { for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) {
for(uint8_t i = 0; i < CHUNK_ENTITY_COUNT_MAX; i++) { if(next->entities[i] != 0xFF) continue;
if(next->entities[i] != 0xFF) continue; next->entities[i] = entity->id;
next->entities[i] = entity->id; entity->chunkIndex = chunkIndex;
entity->chunkIndex = chunkIndex; break;
break; }
}
if(entity->chunkIndex != chunkIndex) { if(entity->chunkIndex != chunkIndex) {
consolePrint( consolePrint(
"entitySetChunk: chunk %u has no free entity slots, entity %u " "entitySetChunk: chunk %d has no free entity slots, entity %u "
"left untracked", "left untracked",
chunkIndex, entity->id chunkIndex, entity->id
); );
}
} }
} }
} }
@@ -333,5 +329,5 @@ void entityUpdateChunk(entity_t *entity) {
chunkpos_t cp; chunkpos_t cp;
worldPosToChunkPos(&entity->position, &cp); worldPosToChunkPos(&entity->position, &cp);
chunkindex_t ci = mapGetChunkIndexAt(cp); chunkindex_t ci = mapGetChunkIndexAt(cp);
if(ci != -1) entitySetChunk(entity, (uint8_t)ci); entitySetChunk(entity, ci);
} }
+9 -7
View File
@@ -39,7 +39,7 @@ typedef struct entity_s {
entityinteract_t interact; entityinteract_t interact;
uint8_t chunkIndex; chunkindex_t chunkIndex;
} entity_t; } entity_t;
extern entity_t ENTITIES[ENTITY_COUNT]; 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. * 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 * Pass CHUNK_INDEX_INVALID as chunkIndex to detach the entity from any
* target chunk has no free entity slots, the entity is left detached * chunk. If the target chunk has no free entity slots, the entity is
* (chunkIndex 0xFF) rather than assigned to a chunk that isn't actually * left detached (chunkIndex CHUNK_INDEX_INVALID) rather than assigned to
* tracking it - entityUpdateChunk will keep retrying on subsequent moves. * a chunk that isn't actually tracking it - entityUpdateChunk will keep
* retrying on subsequent moves.
* *
* @param entity Pointer to the entity. * @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 * Resolves the chunk that an entity's current position falls into and
+1
View File
@@ -40,5 +40,6 @@ void entityItemMovement(entity_t *entity) {
if(!entity->data.item.collected) return; if(!entity->data.item.collected) return;
if(uiTextboxMainIsActive()) return; if(uiTextboxMainIsActive()) return;
entitySetChunk(entity, CHUNK_INDEX_INVALID);
entity->type = ENTITY_TYPE_NULL; entity->type = ENTITY_TYPE_NULL;
} }
+29 -6
View File
@@ -98,6 +98,29 @@ errorret_t mapPositionSet(const chunkpos_t newPos) {
MAP.chunkPosition = newPos; MAP.chunkPosition = newPos;
mapRebuildChunkOrder(); 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(); errorOk();
} }
@@ -120,7 +143,7 @@ void mapChunkUnload(chunk_t *chunk) {
if(chunk->entities[i] == 0xFF) continue; if(chunk->entities[i] == 0xFF) continue;
entity_t *entity = &ENTITIES[chunk->entities[i]]; entity_t *entity = &ENTITIES[chunk->entities[i]];
if(!entityCanUnload(entity)) { if(!entityCanUnload(entity)) {
entitySetChunk(entity, 0xFF); entitySetChunk(entity, CHUNK_INDEX_INVALID);
} else { } else {
entity->type = ENTITY_TYPE_NULL; entity->type = ENTITY_TYPE_NULL;
} }
@@ -268,7 +291,7 @@ void mapChunkLoadQueueRemove(chunk_t *chunk) {
chunkindex_t mapGetChunkIndexAt(const chunkpos_t position) { chunkindex_t mapGetChunkIndexAt(const chunkpos_t position) {
if(!mapIsLoaded()) return -1; if(!mapIsLoaded()) return CHUNK_INDEX_INVALID;
chunkpos_t relPos = { chunkpos_t relPos = {
position.x - MAP.chunkPosition.x, position.x - MAP.chunkPosition.x,
@@ -282,14 +305,14 @@ chunkindex_t mapGetChunkIndexAt(const chunkpos_t position) {
relPos.y >= MAP_CHUNK_HEIGHT || relPos.y >= MAP_CHUNK_HEIGHT ||
relPos.z >= MAP_CHUNK_DEPTH relPos.z >= MAP_CHUNK_DEPTH
) { ) {
return -1; return CHUNK_INDEX_INVALID;
} }
return chunkPosToIndex(&relPos); return chunkPosToIndex(&relPos);
} }
chunk_t *mapGetChunk(const uint8_t index) { chunk_t *mapGetChunk(const chunkindex_t index) {
if(index >= MAP_CHUNK_COUNT) return NULL; if(index == CHUNK_INDEX_INVALID || index >= MAP_CHUNK_COUNT) return NULL;
if(!mapIsLoaded()) return NULL; if(!mapIsLoaded()) return NULL;
return MAP.chunkOrder[index]; return MAP.chunkOrder[index];
} }
@@ -300,7 +323,7 @@ tile_t mapGetTile(const worldpos_t position) {
chunkpos_t chunkPos; chunkpos_t chunkPos;
worldPosToChunkPos(&position, &chunkPos); worldPosToChunkPos(&position, &chunkPos);
chunkindex_t chunkIndex = mapGetChunkIndexAt(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); chunk_t *chunk = mapGetChunk(chunkIndex);
assertNotNull(chunk, "Chunk pointer cannot be NULL"); assertNotNull(chunk, "Chunk pointer cannot be NULL");
+5 -3
View File
@@ -132,7 +132,8 @@ void mapRebuildChunkOrder();
* Gets the index of a chunk, within the world, at the given position. * Gets the index of a chunk, within the world, at the given position.
* *
* @param position The chunk 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); chunkindex_t mapGetChunkIndexAt(const chunkpos_t position);
@@ -140,9 +141,10 @@ chunkindex_t mapGetChunkIndexAt(const chunkpos_t position);
* Gets a chunk by its index. * Gets a chunk by its index.
* *
* @param chunkIndex The index of the chunk. * @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. * Gets the tile at the given world position.
+5
View File
@@ -37,6 +37,11 @@ typedef int16_t chunkunit_t;
typedef int16_t chunkindex_t; typedef int16_t chunkindex_t;
typedef uint32_t chunktileindex_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 worldunits_t;
typedef int32_t chunkunits_t; typedef int32_t chunkunits_t;
+5
View File
@@ -8,6 +8,7 @@
#include "save.h" #include "save.h"
#include "util/memory.h" #include "util/memory.h"
#include "assert/assert.h" #include "assert/assert.h"
#include "rpg/cutscene/cutscenesystem.h"
save_t SAVE; save_t SAVE;
@@ -164,6 +165,10 @@ void saveOnDeviceAvailabilityChecked(savedevice_t *device, void *user) {
SAVE.deviceCurrent = (uint8_t)(device - &SAVE.devices[0]); SAVE.deviceCurrent = (uint8_t)(device - &SAVE.devices[0]);
} }
bool_t saveCanSave() {
return CUTSCENE_SYSTEM.scene == NULL;
}
errorret_t saveSaveSettings() { errorret_t saveSaveSettings() {
assertTrue(SAVE.deviceCurrent != 0xFF, "No current device"); assertTrue(SAVE.deviceCurrent != 0xFF, "No current device");
+8
View File
@@ -67,6 +67,14 @@ void saveFindAvailableDevice(
*/ */
void saveOnDeviceAvailabilityChecked(savedevice_t *device, void *user); 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. * Saves the current settings to the current device.
* *
+3 -2
View File
@@ -46,13 +46,14 @@ errorret_t uiPlayerPosDraw() {
stringFormat( stringFormat(
UIPLAYERPOS.text, UIPLAYERPOS.text,
UI_PLAYERPOS_TEXT_MAX - 1, 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.x,
(int_t)player->position.y, (int_t)player->position.y,
(int_t)player->position.z, (int_t)player->position.z,
(int_t)chunkPos.x, (int_t)chunkPos.x,
(int_t)chunkPos.y, (int_t)chunkPos.y,
(int_t)chunkPos.z (int_t)chunkPos.z,
(int_t)player->chunkIndex
); );
UIPLAYERPOS.label.dirty = true; UIPLAYERPOS.label.dirty = true;
+3 -3
View File
@@ -28,9 +28,9 @@ extern uiplayerpos_t UIPLAYERPOS;
errorret_t uiPlayerPosInit(); errorret_t uiPlayerPosInit();
/** /**
* Draws the player world and chunk position on screen. * Draws the player world position, chunk position, and chunk index on
* Searches for the first ENTITY_TYPE_PLAYER and renders: * screen. Searches for the first ENTITY_TYPE_PLAYER and renders:
* WORLDX,WORLDY,WORLDZ[chunkx,chunky,chunkz] * WORLDX,WORLDY,WORLDZ[chunkx,chunky,chunkz]#chunkIndex
* *
* @return Any error that occurs. * @return Any error that occurs.
*/ */