Cache generated building meshes per type #59

Merged

liamjd merged 1 commit from 41-cache-building-meshes into main

2026-09-02 06:46:08 +01:00

liamjd commented

2026-09-02 06:43:31 +01:00

Owner

Closes #41

What

BuildingMeshes.build() now caches generated meshes in a static Dictionary keyed on [type.id, tile_size, level_height], so every placed building of a type (and the placement ghost) shares one Mesh and one Material. build()'s signature is unchanged; the old body moved into a private _generate().

An authored type.mesh still short-circuits before the cache and is never stored, in both directions: it is not cached, and it is not shadowed by a generated mesh already cached under the same id.
Nothing invalidates on a tile_size change. The key carries the size, so a stale entry is only ever under a key nobody asks for again.
The editor bypasses the cache entirely (Engine.is_editor_hint()), which handles the "edit placeholder_color in a .tres and keep getting the old mesh" hazard. This was chosen over wiring clear_cache() into terrain regeneration, which the three-part key makes unnecessary.
clear_cache() is public for the GUT suite, wired into before_each().
The build() doc comment now states the shared, read-only contract. That was already true on the authored path.

Review note

Three existing tests built two types that both used the helper's fixed id test_type with identical sizes: the colour comparison and the two box-vs-cylinder height comparisons. With the cache, the second build is a hit on the first, so the colour test would fail and the height tests would pass vacuously. The helper now takes an optional id and those tests pass distinct ones. The reason is documented in the file header.

Verification

Full GUT suite: 141/141 passing across 8 scripts.
test_building_meshes.gd: 35 tests, 10 new, covering all four acceptance points from #41 plus the stale-until-cleared behaviour.
Not verified visually. The change is resource identity only; the ghost's tint still lives on material_override, and no caller writes to the returned mesh (checked buildings.gd, placement_controller.gd, tools/screenshot_buildings.gd).

🤖 Generated with Claude Code

https://claude.ai/code/session_01MijPibgzE26Nkjti7k2Xyt

Closes #41 ## What `BuildingMeshes.build()` now caches generated meshes in a static `Dictionary` keyed on `[type.id, tile_size, level_height]`, so every placed building of a type (and the placement ghost) shares one `Mesh` and one `Material`. `build()`'s signature is unchanged; the old body moved into a private `_generate()`. - An authored `type.mesh` still short-circuits before the cache and is never stored, in both directions: it is not cached, and it is not shadowed by a generated mesh already cached under the same id. - Nothing invalidates on a `tile_size` change. The key carries the size, so a stale entry is only ever under a key nobody asks for again. - The editor bypasses the cache entirely (`Engine.is_editor_hint()`), which handles the "edit `placeholder_color` in a `.tres` and keep getting the old mesh" hazard. This was chosen over wiring `clear_cache()` into terrain regeneration, which the three-part key makes unnecessary. - `clear_cache()` is public for the GUT suite, wired into `before_each()`. - The `build()` doc comment now states the shared, read-only contract. That was already true on the authored path. ## Review note Three existing tests built two types that both used the helper's fixed id `test_type` with identical sizes: the colour comparison and the two box-vs-cylinder height comparisons. With the cache, the second build is a hit on the first, so the colour test would fail and the height tests would pass vacuously. The helper now takes an optional `id` and those tests pass distinct ones. The reason is documented in the file header. ## Verification - Full GUT suite: 141/141 passing across 8 scripts. - `test_building_meshes.gd`: 35 tests, 10 new, covering all four acceptance points from #41 plus the stale-until-cleared behaviour. - Not verified visually. The change is resource identity only; the ghost's tint still lives on `material_override`, and no caller writes to the returned mesh (checked `buildings.gd`, `placement_controller.gd`, `tools/screenshot_buildings.gd`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MijPibgzE26Nkjti7k2Xyt

liamjd added 1 commit

2026-09-02 06:43:31 +01:00

Cache generated building meshes per type 74bff89db7

BuildingMeshes.build() now caches generated meshes in a static Dictionary
keyed on [type.id, tile_size, level_height], so every building of a type
shares one Mesh and one Material rather than getting its own. The cache is
bypassed in the editor and exposed via clear_cache() for the test suite.

An authored type.mesh still short-circuits before the cache and is never
stored. The build() doc comment now states that the returned mesh is shared
and read-only, which was already true on the authored path.

Three existing tests built two types under the same id and would have
collapsed into one cache entry; they now pass distinct ids.

Closes #41

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MijPibgzE26Nkjti7k2Xyt

liamjd merged commit 1abddba78f into main

2026-09-02 06:46:08 +01:00

liamjd referenced this pull request from a commit

2026-09-02 06:46:09 +01:00

Merge pull request 'Cache generated building meshes per type' (#59) from 41-cache-building-meshes into main