feat: Terrain Quad-Tree LOD, Biome Landscape System & Biome-Aware Tree Allocation

[tcm390]

Assets sub-PR: PlayHyperia/assets#15

---

Summary

This PR introduces three major terrain features: a quad-tree LOD rendering system for large map support, a data-driven biome and landscape system for terrain generation, and a biome-aware tree placement pipeline.

---

1. Quad-Tree LOD Terrain Rendering

A hybrid quad-tree visual LOD system that replaces the flat 100m grid for rendering only — gameplay logic, physics, resource spawning, and server sync continue to use the flat grid.

Config flags (in TerrainSystem.CONFIG):

• USE_QUADTREE_LOD — enables the quad-tree LOD system (set to false to use the previous flat grid renderer, no breakage)
• QUADTREE_DEBUG_WIREFRAME — renders terrain as wireframe with depth-colored chunks: red (depth 0, largest) → orange → yellow → cyan → green (depth 4, finest)

2. Biome & Landscape Terrain System

A data-driven terrain generation pipeline where biomes and landscape features are defined declaratively.

3 biomes defined in TerrainBiomeTypes.ts via the BiomeType enum:

• Forest — default biome, lush deciduous/conifer terrain
• Canyon — arid desert terrain with cliffs
• Tundra — cold sparse terrain

2 landscape types defined in TerrainHeightParams.ts via the LandscapeType enum:

• Mountain — radial envelope × domain-warped noise → terrace quantization, producing organic tiered terrain with configurable layers, shape, edge sharpness, and noise blend
• Pond — terrain depression for water bodies using the same algorithm with inverted strength

Currently 3 example features are placed in LANDSCAPE_FEATURES[]: 2 mountains and 1 pond. To add new biomes or landscapes, add entries to TerrainBiomeTypes.ts (biomes) and TerrainHeightParams.ts (landscape features).

Terrain shader (TerrainShader.ts):

• Replaced flat biome colors with texture sampling — each biome has its own set of terrain textures
• Per-biome cliff colors and biome-aware snow caps
• Biome blending at boundaries using vertex weights

3. Biome-Aware Tree Placement

Each biome now defines its own tree distribution, density, spacing, and per-tree placement rules in TerrainBiomeTypes.ts:

4. GLBTreeBatchedInstancer — Multi-Variant Tree Rendering

A new BatchedMesh-based instancer (GLBTreeBatchedInstancer.ts) for tree types that have multiple model variants (e.g. tree_dead has 5 dead tree models, tree_cactus has 8).

Why BatchedMesh instead of InstancedMesh:

InstancedMesh binds a single geometry — to support N model variants you'd need N separate InstancedMesh objects per material slot per LOD level, multiplying draw calls. BatchedMesh registers all variant geometries via addGeometry() and each instance picks its variant via addInstance(geometryId), keeping it to 1 draw call per material slot regardless of how many variants exist.

The old GLBTreeInstancer (InstancedMesh-based) is still used for single-model resources. TreeGLBVisualStrategy routes to the correct instancer based on whether the manifest entry has modelVariants or a single modelPath.

[claude]

PR Review: Terrain Quad-Tree LOD, Biome Landscape System & Biome-Aware Tree Allocation

Nice work on this PR — the biome-aware tree placement pipeline, batched entity spawning, and tile unload cleanup are solid improvements. Here's a detailed review:

---

Code Quality & Design

Strengths:

• TreeId enum replacing string literals is a great move for type safety and refactoring confidence.
• Moving per-biome tree configs into TerrainBiomeTypes.ts is clean separation of concerns — biome data lives with biome definitions.
• The suppressBroadcast option on spawnEntity is a well-designed escape hatch for batching.
• Entity cleanup on tile unload (destroyEntity in onTerrainTileUnloaded) fixes what would have been a leak / duplicate-ID bug. Good catch.

Suggestions:

1. useHighPriorityBatch is a public mutable field (ResourceSystem.ts). It has no validation and could be toggled at any time. Consider making it private with a setter, or at minimum readonly if it's meant to be set once at construction:

   readonly useHighPriorityBatch = true;

2. Type assertions for network/entityManager (ResourceSystem.ts lines ~857, ~1014, ~1280) — the as { spawnEntity?: ...; } | null pattern is used in multiple places. Consider extracting a shared interface or using the actual system types to avoid drift between the cast shape and the real API.

3. Reference equality for distribution comparison (BiomeResourceGenerator.ts):

   if (posConfig && posConfig.distribution !== tileDist) {

   This compares object references. It works today because getTreeConfigForBiome returns the same static config objects, but it's fragile — if configs are ever cloned or generated dynamically, this silently breaks. A biome ID comparison would be more robust.

---

Potential Bugs

4. waterAffinity logic may be inverted (BiomeResourceGenerator.ts ~lines 287-291):

   if (rules.waterAffinity && rules.waterAffinity > 0) {
     const proximityLimit = rules.waterProximityHeight ?? 10;
     if (heightAboveWater > proximityLimit) {
       if (rng() < rules.waterAffinity) continue;  // skip
     }
   }

   When a tree is far from water (heightAboveWater > proximityLimit), it gets skipped with probability waterAffinity. This means with waterAffinity: 1.0, the tree never spawns far from water — that's correct. But with waterAffinity: 0.3 (coconut palm), only 30% of far-from-water positions are rejected, so 70% of coconut palms still spawn far from water. That seems like a weak affinity. Is this intentional? The naming suggests it should pull trees toward water more strongly. Consider:

   // Skip far-from-water positions with probability (1 - waterAffinity) inverted
   if (rng() > (1 - rules.waterAffinity)) continue;

   Or just document the current behavior clearly as "rejection probability when far from water."

5. Missing batch size limit — batchedEntityData has no upper bound. If a tile generates an unusually large number of resources, a single HIGH-priority packet could be very large. Consider chunking at a reasonable limit (e.g., 50–100 entities per batch packet).

---

Performance Considerations

6. HIGH priority bypasses bandwidth throttling — The PR sends all batched entity spawns at PacketPriority.HIGH, which bypasses per-connection bandwidth budgeting. During rapid exploration (player moving quickly through new tiles), multiple tiles may generate simultaneously, each flushing a HIGH-priority batch. This could spike bandwidth for clients on slower connections. Consider:

   • Using NORMAL priority but with a larger per-message budget allowance
   • Rate-limiting tile generation to max N concurrent tile spawns
   • At minimum, logging a warning if batch size exceeds a threshold

7. getDominantBiome called per candidate tree position — Inside the tree placement loop (BiomeResourceGenerator.ts), ctx.getDominantBiome(worldX, worldZ) is called for every candidate position after Poisson-disk sampling. If getDominantBiome involves noise evaluation, this could be expensive for high-density tiles. The cost depends on the implementation, but worth profiling if tile generation feels slow.

---

Security

No security concerns — the changes are server-authoritative and don't expose new client-controlled inputs.

---

Test Coverage

8. No tests included — This is a large functional change (batched entity networking, biome-aware placement rules, tile unload cleanup) with no test additions. Per the project's testing philosophy, these features should have Playwright tests that verify:
   • Entities appear correctly when entering a new biome tile
   • Entities are cleaned up when tiles unload (no entity leak)
   • Batch packets are received and processed by the client
   • Tree placement respects biome boundaries (no forest trees in canyon)

---

Minor / Nits

9. The config.ts change (line 365) is just a formatting fix — fine, but could be in a separate commit to keep the diff focused.

10. The Socket.ts whitespace changes (() => { } → () => {}) are also formatting-only — same note.

---

Overall: Strong architecture changes with clean data-driven design. The batched entity spawn is a meaningful optimization. Main action items are the potential waterAffinity logic clarification (#4), adding a batch size cap (#5), and considering bandwidth impact of HIGH-priority batches during rapid tile generation (#6). Tests would give good confidence for the networking and tile lifecycle changes.

[claude]

PR Review: Terrain Quad-Tree LOD, Biome Landscape System & Biome-Aware Tree Allocation

Overall this is a well-structured PR that cleanly separates biome-specific tree configs from the core tree type registry, introduces batched entity networking to reduce per-entity overhead, and fixes an entity leak on tile unload. The PR description is excellent. Below are findings organized by severity.

---

Potential Bugs / Correctness

1. Batch broadcast goes to ALL clients, not spatially filtered
ResourceSystem (lines ~1014-1032) uses network.sendHighPriority("entitiesBatchAdded", batchedEntityData) which calls broadcastManager.sendToAll(). This sends every tile's full resource batch to every connected client, regardless of their distance from the tile. For a large map with many concurrent players, this is both wasted bandwidth and defeats the purpose of the bandwidth budget system. Consider using sendToNearby() (which already defaults to PacketPriority.HIGH) to spatially scope the broadcast to players near the generating tile.

2. onEntitiesBatchAdded missing special-case handling from onEntityAdded
In ClientNetwork.ts, onEntityAdded has important logic for position clamping (local player), spectator target detection, and camera setup. The new onEntitiesBatchAdded handler (lines 1426-1434) skips all of this and only calls world.entities.add() + applyPendingModifications(). While resource entities (trees) won't be players, there's no type guard to prevent a non-resource entity from being included in the batch. Consider either:

• Adding a defensive check that skips entities requiring special handling, or
• Documenting clearly that this packet is resource-only, or
• Delegating to onEntityAdded per-item to preserve all side effects

3. Reference equality check for distribution objects may be fragile
In BiomeResourceGenerator.ts (line ~264):

if (posConfig && posConfig.distribution !== tileDist) {

This relies on reference equality. If getTreeConfigForBiome ever returns a copy/clone or if the biome config objects are reconstructed, this guard silently fails and redundantly re-computes treeTypes/totalWeight for every position. Not a bug today, but a subtle assumption worth a comment.

---

Performance Considerations

4. getDominantBiome called per candidate tree position in hot loop
In BiomeResourceGenerator.ts (line ~261), ctx.getDominantBiome(worldX, worldZ) is called for every candidate tree position within the tile. If getDominantBiome involves multi-octave noise sampling or other expensive computation, this could significantly increase tile generation time. Consider:

• Caching the biome result on a coarser sub-grid (e.g., 8m cells) and reusing for nearby positions
• Profiling to confirm this isn't a bottleneck in practice

5. Per-position distribution recomputation
When the position biome differs from the tile biome (lines ~264-272), Object.keys() and Object.values().reduce() are recomputed for every such position. Since there are only 3 biomes, these could be pre-computed once per biome and cached.

---

Code Quality

6. Inline type casts for network in ResourceSystem
In ResourceSystem.ts (lines ~1017-1020), the network object is cast with an inline structural type:

const network = this.world.network as {
  sendHighPriority?: (name: string, data: unknown) => void;
  send?: (name: string, data: unknown) => void;
} | null;

This pattern also appears for entityManager casts. Per the project convention of strong typing and preferring classes/interfaces, consider importing or defining a proper interface (e.g., NetworkWithBroadcast) to avoid inline structural casts that can drift from the real API.

7. useHighPriorityBatch as a public mutable field
ResourceSystem.useHighPriorityBatch = true (line ~178) is a runtime config knob exposed as a bare public property. If this is meant to be toggled, consider making it part of a config object or at minimum marking it readonly if it's not meant to change after init.

8. batchedEntityData typed as unknown[]
In ResourceSystem.ts (line ~860), batchedEntityData is unknown[] but it holds serialized entity data. Using EntityData[] (matching the client-side handler's type) would provide better type safety and documentation.

---

What Looks Good

• TreeId enum replacing magic strings is a solid improvement — centralizes tree IDs and enables refactoring safety.
• treeIdToSubType helper is clean and the enum constraint makes the replace("tree_", "") safe.
• Entity leak fix on tile unload (destroyEntity call in onTerrainTileUnloaded) correctly addresses the duplicate-ID issue on tile revisit.
• suppressBroadcast pattern in EntityManager.spawnEntity is a clean opt-in that doesn't break existing callers.
• Per-biome tree placement rules (TreePlacementRules) with water affinity, height bounds, and slope rejection are well-designed and data-driven.
• Slope rejection using central-difference gradient estimation is efficient (O(4) height samples per candidate).
• Deterministic RNG preserved throughout — reproducible across runs.
• The macOS "chromium" → "chrome" capture channel fix is a nice platform-specific edge case catch.

---

Minor / Nits

• config.ts (line 365): Whitespace-only change (indentation alignment). Fine, but could have been a separate formatting commit.
• Socket.ts: Whitespace-only cleanup (removing spaces inside empty arrow functions). Same note.

---

Summary

The biome system and tree placement architecture are well-designed and data-driven. The main concern is the broadcast-to-all for batched entities (#1 above) — this should be spatially scoped before merging to avoid unnecessary bandwidth on populated servers. The batch handler's missing special-case logic (#2) is worth addressing or explicitly documenting. The performance items (#4, #5) are worth profiling but not blocking.

[claude]

PR Review: Terrain Quad-Tree LOD, Biome Landscape System & Biome-Aware Tree Allocation

Overall this is a well-structured PR that cleanly separates biome-specific tree configuration from the core tree type registry. The batch entity spawning optimization and the per-position biome lookup for tree placement are solid improvements. Here are some specific observations:

---

Code Quality & Design

Strengths:

• The TreeId enum and TreeSpawnConfig / TreePlacementRules interfaces are a great improvement over scattered string literals and flat spawnWeight values. Moving per-biome tree configs to TerrainBiomeTypes.ts is the right separation of concerns.
• The suppressBroadcast option on spawnEntity() is a clean API addition that keeps the batching logic in the caller without polluting the entity manager.
• Test updates are thorough — every test fixture was migrated from distribution to the new trees shape.

Minor items:

• ResourceSystem.ts:860 — batchedEntityData is typed as unknown[]. Consider using EntityData[] (matching the ClientNetwork.onEntitiesBatchAdded parameter type) for stronger type safety, per the project's "NO any types" convention. unknown isn't any, but it loses the signal of what the array actually contains.
• ResourceSystem.ts:1020–1033 — The network object is cast inline with as { sendHighPriority?: ...; send?: ... } | null. This pattern appears a couple of times now. Consider whether a shared interface (e.g. NetworkBroadcaster) could reduce these ad-hoc casts across the codebase.

---

Potential Bugs & Issues

1. onEntitiesBatchAdded handler registration — The handler method is defined on ClientNetwork but I don't see explicit registration for the entitiesBatchAdded packet in this diff. If the framework auto-registers handlers by convention (on + PascalCase packet name), this should work. But if registration is manual, this handler will be silently ignored and batch packets will be dropped on the client. Worth double-checking that the wiring is in place.

2. Tile unload entity destruction — In ResourceSystem.onTerrainTileUnloaded, the new code calls entityManager.destroyEntity(resource.id) before this.resources.delete(resourceId). If destroyEntity triggers any event listeners that reference the resource map synchronously, the resource entry still exists during the event — which is probably fine but is a subtlety to be aware of. The fix comment about duplicate-ID prevention on tile revisit is good context.

3. posConfig.trees !== tileTreeMap identity check (BiomeResourceGenerator.ts:271) — This compares object references. If getTreeConfigForBiome() returns the same default config (FOREST_TREE_CONFIG) that the tile already has, this correctly skips re-computation. But if a future refactor causes getTreeConfigForBiome to return new objects (e.g. spreading/cloning), this identity check will silently break and recompute on every position. A comment noting the reference-equality assumption would help future maintainers.

---

Performance Considerations

• Per-position biome lookup in generateTrees — Calling ctx.getDominantBiome(worldX, worldZ) for every candidate tree position could be expensive depending on the biome system's implementation. The code short-circuits when the result matches the tile-level map (the identity check), which is a good optimization for positions in the tile's interior. For tiles near biome boundaries, the Object.keys() + Object.values().reduce() recomputation per-position is worth monitoring. Consider caching the per-biome config maps if profiling shows this as a hot path.

• Slope calculation (BiomeResourceGenerator.ts:252–259) — 4 height samples per tree candidate. This is standard for gradient estimation and should be fine at current densities (10–15 trees per tile). Good use of early continue to skip before the more expensive biome lookup.

• Batch entity spawning — The HIGH priority bypass is a practical solution for rapid tile generation. The useHighPriorityBatch flag is a good escape hatch. Just be mindful that if many tiles generate simultaneously, HIGH priority batches could still saturate bandwidth. The flag's existence mitigates this well.

---

Security Concerns

No security issues identified. The changes are server-authoritative (entities spawned server-side, broadcast to clients). The onEntitiesBatchAdded client handler has a defensive Array.isArray(batch) check which is good.

---

Test Coverage

• The BiomeResourceSpawning.test.ts updates properly migrate all fixture data to the new trees format.
• Missing test coverage: No tests for the new placement rules (waterAffinity, avoidsWaterBelow, minHeight, maxHeight, slope rejection). These are non-trivial filtering paths in generateTrees that would benefit from targeted unit tests to prevent regressions.
• Missing test coverage: No tests for onEntitiesBatchAdded on the client side or for the suppressBroadcast + batch send flow in ResourceSystem.
• Missing test coverage: No tests for sendHighPriority in ServerNetwork.

---

Summary

Solid PR with clean architecture. The main things to verify before merge:

1. Confirm entitiesBatchAdded handler is actually registered on the client (framework convention vs. manual wiring)
2. Consider adding tests for the new tree placement rules (waterAffinity, height constraints, slope)
3. The posConfig.trees !== tileTreeMap reference-equality check is subtle — add a comment