Integrate the UE container library properly: adopt for engine-owned data (ADR 0012)

[drsnuggles8]

Implements ADR 0012 — read it first; this issue is the worklist, the ADR is the reasoning.

Why

An architecture review measured the ported UE stdlib (Containers/ 22,213, Memory/ 8,390, Templates/ 5,848, Algo/ 1,268 — 37,719 lines) as serving two consumers, with std::string outnumbering FString 206:1 and ~7,000 lines unreachable, and recommended deleting it.

ADR 0012 decides the opposite. Those ratios describe a migration that stopped one file in, not a library nobody wanted. FString exists solely because TArray relocates bitwise and libstdc++'s std::string does not survive that (OloEngine/src/OloEngine/Renderer/MeshSource.h:48-52) — deleting the port removes that symptom while preserving the condition that produced it. Committing to one library removes the boundary.

Scope

Bounded by a distinction the codebase had not previously named:

• Engine-owned data — the engine allocates, owns and iterates it → adopts UE containers.
• Binding surface — its shape is dictated from outside, or a generated binding crosses it → keeps std::. Concretely: entt, yaml-cpp, sol2, Mono, ImGui, Jolt, spdlog, and any ECS component field a generated binding marshals.

The test is "does something outside the engine dictate this type?", not "is this hot?".

Read before touching anything

Violations pass silently on MSVC and abort only under libstdc++. MSVC's std::string keeps no SSO self-pointer; libstdc++'s does. Every relocation mistake in this work is green on the primary dev toolchain and red on the Linux CI / GPU runner. Local green does not imply CI green for any task below. Step 1 exists to move that to compile time — do it first.

---

Worklist

Steps 1 and 2 are independent and can run in parallel. Everything from 3 onward is a chain.

1. Mechanise the reference-stability audit

• Set CMAKE_EXPORT_COMPILE_COMMANDS on the clangcl preset (Ninja supports it natively; there is no compile_commands.json today and no .clang-tidy)
• Write a clang-query matcher for references/pointers bound to a mapped value whose lifetime crosses a mutation. clang-query and clang-tidy are already installed at C:\Program Files\LLVM\bin
• Validate the matcher against a known-safe and a known-unsafe hand-written case before trusting its output

2. Flip TIsTriviallyRelocatable to opt-in + hard error

The trait already exists (OloEngine/src/OloEngine/Templates/UnrealTypeTraits.h:703) and is already asserted by all six containers (Array.h:586, CompactSet.h:327, Deque.h:164, SparseArray.h:703, transitively Map.h / Set.h). It would not have caught any of the four cases this migration creates, because:

1. it defaults to Value = true for every type and is not recursive over members — TArray<std::string> is caught, TArray<StructContainingAString> is silently true;
2. it is OLO_STATIC_ASSERT_WARN, a [[deprecated]]-backed warning, not a gate.

• Default becomes std::is_trivially_copyable_v<T>
• Explicit true specialisations for FString (already at String.h:701 — required, it owns a heap buffer), TArray, TMap, Ref, and the other known-relocatable types
• OLO_STATIC_ASSERT_WARN becomes a hard static_assert
• Must build clean across all six containers, not just TArray
• Add opt-in specialisations until green — expect ~15–20, concentrated in Task/ and the async asset system (TArray<FCallback>, TArray<Tasks::TTask<Ref<Asset>>>, TArray<TRefCountPtr<FThread>>, TArray<LowLevelTasks::FTask>), i.e. internal plumbing rather than the data this is for

No existing TArray<T> violates the trait today (Submesh already uses FString; BoneInfluence / BoneInfo / Vertex / BVHNode / BVHTriangle are POD), so this is free now and gets more expensive with every conversion made before it.

3. Make the OloHeaderTool classifier testable — prerequisite, not optional

tools/OloHeaderTool/main.cpp is 4,717 lines of hand-rolled text parsing with zero tests; every function is static inside a main.cpp and therefore unlinkable from a test binary.

• Lift parse + classify into a linkable module: source text in, ComponentSerInfo out; main.cpp becomes the file-I/O adapter
• Add tests, including a regression test for the parser fix at main.cpp:1934 (an inline method body swallowing an intervening private: label) — found empirically, never tested

A missing emit is loud (ComponentSerializerCoverageTest.cpp:188 fails with the component named). A wrong emit is caught only if a round-trip test happens to construct that component. That gap is why this is a prerequisite.

4. Teach the classifier TArray and FString

• YAML emit path
• Binary emit path
• Recursive nested-struct path (needed for UIDropdownOption inside its component)
• Rebuild GenerateBindings and diff the generated .inl — per CLAUDE.md's diff-the-.inl discipline

5. Convert the four blocking element types to FString

These four are what unblock all fourteen component vectors without converting any component's own string field:

Type	Fields	Home
Material	m_Name (Material.h:478)	Renderer
FoliageLayer	Name, MeshPath, AlbedoPath	Terrain
DialogueChoice	Text, Condition	Dialogue
UIDropdownOption	m_Label	Scene/UI

Each currently makes its enclosing std::vector unconvertible — TArray<UIDropdownOption> is exactly the free(): invalid pointer abort documented at Containers/String.h:26-33.

6. Convert the fourteen component std::vector fields to TArray

Components.h holds 48 std::string, 14 std::vector, 15 std::unordered_*. Vectors convert; std::string fields stay std::string (five generated consumers cross them: scene YAML, binary sidecar, save-games, MCP field registry, C#/Lua).

Nine were already safe (glm::vec3 x2, UUID, AssetHandle, u8, TerrainLayerRule, OffMeshLink, Ref<Texture2D>, ParticleSystem); the other four unblock via step 5.

7. Convert the four subsystems' engine-owned data

In order, following the element types outward: Renderer → Terrain → Dialogue → Scene/UI.

• Run the step-1 matcher over each subsystem's maps before converting them

TMap / TSet are in scope but gated on the audit: std::unordered_map guarantees references to mapped values survive insertion, TMap does not — TSparseArray gives stable indices, not stable addresses (SparseArray.h:14, :731). &map[key] across an insert is fine with one and UB with the other, and unlike relocation there is no compile-time guard available. 727 unordered_map / set declarations exist in production.

8. Stop and measure

• Cost per KLOC, defects found, defects the trait and audit missed
• Decide whether to continue beyond these four subsystems — do not continue by momentum

---

Explicitly out of scope

• No container is deleted, including the unreachable ones (~5,000 lines: TSparseSet 1,805, LinkedList 784, Queue 341, Algo 1,268, MemoryView 300 + 380 test, Memory orphans ~490). Accepted cost of committing to the port's shape. Note TSparseSet stays unreachable by construction — Set.h:42 hardcodes OLO_USE_COMPACT_SET_AS_DEFAULT 1, so TSet is TCompactSet.
• No conversion at third-party seams. FString's implicit-in / explicit-out asymmetry is designed to sit there.
• Component std::string fields. Binding surface.

Related

• ADR 0012 — the decision and its five rejected alternatives
• ADR 0004 — untouched and constrains this: Memory/LockFreeList.cpp's allocator is first-touched from a scheduler worker freeing a Jolt job; not a deletion candidate either
• ADR 0009 — unaffected in target, interacts in timing. If P2996 productionises mid-migration, steps 3–4 are the ones to re-plan (see Track: C++26 reflection (P2996) toolchain availability (MSVC/clang-cl) for OloHeaderTool replacement #688)
• docs/analysis/dead-code.md — partly superseded for these four directories; its include-graph method cannot observe compiled-but-never-instantiated templates

Reversal conditions

Any one is sufficient cause to re-open (ADR 0012 section 7):

1. Step 8's measurement comes back materially worse than projected
2. The clang-query matcher cannot reach an acceptable false-negative rate → drop TMap adoption, keep the vector/string work
3. P2996 ships and makes the classifier work moot before step 4
4. A relocation hazard class emerges the trait cannot express (e.g. a type that registers this externally survives is_trivially_copyable_v and still cannot be relocated)

The section 0 ratios alone are not grounds to re-open — they are the starting condition the ADR exists to explain.

[coderabbitai]

🔗 Related PRs

#94 - Feature/ue_threading_integration [closed]
#291 - feat(renderer): PCSS contact-hardening soft shadows [merged]
#569 - feat(codegen): SceneSerializer unordered_set/unordered_map field support (#451 slice) [merged]
#690 - C++26 reflection experiment (GCC) + the dropped-field data-loss fixes it surfaced [merged]
#701 - fix(ci): preflight the build toolchain on the self-hosted runner [merged]

---

🧪 Issue enrichment is currently in open beta.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!