From 2ed6da9cb2e4a0063a1c72b8dc91336bd81c9a6d Mon Sep 17 00:00:00 2001 From: Cervator Date: Tue, 31 Mar 2026 20:37:01 -0400 Subject: [PATCH 1/5] fix: null-check setter and pass value in ObjectFieldMapTypeHandler.deserialize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in deserialize for private fields: 1. findSetter(field) can return null — no null check (NPE risk) 2. setter.invoke(result) passed no value argument — the setter was called with zero args, silently failing to set the field Fixed to match the serialize side's pattern: null-check the setter, log an error if missing, and pass fieldValue.get() as the argument. CodeRabbit flagged this on PR #5299 (outside-diff-range, critical). Co-Authored-By: Claude Opus 4.6 --- .../coreTypes/ObjectFieldMapTypeHandler.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/ObjectFieldMapTypeHandler.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/ObjectFieldMapTypeHandler.java index bedef481a13..80877c13a46 100644 --- a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/ObjectFieldMapTypeHandler.java +++ b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/ObjectFieldMapTypeHandler.java @@ -119,10 +119,15 @@ public Optional deserialize(PersistedData data) { if (fieldValue.isPresent()) { if (Modifier.isPrivate(field.getModifiers())) { - try { - ReflectionUtil.findSetter(field).invoke(result); - } catch (InvocationTargetException e) { - logger.error("Failed to invoke setter for field {}", field); + Method setter = ReflectionUtil.findSetter(field); + if (setter != null) { + try { + setter.invoke(result, fieldValue.get()); + } catch (InvocationTargetException e) { + logger.error("Failed to invoke setter for field {}", field); + } + } else { + logger.error("Field {} is inaccessible - private and has no setter.", field); } } else { field.set(result, fieldValue.get()); From e7ddacc108010c7cbce047dce17e0f9cba9871ea Mon Sep 17 00:00:00 2001 From: Cervator Date: Tue, 31 Mar 2026 21:17:41 -0400 Subject: [PATCH 2/5] fix: remove duplicate populateWithDefaultHandlers call in TypeHandlerLibraryImpl forModuleEnvironment() creates an instance via the @Inject constructor (which already calls populateWithDefaultHandlers), then called it again, duplicating all default handler/factory registrations. The handlers append to a list without deduplication. CodeRabbit flagged this on PR #5299 (outside-diff-range, major). Co-Authored-By: Claude Opus 4.6 --- .../persistence/typeHandling/TypeHandlerLibraryImpl.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/engine/src/main/java/org/terasology/engine/persistence/typeHandling/TypeHandlerLibraryImpl.java b/engine/src/main/java/org/terasology/engine/persistence/typeHandling/TypeHandlerLibraryImpl.java index 0cbfabed82a..8ac14237b24 100644 --- a/engine/src/main/java/org/terasology/engine/persistence/typeHandling/TypeHandlerLibraryImpl.java +++ b/engine/src/main/java/org/terasology/engine/persistence/typeHandling/TypeHandlerLibraryImpl.java @@ -111,11 +111,8 @@ public static TypeHandlerLibrary withReflections(Reflections reflections) { } public static TypeHandlerLibrary forModuleEnvironment(ModuleManager moduleManager, TypeRegistry typeRegistry) { - TypeHandlerLibrary library = new TypeHandlerLibraryImpl(moduleManager, typeRegistry); - - populateWithDefaultHandlers(library); - - return library; + // The @Inject constructor already calls populateWithDefaultHandlers + return new TypeHandlerLibraryImpl(moduleManager, typeRegistry); } private static void populateWithDefaultHandlers(TypeHandlerLibrary serializationLibrary) { From 5fadcc380acf434dd58aba144629a06ed0034dc9 Mon Sep 17 00:00:00 2001 From: Cervator Date: Tue, 31 Mar 2026 21:26:38 -0400 Subject: [PATCH 3/5] refactor: accept EngineEntityManager directly in EntityAwareWorldProvider Replace EntityManager parameter with EngineEntityManager in the @Inject constructor, eliminating the runtime downcast. Updated the two WorldProviderCoreWorkAround subclasses to match. Gestalt DI auto-maps PojoEntityManager to all its interfaces including EngineEntityManager. CodeRabbit flagged this on PR #5299 (minor). Co-Authored-By: Claude Opus 4.6 --- .../core/modes/loadProcesses/InitialiseRemoteWorld.java | 3 ++- .../engine/core/modes/loadProcesses/InitialiseWorld.java | 3 ++- .../engine/world/internal/EntityAwareWorldProvider.java | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java index 0388c963375..69388f54a0d 100644 --- a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java +++ b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java @@ -10,6 +10,7 @@ import org.terasology.engine.core.modes.SingleStepLoadProcess; import org.terasology.engine.core.subsystem.RenderingSubsystemFactory; import org.terasology.engine.entitySystem.entity.EntityManager; +import org.terasology.engine.entitySystem.entity.internal.EngineEntityManager; import org.terasology.engine.game.GameManifest; import org.terasology.engine.logic.players.LocalPlayer; import org.terasology.engine.recording.DirectionAndOriginPosRecorderList; @@ -91,7 +92,7 @@ public static final class WorldProviderCoreWorkAround extends EntityAwareWorldPr implements WorldProviderCore, BlockEntityRegistry { @Inject public WorldProviderCoreWorkAround(WorldInfo info, ChunkProvider chunkProvider, BlockManager blockManager, - EntityManager entityManager, ComponentSystemManager componentSystemManager) { + EngineEntityManager entityManager, ComponentSystemManager componentSystemManager) { super(new WorldProviderCoreImpl(info, chunkProvider, blockManager, entityManager), entityManager, componentSystemManager); } } diff --git a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java index 112dd9e3f45..ec655fce44f 100644 --- a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java +++ b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java @@ -16,6 +16,7 @@ import org.terasology.engine.core.modes.StateMainMenu; import org.terasology.engine.core.subsystem.RenderingSubsystemFactory; import org.terasology.engine.entitySystem.entity.EntityManager; +import org.terasology.engine.entitySystem.entity.internal.EngineEntityManager; import org.terasology.engine.game.GameManifest; import org.terasology.engine.logic.players.LocalPlayer; import org.terasology.engine.persistence.StorageManager; @@ -160,7 +161,7 @@ public static final class WorldProviderCoreWorkAround extends EntityAwareWorldPr implements WorldProviderCore, BlockEntityRegistry { @Inject public WorldProviderCoreWorkAround(WorldInfo info, ChunkProvider chunkProvider, BlockManager blockManager, - EntityManager entityManager, ComponentSystemManager componentSystemManager) { + EngineEntityManager entityManager, ComponentSystemManager componentSystemManager) { super(new WorldProviderCoreImpl(info, chunkProvider, blockManager, entityManager), entityManager, componentSystemManager); } } diff --git a/engine/src/main/java/org/terasology/engine/world/internal/EntityAwareWorldProvider.java b/engine/src/main/java/org/terasology/engine/world/internal/EntityAwareWorldProvider.java index cee1299afa7..1827f6738d9 100644 --- a/engine/src/main/java/org/terasology/engine/world/internal/EntityAwareWorldProvider.java +++ b/engine/src/main/java/org/terasology/engine/world/internal/EntityAwareWorldProvider.java @@ -77,9 +77,9 @@ public EntityAwareWorldProvider(WorldProviderCore base, Context context) { } @Inject - public EntityAwareWorldProvider(WorldProviderCore base, EntityManager entityManager, ComponentSystemManager componentSystemManager) { + public EntityAwareWorldProvider(WorldProviderCore base, EngineEntityManager entityManager, ComponentSystemManager componentSystemManager) { super(base); - this.entityManager = (EngineEntityManager) entityManager; + this.entityManager = entityManager; componentSystemManager.register(getTime()); } From 617c252361153785554193123a8927195c4df07e Mon Sep 17 00:00:00 2001 From: Cervator Date: Tue, 31 Mar 2026 21:28:36 -0400 Subject: [PATCH 4/5] docs: update EngineSubsystem lifecycle Javadoc for ServiceRegistry The Javadoc still said "add anything into the root context" when the methods now receive a ServiceRegistry. Updated to say "register services" and "build the root context" to match the DI migration. CodeRabbit flagged this on PR #5299 (minor). Co-Authored-By: Claude Opus 4.6 --- .../engine/core/subsystem/EngineSubsystem.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java b/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java index a19713777a4..e2f1d6e61a3 100644 --- a/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java @@ -20,19 +20,19 @@ public interface EngineSubsystem { /** * Called on each system before initialisation. - * This is an opportunity to add anything into the root context that will carry across the entire run of the engine, - * and may be used by other systems. + * This is an opportunity to register services that will be available from the root context + * for the entire run of the engine, and may be used by other systems. * - * @param serviceRegistry the service registry used to create the root context that will carry across the entire run of the engine. + * @param serviceRegistry the service registry used to build the root context for the entire run of the engine. */ default void preInitialise(ServiceRegistry serviceRegistry) { } /** - * Called to initialise the system + * Called to initialise the system. * - * @param engine The game engine - * @param serviceRegistry The service registry used to create the context that will carry across the entire run of the engine. + * @param engine The game engine + * @param serviceRegistry The service registry used to build the context for the entire run of the engine. */ default void initialise(GameEngine engine, ServiceRegistry serviceRegistry) { } From 2ed7b370a87da7248b8689e195cb96d34abe7e6f Mon Sep 17 00:00:00 2001 From: Cervator Date: Tue, 31 Mar 2026 22:02:00 -0400 Subject: [PATCH 5/5] fix: remove unused EntityManager imports, align Javadoc style Remove unused EntityManager imports left behind after switching to EngineEntityManager. Align @param Javadoc style (capitalize, drop trailing periods) to match existing conventions in the file. CheckStyle + CodeRabbit review feedback on PR #5318. Co-Authored-By: Claude Opus 4.6 --- .../core/modes/loadProcesses/InitialiseRemoteWorld.java | 1 - .../engine/core/modes/loadProcesses/InitialiseWorld.java | 1 - .../org/terasology/engine/core/subsystem/EngineSubsystem.java | 4 ++-- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java index 69388f54a0d..f40e914e3cc 100644 --- a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java +++ b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRemoteWorld.java @@ -9,7 +9,6 @@ import org.terasology.engine.core.TerasologyConstants; import org.terasology.engine.core.modes.SingleStepLoadProcess; import org.terasology.engine.core.subsystem.RenderingSubsystemFactory; -import org.terasology.engine.entitySystem.entity.EntityManager; import org.terasology.engine.entitySystem.entity.internal.EngineEntityManager; import org.terasology.engine.game.GameManifest; import org.terasology.engine.logic.players.LocalPlayer; diff --git a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java index ec655fce44f..1e874ccc671 100644 --- a/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java +++ b/engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java @@ -15,7 +15,6 @@ import org.terasology.engine.core.modes.SingleStepLoadProcess; import org.terasology.engine.core.modes.StateMainMenu; import org.terasology.engine.core.subsystem.RenderingSubsystemFactory; -import org.terasology.engine.entitySystem.entity.EntityManager; import org.terasology.engine.entitySystem.entity.internal.EngineEntityManager; import org.terasology.engine.game.GameManifest; import org.terasology.engine.logic.players.LocalPlayer; diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java b/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java index e2f1d6e61a3..65c79e00590 100644 --- a/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/EngineSubsystem.java @@ -23,7 +23,7 @@ public interface EngineSubsystem { * This is an opportunity to register services that will be available from the root context * for the entire run of the engine, and may be used by other systems. * - * @param serviceRegistry the service registry used to build the root context for the entire run of the engine. + * @param serviceRegistry The service registry used to build the root context for the entire run of the engine */ default void preInitialise(ServiceRegistry serviceRegistry) { } @@ -32,7 +32,7 @@ default void preInitialise(ServiceRegistry serviceRegistry) { * Called to initialise the system. * * @param engine The game engine - * @param serviceRegistry The service registry used to build the context for the entire run of the engine. + * @param serviceRegistry The service registry used to build the context for the entire run of the engine */ default void initialise(GameEngine engine, ServiceRegistry serviceRegistry) { }