Cache generated building meshes per type #41

New issue

Closed

opened 2026-08-28 22:32:54 +01:00 by liamjd · 0 comments

liamjd commented

2026-08-28 22:32:54 +01:00

Owner

BuildingMeshes.build() currently generates a fresh PrimitiveMesh and a fresh
StandardMaterial3D on every call. Deferred deliberately from #36: at the time there
were no callers at all, and the cache key is the entire design decision — it depends on
how #39's ghost and the real placer actually invoke it.

Do not pick this up until #39 and a real building placer exist. The point of waiting
is to key the cache against real call sites rather than imagined ones.

Why cache

Not for generation speed — building a PrimitiveMesh is just setting properties, and the
surface arrays are built lazily by the engine. The win is resource sharing: without a
cache, twenty ore mines are twenty distinct Mesh resources and twenty distinct
materials, rather than one of each shared by twenty MeshInstance3Ds.

The ghost and the real buildings can share the same cached mesh, because tinting happens
via MeshInstance3D.material_override, which replaces the baked
PrimitiveMesh.material at the instance. Nothing should give the ghost its own copy.

Shape

static var _cache: Dictionary = {}   # [type_id, tile_size, level_height] -> Mesh
static func clear_cache() -> void

build()'s signature does not change, so this is entirely internal.

Key on all three inputs. A cache keyed on type.id alone silently returns a mesh
built for a previous tile_size — and tile_size is an @export on TerrainGrid, not a
constant. Never invalidate on a tile_size change: a stale entry under a key nobody asks
for again is harmless, a wrong-sized mesh under the right key is not.

Three things that will bite

Static vars live for the whole process, including across scene reloads and @tool
script reloads in the editor. Edit placeholder_color in a .tres and you keep getting
the old mesh until restart. Either guard with Engine.is_editor_hint() or call
clear_cache() where terrain regenerates.
GUT needs clear_cache() in before_each, or a test that mutates a BuildingType
and rebuilds gets the previous test's mesh. That alone is why clear_cache() is public
rather than private.
The returned mesh becomes shared and must be treated as read-only. Same hazard class
as BuildingFootprint.rotated() returning a copy and the shared-preset warning on
TerrainGrid.next_shape(). Note this constraint is already live on the authored path —
build() returns type.mesh by reference today — so the doc comment stating it belongs
on build() regardless of whether this issue is ever done.

Acceptance

Two build() calls with identical arguments return the same Mesh instance.
Differing tile_size or level_height return different instances, each correctly sized.
An authored type.mesh still short-circuits and is never cached.
clear_cache() empties it, and the GUT suite passes with it wired into before_each.

`BuildingMeshes.build()` currently generates a fresh `PrimitiveMesh` and a fresh `StandardMaterial3D` on every call. Deferred deliberately from #36: at the time there were no callers at all, and the cache key is the entire design decision — it depends on how #39's ghost and the real placer actually invoke it. **Do not pick this up until #39 and a real building placer exist.** The point of waiting is to key the cache against real call sites rather than imagined ones. ## Why cache Not for generation speed — building a `PrimitiveMesh` is just setting properties, and the surface arrays are built lazily by the engine. The win is **resource sharing**: without a cache, twenty ore mines are twenty distinct `Mesh` resources and twenty distinct materials, rather than one of each shared by twenty `MeshInstance3D`s. The ghost and the real buildings can share the same cached mesh, because tinting happens via `MeshInstance3D.material_override`, which replaces the baked `PrimitiveMesh.material` at the instance. Nothing should give the ghost its own copy. ## Shape ```gdscript static var _cache: Dictionary = {} # [type_id, tile_size, level_height] -> Mesh static func clear_cache() -> void ``` `build()`'s signature does not change, so this is entirely internal. Key on **all three** inputs. A cache keyed on `type.id` alone silently returns a mesh built for a previous `tile_size` — and `tile_size` is an `@export` on `TerrainGrid`, not a constant. Never invalidate on a `tile_size` change: a stale entry under a key nobody asks for again is harmless, a wrong-sized mesh under the right key is not. ## Three things that will bite - **Static vars live for the whole process**, including across scene reloads and `@tool` script reloads in the editor. Edit `placeholder_color` in a `.tres` and you keep getting the old mesh until restart. Either guard with `Engine.is_editor_hint()` or call `clear_cache()` where terrain regenerates. - **GUT needs `clear_cache()` in `before_each`**, or a test that mutates a `BuildingType` and rebuilds gets the previous test's mesh. That alone is why `clear_cache()` is public rather than private. - **The returned mesh becomes shared and must be treated as read-only.** Same hazard class as `BuildingFootprint.rotated()` returning a copy and the shared-preset warning on `TerrainGrid.next_shape()`. Note this constraint is *already live* on the authored path — `build()` returns `type.mesh` by reference today — so the doc comment stating it belongs on `build()` regardless of whether this issue is ever done. ## Acceptance - Two `build()` calls with identical arguments return the same `Mesh` instance. - Differing `tile_size` or `level_height` return different instances, each correctly sized. - An authored `type.mesh` still short-circuits and is never cached. - `clear_cache()` empties it, and the GUT suite passes with it wired into `before_each`.

liamjd added this to the Building placement milestone

2026-08-28 22:32:54 +01:00

liamjd added the

enhancement

label

2026-08-28 22:32:54 +01:00

liamjd referenced this issue

2026-08-28 22:51:07 +01:00

Building footprints, data model and generated placeholder meshes (#36) #42

liamjd referenced this issue

2026-08-29 12:41:26 +01:00

When placing a building, highlight the tiles under the cursor and show placement validity #37