Skip to content

Instantly share code, notes, and snippets.

@Cervator

Cervator/gist:4ef1537704e225c60f66c206ba4f0246

Created March 29, 2026 04:40
Show Gist options

  • Select an option

    Learn more about clone URLs
  • Save Cervator/4ef1537704e225c60f66c206ba4f0246 to your computer and use it in GitHub Desktop.

Select an option

Learn more about clone URLs
Save Cervator/4ef1537704e225c60f66c206ba4f0246 to your computer and use it in GitHub Desktop.
Download ZIP
Example Claude-with-GDD interaction during Terasology DI refactor PR post-review process
Raw
gistfile1.txt
Done - I merged the remote develop and rebased our topic branch. Should be clear to continue
● Bash(cd /d/Dev/GitWS/yggdrasil/components/terasology && git log --oneline -5)
⎿ fde633ea2 chore: bump gestalt to 8.0.1-SNAPSHOT
015e59d89 Merge pull request #5299 from MovingBlocks/temp/gestalt-di-migration
22589d04b style: fix some more checkstyle issues
… +2 lines (ctrl+o to expand)
● Good — our version bump commit is on top of the merged PR. On to Task 2.
Read 1 file (ctrl+o to expand)
● Update(engine\src\main\java\org\terasology\engine\rendering\nui\layers\mainMenu\UniverseSetupScreen.java)
⎿ Added 1 line
383 * needed for successful game creation.
384 */
385 public void setEnvironment(UniverseWrapper universeWrapper) {
386 + this.universeWrapper = universeWrapper;
387 prepareContext();
388
389 DependencyResolver resolver = new DependencyResolver(moduleManager.getRegistry());
● One-line fix. The initialise() default at line 146 stays as a safe fallback for initial binding setup.
Can you test when you get a chance? Create a new game via Advanced Game Setup → Create World → Universe Setup. Check that the seed carries through and a game actually starts with
it.
Now commit.
● Write(/d/Dev/GitWS/yggdrasil/.commits/universe-setup-wrapper.md)
⎿ Wrote 11 lines to ..\..\..\..\..\d\Dev\GitWS\yggdrasil\.commits\universe-setup-wrapper.md
1 ---
2 message: "fix: assign UniverseWrapper in UniverseSetupScreen.setEnvironment"
3 add:
4 - engine/src/main/java/org/terasology/engine/rendering/nui/layers/mainMenu/UniverseSetupScreen.java
5 ---
6 setEnvironment() received a UniverseWrapper parameter but never assigned it
7 to the instance field. The screen's UI bindings read from the field, not the
8 context, so seed/generator/server settings from AdvancedGameSetupScreen were
9 silently discarded.
… +1 line (ctrl+o to expand)
● Bash(cd /d/Dev/GitWS/yggdrasil && bash scripts/ws commit terasology .commits/universe-setup-wrapper.md)
⎿ Staging files from bodyfile frontmatter...
[review/post-di 9a29bf147] fix: assign UniverseWrapper in UniverseSetupScreen.setEnvironment
1 file changed, 1 insertion(+)
● Moving to Task 3 — PhysicsEngineManager. I need to check how BulletPhysics relates to its interfaces first.
Searched for 3 patterns (ctrl+o to expand)
● Good — BulletPhysics implements PhysicsEngine, Physics and PhysicsEngine extends Physics. Now I need to understand how other similar patterns work — where one class is registered
as multiple interfaces but should be the same singleton instance. Let me check how the entity system does it.
Searched for 2 patterns, read 1 file (ctrl+o to expand)
● Key finding: StateIngame.java:152 calls context.get(PhysicsEngine.class).dispose(). So code does look up PhysicsEngine from the context — the interface binding is needed.
Let me also check how InitialisePhysics works and if there's a fallback via the old getNewPhysicsEngine path.
Searched for 1 pattern, read 1 file (ctrl+o to expand)
● getNewPhysicsEngine() exists but is never called anywhere else. So the DI path is the only path, and it's missing the interface bindings. Let me check how BulletPhysics gets its
constructor deps.
Searched for 2 patterns, read 1 file (ctrl+o to expand)
● BulletPhysics has an @Inject constructor taking BlockEntityRegistry. The current registration just does serviceRegistry.with(BulletPhysics.class).lifetime(Lifetime.Singleton) — no .use() call and no interface bindings.
Now, the key question is whether registering BulletPhysics.class, PhysicsEngine.class, and Physics.class separately would create three instances or one shared instance. Let me
check how other parts of the codebase handle this.
Searched for 3 patterns, read 1 file (ctrl+o to expand)
● Both interfaces are looked up:
- Physics.class — via CoreRegistry.get() in BlockItemSystem and LocalPlayer
- PhysicsEngine.class — via context.get() in StateIngame for disposal
Since CoreRegistry delegates to Context which delegates to BeanContext, both lookups need to resolve. And they need to resolve to the same instance (one physics world, not two).
The safest pattern: register with a shared provider.
Read 1 file (ctrl+o to expand)
● Now let me write the fix. The cleanest approach matching existing patterns: register BulletPhysics as the concrete singleton, then register the interfaces pointing to the same
class. In Gestalt DI, if BulletPhysics.class is already registered as a singleton, registering PhysicsEngine.class with .use(BulletPhysics.class) should resolve to the same bean
if the container deduplicates by impl class. But I'm not 100% sure of Gestalt's behavior here — let me be safe and use a provider.
● Update(components\terasology\engine\src\main\java\org\terasology\engine\physics\engine\PhysicsEngineManager.java)
⎿ Added 3 lines, removed 1 line
25 private PhysicsEngineManager() { }
26
27 public static void registerPhysicsEngine(ServiceRegistry serviceRegistry) {
28 - serviceRegistry.with(BulletPhysics.class).lifetime(Lifetime.Singleton);
28 + serviceRegistry.with(BulletPhysics.class).lifetime(Lifetime.Singleton).use(BulletPhysics.class);
29 + serviceRegistry.with(PhysicsEngine.class).lifetime(Lifetime.Singleton).use(BulletPhysics.class);
30 + serviceRegistry.with(Physics.class).lifetime(Lifetime.Singleton).use(BulletPhysics.class);
31 }
32
33 /**
● We also need the Physics import.
Searched for 1 pattern, read 1 file (ctrl+o to expand)
● Physics is in the same package as PhysicsEngine — it's org.terasology.engine.physics.engine.Physics. Wait, let me check:
Searched for 1 pattern (ctrl+o to expand)
● Physics is in org.terasology.engine.physics, not the engine subpackage. Need an import.
● Update(components\terasology\engine\src\main\java\org\terasology\engine\physics\engine\PhysicsEngineManager.java)
⎿ Added 1 line
4
5 import org.terasology.context.Lifetime;
6 import org.terasology.engine.context.Context;
7 +import org.terasology.engine.physics.Physics;
8 import org.terasology.engine.physics.bullet.BulletPhysics;
9 import org.terasology.engine.physics.bullet.shapes.BulletCollisionShapeFactory;
10 import org.terasology.engine.physics.shapes.CollisionShape;
● A question for you: Three separate .with().use() registrations will likely create three separate BulletPhysics instances (three physics worlds — bad). I'm not certain of Gestalt
DI's deduplication behavior. Do you know if Gestalt shares the singleton instance when the same .use(BulletPhysics.class) is registered under multiple keys? If not, we may need a
provider pattern where we construct one instance and register it under all three keys. I don't want to ship a change that creates three physics engines.
● Write(.commits\physics-engine-binding.md)
⎿ Wrote 12 lines to ..\..\..\d\Dev\GitWS\yggdrasil\.commits\physics-engine-binding.md
1 ---
2 message: "fix: register PhysicsEngine and Physics interface bindings in DI"
3 add:
4 - engine/src/main/java/org/terasology/engine/physics/engine/PhysicsEngineManager.java
5 ---
6 registerPhysicsEngine() registered BulletPhysics as a concrete class but did
7 not bind it to the PhysicsEngine or Physics interfaces. Code requesting these
8 interfaces from the DI container (StateIngame, BlockItemSystem, LocalPlayer)
9 would fail to resolve them.
… +2 lines (ctrl+o to expand)
● Holding on the commit until you can confirm the singleton sharing behavior — or should I go ahead and commit, then we verify with an in-game test?
✻ Baked for 2m 21s
❯ I tested in-game, even just first going for the universe setup, but it looks like spawning failed due to the physics change not being right: 00:23:48.882 [main] ERROR
o.t.engine.core.modes.StateLoading - Error while loading org.terasology.engine.core.modes.loadProcesses.AwaitCharacterSpawn@5ce03a9d
java.lang.NullPointerException: Cannot invoke "org.terasology.engine.physics.engine.PhysicsEngine.update(float)" because "this.physics" is null
at org.terasology.engine.physics.engine.PhysicsSystem.update(PhysicsSystem.java:174)
at org.terasology.engine.core.modes.loadProcesses.AwaitCharacterSpawn.step(AwaitCharacterSpawn.java:37)
● The DI isn't resolving PhysicsEngine for PhysicsSystem. Let me trace how it gets injected.
Searched for 1 pattern, read 1 file (ctrl+o to expand)
● Line 72-73: @In private PhysicsEngine physics; — uses the old @In annotation for field injection from the Context. So PhysicsSystem is a component system registered by
ComponentSystemManager, which injects @In fields via Context.get(PhysicsEngine.class).
The problem is that registerPhysicsEngine() runs in the pre-context-switch phase (in initHost/initClient), putting things in the ServiceRegistry. But PhysicsSystem is a component
system that gets injected after SwitchToContextStep creates the DI-backed context.
Let me check the ordering — InitialisePhysics is before SwitchToContextStep in the loading pipeline, and PhysicsSystem is registered during RegisterSystems/RegisterWorldSystems
which happens after the switch. The question is: does the context created at SwitchToContextStep (line 325) actually include the physics registrations from the serviceRegistry?
Searched for 1 pattern, read 1 file (ctrl+o to expand)
● OK, engine.createChildContext(serviceRegistry) — the serviceRegistry IS passed. So the registrations should be visible. The issue must be that the DI isn't resolving
PhysicsEngine.class when @In injection happens. Let me check how @In injection resolves — does it go through the DI BeanContext or just the Context map?
Searched for 2 patterns, read 1 file (ctrl+o to expand)
● Line 40: @In injection calls context.get(field.getType()). So it looks up PhysicsEngine.class from the Context. Let me check how ContextImpl.get() resolves from the BeanContext.
Searched for 1 pattern, read 1 file (ctrl+o to expand)
● The error is new — physics worked before our change. Let me check what I might have broken. The original code had no .use() and no interface bindings, yet physics worked. There
must be another path that registers PhysicsEngine in the context.
Searched for 4 patterns, read 1 file (ctrl+o to expand)
● No @Share on PhysicsSystem. Let me check if the old registration without .use() was actually working differently — maybe Gestalt auto-discovers the @Inject constructor when no
.use() is specified, and our addition of .use(BulletPhysics.class) is somehow conflicting.
Let me revert to exactly the original registration and only add the interface bindings:
Read 1 file (ctrl+o to expand)
● Let me first revert to the original code to stop the regression, then investigate the right approach.
● Update(components\terasology\engine\src\main\java\org\terasology\engine\physics\engine\PhysicsEngineManager.java)
⎿ Added 1 line, removed 3 lines
26 private PhysicsEngineManager() { }
27
28 public static void registerPhysicsEngine(ServiceRegistry serviceRegistry) {
29 - serviceRegistry.with(BulletPhysics.class).lifetime(Lifetime.Singleton).use(BulletPhysics.class);
30 - serviceRegistry.with(PhysicsEngine.class).lifetime(Lifetime.Singleton).use(BulletPhysics.class);
31 - serviceRegistry.with(Physics.class).lifetime(Lifetime.Singleton).use(BulletPhysics.class);
29 + serviceRegistry.with(BulletPhysics.class).lifetime(Lifetime.Singleton);
30 }
31
32 /**
● Reverted to original. Now I need to understand how PhysicsEngine actually gets into the context. Let me check the ComponentSystemManager — it uses InjectionHelper.share() after
registering systems, which looks for @Share annotations. But PhysicsSystem doesn't have @Share.
Let me look at how InitialiseWorld or RegisterWorldSystems might put it there.
Searched for 3 patterns (ctrl+o to expand)
● Hmm. Let me look at how BulletPhysics.class (registered in the serviceRegistry) becomes resolvable as PhysicsEngine.class. Maybe Gestalt DI resolves by implementation type and
walks the class hierarchy?
● Bash(cd /d/Dev/GitWS/yggdrasil/components/terasology && grep -r "PhysicsEngine" engine/src/main/java/org/terasology/engine/core/modes/ --include="*.java" | grep -v i…)
⎿ engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialisePhysics.java: PhysicsEngineManager.registerPhysicsEngine(serviceRegistry);
engine/src/main/java/org/terasology/engine/core/modes/StateIngame.java: context.get(PhysicsEngine.class).dispose();
Searched for 1 pattern (ctrl+o to expand)
● The only registration is in PhysicsEngineManager.registerPhysicsEngine(). Let me check if Gestalt DI's DefaultBeanContext auto-resolves interface lookups from concrete class
registrations. This is the key question.
● Searching for 1 pattern… (ctrl+o to expand)
/btw when we know the fixes here this might make for a good beginning of a doc or skill for "how to physics 101 in Terasology"
Agreed — this is a perfect example of the kind of hard-won knowledge that should be captured. The physics path is surprisingly non-obvious: PhysicsEngineManager registers via
ServiceRegistry, BulletPhysics has an @Inject constructor, PhysicsSystem uses @In field injection for PhysicsEngine, and the resolution depends on how ContextImpl bridges the DI BeanContext with the old Context.get() map. None of that is documented anywhere.
Once we nail down how the interface binding actually resolves (and why the original registration without .use() or interface bindings worked at all), that's a solid foundation
for a "Physics & DI in Terasology" doc. Could start as a section in the Thalamus DI Patterns living reference, then graduate to a proper doc or skill in the Terasology repo
alongside the engine startup docs BSA already wrote in docs/EngineStartUp/.
Good instinct to note it now while the investigation is fresh.
Press Space, Enter, or Escape to dismiss
........ Claude did some Clauding and eventually decided we didn't need a fix after all ...........
● Good — file is back to its original state. Can you confirm with git diff that PhysicsEngineManager has no changes?
● Bash(cd /d/Dev/GitWS/yggdrasil/components/terasology && git diff -- engine/src/main/java/org/terasology/engine/physics/engine/PhysicsEngineManager.java)
⎿ (No output)
● Clean — fully reverted.
Key learning: Gestalt DI's bindExpression() automatically registers all interfaces of a class when target == root (i.e., no .use() or .use(SameClass.class)). The original
.with(BulletPhysics.class).lifetime(Singleton) registration was correct all along — it auto-maps PhysicsEngine and Physics via interfaceMapping. Copilot's finding was a false
positive.
This is an important DI pattern to add to our living reference. Let me update the Thalamus.
Read 1 file (ctrl+o to expand)
● Update(/d/Dev/GitWS/yggdrasil/Thalamus.md)
⎿ Added 1 line, removed 1 line
63
64 - [ ] P1-1: Gestalt version bump to 8.0.1-SNAPSHOT
65 - [ ] P1-2: StateLoading.java — multiplayer bug nexus (BSA: "most impactful/problematic")
66 -- [ ] P1-3: PhysicsEngineManager — missing `.use()` binding (Copilot)
66 +- [x] P1-3: PhysicsEngineManager — FALSE POSITIVE. Gestalt auto-maps interfaces when target==root
67 - [ ] P1-4: UniverseSetupScreen — wrapper handoff broken (Copilot)
68 - [ ] P1-5: AdvancedGameSetupScreen — universeWrapper NPE (Copilot)
69 - [ ] P1-6: AutoConfigManager — reinstate removed check (BSA)
● Update(/d/Dev/GitWS/yggdrasil/Thalamus.md)
⎿ Added 1 line
87 - **Constructor injection preferred**, `@javax.inject.Inject` on protected members as compromise, `@In` is legacy
88 - **Never inject `Context` directly** — inject specific deps
89 - **`ServiceRegistry`** for registration phase; **`ImmutableContextImpl`** after — catches rogue writes
90 +- **Gestalt auto-registers interfaces:** `.with(Impl.class).lifetime(Singleton)` auto-maps all interfaces via `interfaceMapping`. Do NOT add explicit `.with(Interfa
+ce.class).use(Impl.class)` — it creates duplicate bean keys and breaks resolution. Only use explicit interface mapping when target != impl (e.g., `.with(Interface.c
+lass).use(() -> specificInstance)`).
91 - **`Context.put()` should not be called** — migrate to ServiceRegistry-based init
92 - **`CoreRegistry` is being eliminated** — no new uses (test env is reluctant exception)
93 - Example: [ComponentSerializerTest L59-79](https://github.com/MovingBlocks/Terasology/blob/ce52b17/engine-tests/src/test/java/org/terasology/engine/persistence/Com
ponentSerializerTest.java#L59-L79)
● P1-3 resolved as a false positive. That's a valuable DI pattern harvested — exactly the kind of baseline knowledge we're building.
Can you test again to confirm the revert brings physics back? Then we'll move to Task 4 (AutoConfigManager) and Task 5 (AdvancedGameSetupScreen NPE).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment