Skip to content

bug: inventory items intermittently lost across save/reload #5364

Description

@soloturn

AI-assisted issue. Filed by agent driven by @soloturn via GDD.

Context

Investigating a report that saved games silently lose inventory items on reload. Started narrow (a ManualLabor-crafted tool wasn't surviving reload - root-caused to ToolItemOutputProcessPartCommonSystem mis-setting entity persistence, fixed separately in Terasology/ManualLabor#64). During verification, a second, unrelated symptom kept reproducing.

Problem / Current State

Individual inventory items intermittently vanish across a save/exit/reload cycle, independent of item type:

  • Reproduced in a JoshariasSurvival world: whatever item occupied the 2nd inventory slot was reliably the one lost, regardless of which item type was actually in that slot (confirmed with a debug dump showing persistent=true for that exact entity right before the save that then lost it - see Technical Notes).
  • Reproduced separately in a much lighter/core-play world (few modules active): other items (e.g. a picked-up "Snow" item) present before save, gone after reload - not necessarily the 2nd slot this time, but still within the inventory list.

So it isn't tied to one item type, one module set, or one fixed slot index - it looks like a timing/ordering-sensitive entity persistence issue that gets more likely to trigger with a larger active entity/system graph (JSS) but isn't exclusive to it.

Acceptance Criteria

  • Root-cause why a List<EntityRef> field (e.g. InventoryComponent.itemSlots) can end up missing an entry that was persistent=true at save time.
  • Reliable repro (e.g. an MTE integration test: give an entity, save, reload, assert inventory slot still resolves) added to guard against regression.
  • Fix such that a persistent entity referenced from another persistent entity's component always survives a save/reload cycle, regardless of load order.

Technical Notes

Root cause pinned down precisely:

  • engine/src/main/java/org/terasology/engine/persistence/internal/EntityRestorer.java:35: store.getEntityList().forEach(serializer::deserialize) - single sequential pass over every entity in the store, in whatever order they were written.
  • engine/src/main/java/org/terasology/engine/persistence/serializers/EntitySerializer.java:158-166:
    public EntityRef deserialize(EntityData.Entity entityData) {
        Map<Class<? extends Component>, Component> componentMap = createInitialComponents(entityData);
        deserializeOntoComponents(entityData, componentMap);   // resolves every EntityRef field NOW
        if (ignoringEntityId) {
            return entityManager.create(componentMap.values());
        } else {
            return entityManager.createEntityWithId(entityData.getId(), componentMap.values());  // entity only registered HERE
        }
    }
    Component data - including every EntityRef/List<EntityRef> field such as InventoryComponent.itemSlots - is fully deserialized and resolved before the entity that owns those components is registered with the entity manager.
  • engine/src/main/java/org/terasology/engine/persistence/typeHandling/extensionTypes/EntityRefTypeHandler.java, deserialize(): resolves a referenced id via entityManager.getEntity(id) synchronously, once, with no deferred/lazy binding.
  • engine/src/main/java/org/terasology/engine/entitySystem/entity/internal/PojoEntityManager.java:322-327, getEntity(long id): getPool(id).map(pool -> pool.getEntity(id)).orElse(EntityRef.NULL) - if that id hasn't been registered yet, this returns EntityRef.NULL (a permanent "doesn't exist" stub, not a Java null), so the earlier suspicion that CollectionTypeHandler.deserialize's element.ifPresent(items::add) silently drops list entries doesn't apply to EntityRef elements specifically - EntityRefTypeHandler never returns Optional.empty(). The list position is preserved, just filled with a stub that never resolves to anything.

Net effect: any entity A whose component references entity B by id will permanently lose that reference if B happens to appear later than A in store.getEntityList() - there is no second resolution pass once the rest of the store has been loaded. This is a general forward-reference bug in entity deserialization, not inventory-specific; inventories just make a dropped reference visible as a missing item. Larger/more interconnected entity graphs (more active modules, e.g. JoshariasSurvival) make hitting a backward-ordered pair more likely, but a small core-play world can still hit it by chance.

Likely fix shape: split EntityRestorer/EntitySerializer into two passes - (1) reserve/register every entity id in the store first (empty or minimal placeholder), so entityManager.getEntity(id) can resolve any id in the store from the very start of pass 2, then (2) deserialize and apply full component data for every entity now that all ids are resolvable.

Diagnostic command used to confirm persistent=true at save time (temporary, can be removed once this is fixed): jsInventoryDump added to modules/JoshariasSurvival/.../systems/DemoSystem.java in this investigation - not intended to ship, just flagging it exists if useful for repro.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type: BugIssues reporting and PRs fixing problems

    Type

    No type

    Projects

    Status
    No status
    Status
    ✅ Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions