Files

T

jpmschweitzerandClaude Opus 4.6 3005294c98 docs(docs): replace Commonwealth with Settled Reach across docs

Updated in-universe "Commonwealth" references to "the Settled Reach"
in decisions, architecture docs, design docs, workshop outputs,
README, and wiki. Kept all references to Hamilton's books as
inspiration/comparison in historical discussions and wiki-review
workshop rounds.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

2026-03-05 11:26:52 +01:00

40 KiB

Raw Blame History

Architecture Review Audit — The Settled Reach

Date: 2026-02-11 Reviewers: Tyre (Technical Architect, internal) + Troblum (External Technical Consultant) Scope: Full architecture review covering all 40 confirmed decisions, existing implementation, current code, and technical risk profile. Format: Independent reviews (Round 1) followed by cross-review debate (Round 2), synthesized into consensus recommendations.

Executive Summary

Both reviewers independently conclude the architecture is sound and well-chosen. The Godot 4 client + Rust/bevy_ecs server via subprocess/IPC design (D-020) correctly enforces information boundaries at the protocol level, supports deterministic simulation, and future-proofs multiplayer without incurring multiplayer tax in v0.1.

Architecture health: GREEN.

The primary concerns are not about the architecture itself but about underspecified systems (knowledge graph, observer snapshot pipeline) and operational gaps (blocking I/O, error recovery, profiling infrastructure). The reviewers diverge on urgency of some items but converge on the final priority list.

Key consensus findings:

The information boundary system (pillar 1) is the most important and least specified critical system
Blocking I/O in LocalBridge must be resolved before end-to-end integration works
Deterministic replay (#201) is non-negotiable by Sprint 2
HashMap iteration order is a determinism time bomb — enforce BTreeMap/IndexMap in simulation code
IPC round-trip latency must be measured as a Sprint 1 exit criterion
Sprint 1 is on track; the building blocks for "character moves on screen through IPC" are in place

Round 1: Tyre's Architecture Review
Round 1: Troblum's Architecture Evaluation
Round 2: Tyre's Debate Response
Round 2: Troblum's Debate Response
Consensus Recommendations

Round 1: Tyre's Architecture Review

Author: Tyre (Technical Architect)

1. Architecture Overview Assessment

1.1 Core Architecture: Godot 4 Client + Rust/bevy_ecs Server via Subprocess/IPC

The fundamental architecture is sound and well-chosen. D-020 (subprocess/IPC over GDExtension) was the right call, and Troblum's risk assessment that pushed us away from GDExtension was spot-on. The current architecture eliminates entire categories of risk that would have consumed sprints by now.

What is working well:

True client-server separation. The server/ crate has ZERO Godot dependencies. Cargo.toml confirms: bevy_ecs, bevy_app, serde, rmp-serde, standard Rust crates. No godot crate. This is exactly right.
SimBridge trait abstraction (server/src/bridge/mod.rs lines 28-34) cleanly separates transport from protocol. LocalBridge (Unix sockets) and future NetworkBridge implement the same interface. Multiplayer really would be a transport swap, not a rewrite.
MessagePack serialization with length-prefixed framing (server/src/bridge/framing.rs) — simple, correct, tested. 4-byte big-endian length prefix, 16MB max message, EOF-safe reads.
Deterministic simulation foundation. Injectable SimulationTime, SimRng (ChaCha20 with stored seed), and InputQueue resources (server/src/simulation/). D-030 requirement 7 (deterministic replay) has its plumbing in place.
Protocol codec parity. The GDScript Protocol class (client/scripts/protocol/protocol.gd) correctly handles rmp_serde's named-field encoding (unit variants as bare strings, data variants as single-element maps). Cross-language fixture files exist in client/tests/fixtures/msgpack/.
ObserverSnapshot as sole boundary type. D-010 principle 2 enforced at the API level — client never sees raw world state.

Architecture Rating: GREEN. The core split is the right one. The implementation so far is clean, well-documented, and testable independently on both sides.

1.2 Implementation Progress

Current implementation status as of Sprint 1:

Component	Status	Files
Server binary + bevy_app bootstrap	Working (single-tick)	`server/src/main.rs`
SimulationPlugin (time, rng, input, movement)	Working, tested	`server/src/simulation/`
BridgePlugin (SimBridge trait, LocalBridge)	Working, tested	`server/src/bridge/`
NPC component model (10 axes)	Stubbed with types	`server/src/npc/mod.rs`
PerceptionPlugin	Stub only	`server/src/perception/mod.rs`
StorytellerPlugin	Stub only	`server/src/storyteller/mod.rs`
CauseChain (provenance tracking)	Working, tested	`server/src/cause_chain.rs`
WalkabilityMap + tile collision	Working, tested	`server/src/simulation/movement.rs`
Godot client main loop	Working (test mode)	`client/scripts/main.gd`
SimBridge autoload (GDScript)	Working (test mode + wire protocol)	`client/scripts/autoloads/sim_bridge.gd`
Protocol codec (MessagePack)	Working, tested via fixtures	`client/scripts/protocol/protocol.gd`
EntityRenderer	Placeholder	`client/scripts/rendering/entity_renderer.gd`
FogRenderer	Stub	`client/scripts/rendering/fog_renderer.gd`

Sprint 1 goal: "get a player-controlled character moving on screen through working IPC bridge" — tickets #78/#79/#81/#82/#83. The building blocks are in place. The gap is the actual subprocess spawn and real-time loop.

1.3 Test Infrastructure

Rating: GREEN with minor gaps.

D-030 three-layer IPC testing architecture partially implemented:

Layer 1 (fixture-based serialization): Done. server/tests/serialization.rs does MessagePack roundtrip on all types. server/tests/gen_fixtures.rs generates cross-language fixtures. Client has client/tests/fixtures/msgpack/ with 5 fixture files.
Layer 2 (protocol state machine): Partially done. server/tests/bridge_ipc.rs tests snapshot and input roundtrip over real Unix sockets. Missing: mock protocol state machine for client-side testing.
Layer 3 (real subprocess integration): Not yet implemented. Sprint 1 ticket #81 addresses this.

Build pipeline via Makefile is solid: make ci chains lint -> build -> test for both sides. cargo nextest for Rust, gdUnit4 for Godot.

2. Decision Coherence Analysis

2.1 Internal Consistency

Reviewed all 40 confirmed decisions across 5 domain files. The decisions are remarkably consistent — zero hard contradictions found.

Strong coherence chains:

D-010 (multiplayer-ready baseline) -> D-020 (subprocess/IPC) -> D-030 (testability) form a clean dependency chain.
D-011 (fog of perception) -> D-015 (locked camera) -> D-017 (perception modes) -> D-018 (three-range sound) form a coherent perception stack.
D-005 (single character) -> D-027 (two-character vertical slice) -> D-034 (THE FRIEND pattern) demonstrate scope escalation that is architecturally safe.
D-023 (three-tier content) -> D-024 (10-axis NPC model) -> D-025 (social site templates) -> D-028 (tagged line pools) -> D-035 (tag taxonomy) form a complete content pipeline specification.

Mild tension points (not contradictions):

D-003 vs current scope. D-003 says "build a framework/engine, the Settled Reach is the first campaign." But the NPC axes (D-024), contraband spec (D-037), and setting details (D-036) are deeply Settled Reach-specific. Fine for v0.1 — the "framework" claim should be understood as aspirational, not architectural.
D-012 (chunk-based maps, future borderless) vs D-014 (bounded 150x150). No contradiction, but chunk-based architecture is over-engineered for v0.1 scope (150x150 = ~25 chunks at 32x32). Investment justified by design principle.
D-009 (multiplayer-ready) cost estimate ("15-20% slower"). Unverifiable at this stage. With subprocess/IPC, multiplayer readiness is essentially free because the architecture IS client-server.

2.2 Decision Gaps

Gap	Severity	Notes
Save/Load system	HIGH	bevy_ecs World serialization is non-trivial. No decision or design exists.
Pathfinding algorithm	MEDIUM	MoveIntent handles tile-by-tile validation but no A* or similar for multi-tile paths.
Shadowcasting algorithm	MEDIUM	D-011 mandates LOS shadowcasting. Implementation not specified.
NPC AI state machine	HIGH	D-026 mentions "4 state machines" for Background tier. No specification of states or transitions.
Knowledge graph data structure	HIGH	D-010 principle 2 requires state tagged with "who knows it." Actual data structure for entity knowledge is unspecified.
Map data format	MEDIUM	D-014 specifies 5-8 hand-crafted buildings. Authoring pipeline undefined.
Sound propagation algorithm	LOW	D-018 specifies three ranges. Algorithm unspecified.
Entity ID stability	MEDIUM	`VisibleEntity.entity_id` is a `u64`. How stable IDs are generated and mapped client-side needs specification.

3. Technical Risk Assessment

RISK 1: Information Boundary Implementation (CRITICAL)

The core mechanic depends on getting this right, and it is the least-specified critical system.

D-010 principle 2 says "every piece of game state is tagged with who knows it." D-024 defines InformationInventory as Vec<String> — a placeholder. For v0.1 with 15 NPCs, this might work as a prototype. For 500+ active NPCs with knowledge graphs about other NPCs, this is O(N^2) in information space.

What is needed and not yet designed:

A KnowledgeState component tracking "what does entity A know about entity B's state at time T"
Efficient observer queries computing ObserverSnapshot without iterating the entire world
Spatial indexing for perception queries
Information decay (D-011: "fog returns when you leave")
Q-016 (knowledge hierarchy) is open and load-bearing

Mitigation: Dedicated architecture spike before Sprint 2. Define knowledge data structure, observer query algorithm, and profile at 80 NPCs.

RISK 2: Observer Snapshot Bandwidth (MEDIUM)

ObserverSnapshot currently contains only tick and Vec<VisibleEntity>. Per D-020, it needs fog/visibility grid, sound events, internal monologue triggers, HUD widget data, entity relationship colors, ambient state changes.

At 10 tps with 30-80 visible entities plus fog grid plus sound events — snapshot size matters. MessagePack is compact but frame budget is ~100ms per tick.

Current risk: LOW at v0.1 scale (15 NPCs). MEDIUM at full Active tier (80 NPCs).

Mitigation: Delta compression (only send changes since last acknowledged snapshot). bevy_ecs Changed<T> provides infrastructure for dirty-flagging.

RISK 3: Synchronous IPC Blocking (HIGH)

Current LocalBridge uses blocking I/O on Unix sockets. Both send_snapshot and receive_inputs are blocking calls behind a Mutex. In a real game loop:

If client is slow to read, send_snapshot blocks the server tick.
If client is slow to send input, receive_inputs blocks until data arrives or EOF.

Mitigation: Sprint 1 must address this. Recommended: separate IO thread with crossbeam channels feeding into the bevy_ecs game loop. The SimBridge trait already abstracts this — the fix is internal to LocalBridge.

RISK 4: Client-Side Entity Lifecycle Management (MEDIUM)

Current issues in entity_renderer.gd:

Checks for entity_data.has("id") (line 19) but Protocol returns entity_id, not id. Will silently skip all entities.
Entity creation uses ColorRect — fine for prototyping.
No interpolation between ticks. At 10 tps, entities teleport between tiles.
No entity pooling.

RISK 5: Bevy Version Churn (LOW-MEDIUM)

Cargo.toml shows bevy_ecs = "0.18", bevy_app = "0.18". Version pinning is correct. Risk is over project lifetime.

RISK 6: GDScript MessagePack Library Dependency (LOW-MEDIUM)

Fixture-based cross-language tests (D-030 Layer 1) are the correct mitigation.

RISK 7: Kill Switch Timeline (META)

D-020 specifies: "If no working prototype by week 8, pivot to pure Godot." Current trajectory is healthy — Sprint 1 is on track. Week 8 deadline should be tracked explicitly.

4. Scalability and Performance Assessment

4.1 Simulation Tier Budget Feasibility

D-026 defines four tiers:

Tier	Count	Update Rate	Per-Entity Cost
Active	30-80	10-20 ticks/sec, full sim	HIGH
Background	500-2,000	1/game-minute (10 ticks)	LOW
State-saved	10,000+	Frozen, ~1-2KB serialized	ZERO
Ungenerated	unlimited	N/A	ZERO

At 80 Active NPCs, 10 tps: 80 perception queries per tick at ~1ms each = ~80ms. Exceeds 100ms tick budget with naive implementation. But: not all 80 NPCs need full LOS every tick. Spatial hashing and staggered updates bring this within budget.

For v0.1 with 15 NPCs, performance is a non-issue. Scaling concern is real for the full game but achievable with spatial indexing and staggered perception.

4.2 Asymmetric Information at Scale

The InformationInventory { known_facts: Vec<String> } placeholder needs to become a structured, queryable data store. Recommended: HashMap<Entity, KnowledgeEntry> per entity, with spatial queries using grid-based visibility cache.

5. Implementation Readiness Assessment

5.1 Well-Specified (Ready to Build)

System	Quality	Key Decisions
IPC transport (bridge)	Excellent	D-020, D-030
Time system	Excellent	D-031
NPC component model	Good	D-024
Content tag taxonomy	Excellent	D-035
Tile collision	Good	D-012
Test architecture	Good	D-030
Build pipeline	Good	DEVOPS.md

5.2 Underspecified (Needs Architecture Spike)

System	Priority
Knowledge graph / information boundaries	CRITICAL — Sprint 2 blocker
Observer snapshot generation	CRITICAL — Sprint 2 blocker
LOS shadowcasting	HIGH — Sprint 2
NPC AI / state machines	HIGH — Sprint 3
Pathfinding	HIGH — Sprint 3
Save/Load	MEDIUM — before vertical slice
Map data authoring	MEDIUM — Sprint 2-3
Entity ID stability	MEDIUM — Sprint 1 blocker
Sound propagation	LOW — v0.1 visual only
Storyteller design	LOW for v0.1, HIGH for vertical slice

6. Tyre's Prioritized Recommendations

Tier 1: CRITICAL (Before Sprint 2)

R-01: Design Knowledge Graph data structure (replace Vec<String> with structured KnowledgeGraph component)
R-02: Design Observer Snapshot generation pipeline (spatial query -> LOS check -> sound propagation -> knowledge filtering -> assembly)
R-03: Fix blocking I/O in LocalBridge (separate IO thread with crossbeam channels)

Tier 2: HIGH (Sprint 2-3)

R-04: Choose and implement shadowcasting algorithm (symmetric or recursive)
R-05: Define NPC pathfinding strategy (A* on WalkabilityMap)
R-06: Specify NPC AI state machines
R-07: Fix entity_renderer.gd field name mismatch (id vs entity_id)

Tier 3: MEDIUM (Before Vertical Slice)

R-08: Design Save/Load architecture (custom SaveState struct)
R-09: Define map authoring pipeline (Tiled recommended)
R-10: Add delta compression interface to ObserverSnapshot

Tier 4: LOW (Track)

R-11: Resolve Q-007 (target platforms — Unix sockets vs Windows)
R-12: Document entity ID strategy
R-13: Add performance benchmarks

7. Strengths Worth Preserving

Subprocess/IPC over GDExtension (D-020). Every day without fighting gdext FFI validates this choice.
Pure Rust server with zero Godot dependencies. Testable without Godot. Integration tests run in CI without display server.
bevy_ecs for the simulation. Component model maps naturally to NPC axes. Change detection essential for efficient observer queries.
The test architecture (D-030). Three-layer IPC testing, cross-language fixtures, CauseChain as production component.
Kill switch criteria (D-020). Honest, measurable, time-boxed.
Single-ownership NPC model with reference links (D-025). Avoids lifecycle management nightmare.

8. Tyre's Summary

Overall Architecture Health: GOOD. Core decisions sound, implementation clean, risk profile well-understood.

Primary concern: Information boundary system is the most important and least specified part.

Secondary concern: Sprint 1 IPC loop needs blocking I/O resolved.

Trajectory: Week 8 kill switch comfortably far away. Current pace healthy. Biggest risk is not technical failure but scope creep in content design.

Round 1: Troblum's Architecture Evaluation

Author: Troblum (External Technical Consultant)

1. Technology Stack Assessment

1.1 The Subprocess/IPC Choice

For the stated requirements (asymmetric information, occlusion-based detective game, Rimworld-style simulation, multiplayer-ready), the Godot 4 + Rust/bevy_ecs subprocess/IPC architecture is well-suited.

The shift from GDExtension to subprocess/IPC eliminated the highest-severity risks. Good decision.

Comparison to alternatives:

Stack	Pros	Cons	Verdict
Current (Godot + Rust/IPC)	Info boundaries enforced, deterministic sim, multiplayer-ready	IPC latency, two-language debugging	Best fit
Pure Godot (GDScript)	Faster prototyping, single language	Performance ceiling ~100 NPCs, harder to retrofit multiplayer	Viable for v0.1 only
Pure Bevy	Everything in Rust, no IPC, excellent ECS	No visual editor, immature UI, steeper learning curve	Wrong for developer profile
GDExtension (R-006)	No IPC latency, tighter Godot integration	FFI complexity, thread safety, gdext pre-1.0 instability	Correctly rejected

2. IPC & Integration Risk

2.1 Unix Socket Architecture

SimBridge trait -> LocalBridge (Unix domain sockets) -> NetworkBridge (future TCP). Architecture is clean.

Framing protocol: 4-byte big-endian length prefix + MessagePack payload. Maximum 16MB. EOF detection. This is solid.

What can go wrong:

Blocking I/O — current implementation blocks on read/write
Version drift — no protocol handshake
Flow control — no backpressure mechanism
Error recovery — undefined

2.2 Serialization Assessment

MessagePack is a good choice: compact binary, schema-optional, cross-language. The fixture-based testing (D-030 Layer 1) catches encoding divergence early.

Performance concern: GDScript MessagePack library maturity and performance at scale needs monitoring.

3. bevy_ecs Suitability

3.1 Entity-Component Fit

The 10-axis NPC model (D-024) maps naturally to bevy_ecs components. Each axis = one component. Systems operate on component queries. Standard ECS pattern.

Critical gap: Spatial partitioning. Perception queries without spatial indexing are O(observers x entities). At 80 Active NPCs, this exceeds tick budget.

Recommendation: Implement grid-based spatial partitioning (~200 lines of Rust) before perception systems exist.

3.2 Determinism Support

Systems using .after() for explicit ordering. movement::validate_movement runs after time::advance_tick. Deterministic.

Gap: Only two systems registered so far. When 10 systems are added with cross-dependencies, will ordering be maintained?

Recommendation: Create a system dependency graph before Sprint 2. Document which systems read/write which components.

3.3 Storyteller System

Architecturally straightforward in bevy_ecs — just another system. But it needs access to almost everything and is likely the most complex single system. Needs early prototyping.

3.4 Chunk-Based Maps

At 150x150 map with 15x15 tile chunks: 100 chunks, 9 loaded at once, ~14,000 tiles in memory. Negligible. Future borderless supported by same architecture.

4. Godot 4 Client Concerns

4.1 GDScript Performance

GDScript is adequate for the client's role (deserializing snapshots, updating sprites, UI, audio). All heavy computation (LOS, pathfinding, AI) is on the Rust side. Correct separation.

Concern: D-017 dynamic HUD composition requires runtime UI creation. Profile HUD rebuild; if >16ms, pre-instantiate widgets and show/hide instead of create/destroy.

4.2 Fog-of-War Rendering

Options: TileMapLayer (simplest), shader-based (most performant), LightOccluder2D (visually impressive but complex).

Recommendation: Start with TileMapLayer for v0.1. Upgrade to shader if profiling shows tile updates are slow.

4.3 Entity Color (D-033)

CanvasItem.modulate multiplies existing pixel colors. A red sprite modulated green looks brown, not green.

Recommendation: Base sprites in white/grayscale, apply relationship color via modulate.

4.4 Audio

Godot's audio system is excellent for 8 audio files with event-driven + ambient loops. No concerns.

5. Architectural Patterns & Anti-patterns

5.1 Patterns Working Well

Client-server separation (D-010, D-020)
Observer-query model (D-017)
Resource-based injection (SimRng, SimulationTime, InputQueue)
Chunk-based maps (D-012)
Three-tier content model (D-023)

5.2 Patterns That Are Concerning

No spatial partitioning — critical gap for scaling
Error handling incomplete — no recovery strategy defined
Flow control undefined — pipe buffer overflow possible at scale
No protocol versioning — version drift will cause subtle bugs
Determinism designed but untested — #201 not implemented

5.3 Missing Patterns

Save/load architecture — not addressed
Mod support (Q-004) — not addressed
Crash reporting / telemetry — gap for external playtesting
Performance monitoring — no cross-boundary profiling

5.4 Anti-patterns Correctly Avoided

NOT using GDExtension for game logic
NOT storing game state in Godot scene tree
NOT baking player identity into simulation
NOT using wall-clock time in simulation

6. Scope vs Architecture Fit

6.1 v0.1 Scope Assessment

Architecture is adequate for v0.1 scope. Possibly over-engineered (15 NPCs don't need ECS or client-server), but over-engineering pays off in future-proofing.

6.2 Scaling Limits

The 500-2,000 NPC target is achievable IF:

Spatial partitioning is implemented
Tier transitions work as designed
Perception queries are optimized

6.3 Scope Cut Priority (If Needed)

First cut: Background tier size (500-2,000 -> 100-500)
Second cut: Perception mode count (6 -> 3)
Third cut: Multi-character vertical slice (drop detective)
Fourth cut: Chunk-based procedural maps (hand-craft everything)
Nuclear option: Pure Godot, abandon Rust/ECS

6.4 Additional Kill Switch Criteria (Proposed)

Perception queries exceed 50ms per tick at 30 NPCs
IPC latency exceeds 10ms per tick

7. Troblum's Recommendations

CRITICAL (Do Now)

Implement spatial partitioning this sprint
Add protocol version handshake (~50 lines)
Define error recovery strategy
Profile the full IPC loop now

HIGH (Sprint 2-3)

Implement deterministic replay (#201) — non-negotiable by Sprint 2
Build minimal storyteller prototype
Stress-test tier transitions (500 dummy NPCs)
Document system dependency graph

MEDIUM (Sprint 4-5)

Add performance telemetry
Plan save/load architecture
Prototype one non-natural perception mode

LOW (Post-v0.1)

Investigate GDScript MessagePack performance
Add crash reporting

8. Time Bombs

The determinism assumption. Determinism is fragile. A single non-deterministic operation breaks it:

HashMap iteration order (use BTreeMap or IndexMap)
Parallel system execution without explicit ordering
Floating-point non-associativity
External state reads

Ticket #201 (deterministic replay) is non-negotiable. Implement by end of Sprint 2.

9. Troblum's Verdict

Yes, this is the right stack. For the stated requirements, the architecture is well-suited. The shift from GDExtension to subprocess/IPC eliminated the highest-severity risks.

The biggest risk is not the architecture — it's execution. The risk is spending 8 weeks building infrastructure with zero visible gameplay. D-020's kill switch is the right safety valve.

Path to success: Each sprint delivers visible gameplay. Infrastructure in service of gameplay, not the other way around.

Round 2: Tyre's Debate Response

In response to: Troblum's Round 1 findings

1. Spatial Partitioning

AGREE on the need. PUSH BACK on timing.

At v0.1 scale (1 player observer, ~15-25 NPCs), perception is 25 checks per tick — microseconds on modern hardware. The O(N^2) alarm is accurate for scaling but irrelevant at current scope.

Where Troblum IS right: writing for entity in all_entities in perception is an anti-pattern that becomes load-bearing technical debt.

Where I push back: Sprint 1 has two server tickets (#78, #236). Adding spatial partitioning scope-creeps the sprint.

Synthesis: Define the SpatialIndex trait now, use naive Vec<Entity> behind it, swap in grid-based lookup in Sprint 2.

pub trait SpatialIndex {
    fn entities_in_range(&self, center: IVec2, range: i32) -> Vec<Entity>;
    fn entities_at(&self, position: IVec2) -> Vec<Entity>;
    fn update(&mut self, entity: Entity, old_pos: IVec2, new_pos: IVec2);
}

Priority: Define trait Sprint 1. Implement grid Sprint 2.

2. Protocol Version Handshake

AGREE. Fully.

15 minutes of work, eliminates an entire class of debugging pain. I did not call this out. Troblum caught it. He is right.

Priority: Sprint 1, alongside #78.

3. Error Recovery Strategy

AGREE on the gap. PUSH BACK on urgency.

For v0.1 single-player subprocess, the failure modes are:

Deserialization failure: Development-time bug. Log, dump bytes, show "Internal error."
Server crash: Godot gets SIGCHLD. Show "Simulation crashed," offer restart.
Client disconnect: In subprocess mode, the pipe broke = child died.

Recovery (reconnection, state reload) is multiplayer infrastructure. Defer to post-v0.1.

Synthesis: Define minimal error handling in Sprint 1. Not recovery — handling.

4. Full IPC Loop Profiling

AGREE. Strongly.

D-020 says "1-5ms serialization latency per tick, acceptable." That was my estimate. Estimates are not measurements.

If the loop takes 1ms, golden. If 20ms, problem at 10 tps. If GDScript MessagePack is the bottleneck, we need to know NOW.

Synthesis: Sprint 1 exit criterion. IPC round-trip measured. Measure:

rmp_serde::to_vec() on representative ObserverSnapshot (15 entities)
pipe.write() latency
GDScript StreamPeer.get_data() + MessagePack decode
Scene tree update (create/move 15 sprites)
Total tick-to-render latency

If total exceeds 5ms, flag as risk before Sprint 2.

5. Deterministic Replay by Sprint 2

AGREE. Reinforces D-030 Decision 7.

Troblum's framing is sharper: "determinism is a claim you are making, and claims require proof." A deterministic replay system IS the proof.

Synthesis: Sprint 2 hard requirement. Record (tick_number, input) pairs. Replay with same seed + inputs. Compare entity positions. ~100 lines of Rust.

6. HashMap Iteration Order

AGREE. Genuine time bomb.

Rust's HashMap uses RandomState by default — randomized hasher, different iteration orders across runs. If ANY system iterates a HashMap and order affects outcomes, the simulation is non-deterministic even with the same seed.

I did not flag this. Troblum caught it. Exactly the kind of detail that matters for D-010 principle 4.

Synthesis: Coding standard from Sprint 1. No HashMap in gameplay-affecting code. Use BTreeMap (deterministic, sorted) or IndexMap (deterministic, insertion order). Add #[deny(clippy::disallowed_types)] for std::collections::HashMap in the simulation crate.

7. Flow Control / Backpressure

PUSH BACK. Not a real risk at v0.1 scale.

An ObserverSnapshot with 15 entities: ~3.5KB. At 10 tps = 35KB/s. Pipe buffer is 64KB. Client needs to stall for ~2 seconds before pipe blocks.

At D-026 scale (80 NPCs, ~20KB snapshots, 20 tps = 400KB/s) — pipe fills in 160ms. THAT is a backpressure scenario, but not v0.1.

Synthesis: Monitor pipe write latency in Sprint 1 profiling. Implement only if measured as problem.

8. System Dependency Graph

AGREE. But let the code generate it.

Hand-maintained dependency graph will be stale by Sprint 2. bevy_ecs already knows the dependency graph.

Synthesis: Write a print_system_graph debug command that dumps bevy_ecs schedule ordering. Run in CI. Diff across commits.

9-10. Tier Transition Stress Test + Archetype Fragmentation

PUSH BACK. Premature.

D-026 tier transitions are not implemented yet. The simulation does not have NPCs yet, let alone 500 of them. Stress-testing a system that does not exist is speculative benchmarking.

Troblum flagged archetype fragmentation as LOW risk in his own original evaluation. He is now contradicting his earlier assessment. At v0.1 scale (25 NPCs, 3 tiers), fragmentation produces at most 3 archetypes with 5-25 entities each.

Synthesis: Define performance budget now. Benchmark when code exists.

Troblum's Unique Additions Accepted

Finding	Verdict
TileMapLayer for fog rendering	Accept — debuggable, simplest for v0.1
Sprite modulate for entity color	Accept — grayscale base sprites convention
Web export (Q-007)	Already flagged. No new action.
Crash reporting	Accept as post-v0.1 concern
Performance monitoring (`tracing`)	Accept as Sprint 1 infrastructure

Where Troblum Under-Indexes

The content architecture. His review focuses on infrastructure robustness but does not engage with the content systems (monologue pipeline, CauseChain, observation events). These are where architecture meets gameplay — and where the project succeeds or fails.

Round 2: Troblum's Debate Response

In response to: Tyre's Round 1 findings

1. Knowledge Graph — CRITICAL Agreement + Push Forward

AGREE with Tyre's assessment. PUSH BACK that he didn't specify the structure.

Tyre correctly identifies InformationInventory { known_facts: Vec<String> } as a placeholder and rates it CRITICAL. But he doesn't propose a replacement.

What's needed:

#[derive(Component)]
pub struct KnowledgeGraph {
    pub entities: HashMap<Entity, EntityKnowledge>,
}

pub struct EntityKnowledge {
    pub last_known_position: Option<IVec2>,
    pub last_known_tick: u64,
    pub confidence: KnowledgeConfidence,
    pub relationship_state: Option<RelationshipSnapshot>,
    pub known_facts: HashSet<FactId>,
    pub source: KnowledgeSource,
}

pub enum KnowledgeConfidence {
    Certain,   // Directly observed
    Probable,  // Inferred
    Rumor,     // Told by someone
    Outdated,  // Haven't seen recently
}

This is pillar-1 foundation. Everything depends on it — observation, monologue, dialogue, success criteria.

2. Observer Snapshot Pipeline — CRITICAL Agreement + Algorithm Proposal

AGREE. But Tyre flags CRITICAL without proposing the algorithm.

fn generate_observer_snapshot(
    world: &World,
    observer: Entity,
    knowledge_graph: &KnowledgeGraph,
) -> ObserverSnapshot {
    // 1. Spatial query — what's nearby?
    let nearby = spatial_query(observer.position, MAX_RENDER_DISTANCE);

    // 2. Line-of-sight — what can observer SEE?
    let visible = nearby.filter(|e| line_of_sight(observer, e));

    // 3. Sound propagation — what can observer HEAR?
    let audible = sound_events.filter(|e| sound_reaches(observer, e));

    // 4. Knowledge filtering — what is observer ALLOWED to know?
    let filtered = visible.map(|e| filter_by_knowledge(observer, e, knowledge_graph));

    // 5. Snapshot assembly
    ObserverSnapshot { ... }
}

Each step has edge cases: spatial query data structure, shadowcasting integration, three-range model, knowledge filtering depends on KnowledgeGraph.

3. Blocking I/O — Upgrade to CRITICAL

Tyre rates HIGH. I rate CRITICAL.

Tyre says "will deadlock." My diagnosis: won't deadlock, will STUTTER. Stutter is worse — the game "works" but feels terrible.

If simulation tick takes 50-100ms and client calls bridge.tick() synchronously, the entire Godot render thread blocks. At 60 FPS (16ms budget), a 50ms block = 3-4 dropped frames every tick.

Recommended fix: Threaded bridge with crossbeam channels.

impl SimBridge for LocalBridge {
    fn tick(&mut self, input: PlayerInput) -> Option<ObserverSnapshot> {
        self.input_tx.send(input);
        self.snapshot_rx.try_recv().ok()  // Non-blocking, 1-frame delay acceptable
    }
}

4. Save/Load — Versioning Is Non-Negotiable

AGREE with Tyre's custom SaveState approach over bevy_reflect.

Push back: SaveState is a third data model alongside ECS + ObserverSnapshot. Every NPC component change requires updating 3 places. Real maintenance burden.

Addition: Save versioning with migration strategy. Players will complain if saves break.

5. Missing Operational Concerns

Tyre's review finds missing game systems. He didn't look for missing operational systems.

5.1 Error Handling Strategy

What happens when things go wrong?

Simulation crashes -> ?
MessagePack deserialization fails -> ?
Pathfinding exceeds budget -> ?
NPC enters invalid state -> ?

Recommendation:

Unrecoverable (crash): show error, emergency save, exit
Recoverable (bad data): log warning, skip frame, continue
Logic errors (invalid state): log error, reset to safe state

5.2 Profiling Infrastructure

Tyre checks budgets but doesn't specify HOW we measure.

pub struct TickProfiler {
    pub perception_time: Duration,
    pub pathfinding_time: Duration,
    pub ai_time: Duration,
    pub snapshot_generation_time: Duration,
    pub total_time: Duration,
}

Without this, we can't validate D-026 in practice.

5.3 Debug Visualization

When LOS is wrong or information leaks, how do we debug? Debug overlay in Godot showing pathfinding waypoints, LOS rays, vision cones, information boundary tags, tick timing. Development tool, not player-facing.

5.4 Timing Assertions in Integration Tests

#[test]
fn tick_completes_within_budget() {
    let mut sim = SimulationBuilder::new()
        .with_test_map(150, 150)
        .with_npcs(80)
        .build();
    let start = Instant::now();
    sim.tick();
    assert!(start.elapsed() < Duration::from_millis(100));
}

6. Risk Re-Assessment Comparison

Risk	Tyre	Troblum	Rationale
Information Boundaries	CRITICAL	CRITICAL	Full agreement
Observer Snapshot Bandwidth	MEDIUM	MEDIUM	Full agreement
Synchronous IPC Blocking	HIGH	CRITICAL	Blocking I/O makes game unplayable
Error Handling Gaps	Not rated	HIGH	Missing from Tyre's analysis
Profiling Infrastructure	Not rated	MEDIUM-HIGH	Can't validate budgets without measurement

Tyre under-weighted operational risks. Focused on "what's missing" (correct) but not "how do we measure/debug/recover" (also needed).

7. Troblum's Verdict on Tyre's Review

GOOD. Core decisions sound. Gap analysis thorough. Testability defensible. Risk profile well-understood.

But: Operational infrastructure under-specified. Blocking I/O flagged but not elevated enough. KnowledgeGraph and observer pipeline called out but not designed.

Consensus Recommendations

Merged priority list both reviewers would sign off on.

Sprint 1 (Current)

#	Recommendation	Source	Effort
1	Protocol version handshake in IPC connection	Troblum	XS (15 min)
2	Define `SpatialIndex` trait (naive impl behind it)	Troblum + Tyre synthesis	XS (30 min)
3	Basic error handling: log + dump + user message	Troblum, scoped by Tyre	S (1-2 hrs)
4	IPC round-trip timing as Sprint 1 exit criterion	Troblum, endorsed by Tyre	S (2-3 hrs)
5	`HashMap` ban in simulation crate (clippy rule)	Troblum	XS (15 min)
6	`tracing` crate setup for server instrumentation	Troblum	S (1 hr)
7	White/grayscale base sprites + modulate convention	Troblum	Convention only
8	TileMapLayer for fog rendering in v0.1	Troblum	Convention only
9	Fix entity_renderer.gd `id` vs `entity_id` bug	Tyre	XS (5 min)

Sprint 2

#	Recommendation	Source	Effort
10	Grid-based `SpatialIndex` implementation	Troblum + Tyre	M (3-5 days)
11	Deterministic replay: record/replay test (#201)	Both — non-negotiable	M (3-5 days)
12	KnowledgeGraph data model design spike	Both — CRITICAL	M (3-5 days)
13	Observer snapshot generation pipeline design	Both — CRITICAL	M (3-5 days)
14	Async/threaded LocalBridge implementation	Both — HIGH/CRITICAL	M (3-5 days)
15	System dependency graph debug command	Troblum, synthesized by Tyre	S (1-2 days)
16	Tier transition performance budgets defined	Troblum, scoped by Tyre	XS (document)
17	Tick budget overflow policy: slow real-time, don't skip ticks	Troblum	XS (document)

Sprint 3+

#	Recommendation	Source	Effort
18	Backpressure monitoring (only if measured as issue)	Troblum	S if needed
19	Tier transition benchmarks (when code exists)	Troblum	M
20	Crash reporting infrastructure (before external playtest)	Troblum	M
21	Debug visualization overlay	Troblum	M
22	Timing assertions in integration tests	Troblum	S
23	Save/Load architecture design (with versioning)	Both	L
24	Map authoring pipeline (Tiled recommended)	Tyre	M

Not Accepted

Recommendation	Reason
Spatial partitioning implementation THIS sprint	Scope creep. Trait now, impl Sprint 2.
Full error recovery (reconnect, state reload)	Multiplayer concern. v0.1 is subprocess.
500-NPC stress test now	System does not exist yet.
Backpressure implementation now	Math does not support urgency at v0.1 scale.

Final Assessment

Architecture: GREEN. The core decisions are sound. Subprocess/IPC was the right call. The information boundary model is architecturally correct. Deterministic simulation infrastructure is in place.

Implementation plan: AMBER. Critical systems (knowledge graph, observer pipeline) are underspecified. Operational infrastructure (profiling, error handling, debug tooling) has gaps. Blocking I/O needs resolution before end-to-end integration.

Trajectory: ON TRACK. Sprint 1 building blocks are in place. Week 8 kill switch is comfortably distant. The strongest signal from both reviews: measure the IPC loop in Sprint 1, enforce determinism from Sprint 2, and never let infrastructure work orphan from a visible gameplay outcome.

Troblum catches the bugs. Tyre designs the systems. The synthesis is stronger than either review alone.

Files Reviewed

Server (Rust)

server/Cargo.toml
server/src/main.rs
server/src/lib.rs
server/src/bridge/mod.rs
server/src/bridge/framing.rs
server/src/bridge/local.rs
server/src/bridge/types.rs
server/src/simulation/mod.rs
server/src/simulation/input.rs
server/src/simulation/rng.rs
server/src/simulation/tier.rs
server/src/simulation/time.rs
server/src/simulation/movement.rs
server/src/npc/mod.rs
server/src/perception/mod.rs
server/src/storyteller/mod.rs
server/src/cause_chain.rs
server/tests/smoke.rs
server/tests/bridge_ipc.rs
server/tests/movement.rs
server/tests/serialization.rs
server/tests/gen_fixtures.rs

Client (GDScript)

client/scripts/main.gd
client/scripts/autoloads/sim_bridge.gd
client/scripts/autoloads/game_state.gd
client/scripts/autoloads/input_mapper.gd
client/scripts/protocol/protocol.gd
client/scripts/rendering/entity_renderer.gd
client/scripts/rendering/fog_renderer.gd
client/scripts/rendering/world_renderer.gd
client/scenes/main.tscn

Architecture & Decisions

decisions/architecture.md
decisions/perception.md
decisions/scope.md
decisions/content.md
decisions/process.md
decisions/questions.md
decisions/rejected.md
docs/architecture/eval-godot-rust-bridge.md
docs/architecture/risk-godot-rust-bridge.md
docs/sprints/sprint-1/server.md
docs/sprints/sprint-1/client.md
Makefile

40 KiB Raw Blame History