lennart/nightclub-arcadia
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

split the scene into Bootstrap, Systems and Level

Browse Source 
The game now starts from Bootstrap.unity, which additively loads Systems (the
dialogue runner, skills, UI layer, camera rig and the player) and then a level
(geometry, light, navmesh, reveal cameras). The level becomes the active scene so
new objects and lighting land there.

The player moved into Systems. It had been parented under Room_101 — under level
geometry — and it is persistent content, not level content. Moving it also
removes most of the cross-scene breakage on its own.

Unity nulls any serialized reference that crosses a scene boundary. Measured
rather than guessed: the pre-split scene was pulled from git and its null
references diffed against the split result. Exactly seven were lost — both NPCs'
dialogueRunner, dialogueUI and player, plus the reveal camera's tracking target.
A first naive audit reported 210, which turned out to be pre-existing Unity
defaults like Image.m_Material; the baseline diff is what separated the two.

Each of the seven now resolves at runtime. SceneServices.Resolve fills in a null
Inspector reference by searching the loaded scenes, keeping an assigned value if
there is one; CinemachineFollowsPlayer binds the reveal camera once the player
exists. Both follow the pattern already used by CameraFramingVolume and by
NPCStandIn's player lookup, rather than introducing a new one.

Assets/Tests/PlayMode is new, and it immediately paid for itself. BootstrapTests
loads Bootstrap and asserts the whole game comes up; on its first run it caught a
real bug the split had introduced. The player's NavMeshAgent lives in Systems
while the NavMesh is baked into the level, so during load the agent exists
off-mesh and ResetPath logs an error. ClickToMoveController now guards on
agent.isOnNavMesh rather than a bare null check. No EditMode test could have seen
that, because none of it has run yet.

Two of those checks reach their types by name through reflection: NPCStandIn and
the Cinematics namespace live in Assembly-CSharp, and an asmdef test assembly
cannot reference the predefined assemblies. Per-area asmdefs remove the need.

Build settings list all three scenes with Bootstrap at index 0, which
LoadSceneAsync by name requires.

EditMode 42/42, PlayMode 5/5, YarnCheck 9 files / 35 nodes.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
lennart
2026-08-25 20:50:25 +02:00
co-authored by Claude Opus 5
parent 1c1e50809a
commit 40b4547c71
24 changed files with 2545 additions and 1727 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/restructure-plan.md
+86 -56
View File
@@ -10,7 +10,7 @@ Companion reading: `docs/skill-system-refactor-plan.md`, `docs/candidate-system-
---
## 0. Status — Phases 0 and 2 are complete
## 0. Status — Phases 0, 1, 2 and 3 are complete
Executed 2026-08-25. The working tree is clean and tagged `pre-restructure`.
@@ -19,47 +19,38 @@ the UI layer, player control setup, the dialogue-vs-menu interaction fix, and th
A full pre-flight backup was taken first (`.tar.gz` of the tree excluding `Library/`, `Temp/`,
`Logs/`, and build output) plus a separate copy of the scene file.
Phase 2 (the writing pipeline) is also done — see §4. Phases 1, 3, 4 and 5 remain proposal.
Phases 1, 2 and 3 are also done — see §4. Only Phase 4 (code structure) and Phase 5 (replacing the NPC scaffold) remain proposal.
### 0.1 The binary scene — diagnosis corrected
### 0.1 The binary scene — RESOLVED in Phase 3
The original reading of this was **wrong**, and the correction matters for Phase 3.
This went through two wrong diagnoses before the real one. Recorded in full because the wrong
turns are the instructive part.
The theory was that the scene had been saved during a temporary Force Binary session and merely
needed re-saving. It isn't that. Unity **re-writes this scene as binary even now**, with the
project set to Force Text and `EditorSettings.serializationMode` reporting `ForceText` from the
API. Force Text governs conversion **at the moment the setting changes**, not on every
subsequent save — so a file that entered the project as binary stays binary indefinitely.
**First theory (wrong):** the scene had been saved during a temporary Force Binary session and
merely needed re-saving.
Four approaches were tried headlessly, all failing silently with success return codes:
**Second theory (wrong, but closer):** Force Text only converts at the moment the setting
changes, so a file that entered binary stays binary — and the fix is therefore an Editor-UI
toggle. Four headless approaches were tried and all failed silently with success return codes:
`ForceReserializeAssets` does not rewrite scenes; `MarkSceneDirty` + `SaveScene` writes nothing
in batch mode; `SaveScene` to a *new* path reproduces binary; re-assigning `serializationMode`
triggers no reserialize pass.
| Approach | Result |
|---|---|
| `AssetDatabase.ForceReserializeAssets(scenePath)` | Opens and imports the scene; does not rewrite scene files at all |
| `OpenScene` + `MarkSceneDirty` + `SaveScene(samePath)` | Returns `true`, writes nothing — `MarkSceneDirty` does not take in batch mode |
| `OpenScene` + `SaveScene(newPath)` | Writes a fresh file that is **also binary**, byte-identical in size |
| `serializationMode = Mixed` → `ForceText` | Mode changes as logged; no reserialize pass occurs |
**The actual cause:** the `NavMeshSurface` on the `Navigation` root held its baked `NavMeshData`
*embedded in the scene* rather than as an asset. `NavMeshData` prefers binary serialization, and
a single such object forces the entire scene file to binary no matter what the project setting
says. That is why every attempt to re-save it failed — the file was being written correctly each
time, in the only format its contents allowed.
**The remaining fix is Editor-UI only** and takes about thirty seconds:
Found by bisection: moving the roots one at a time into a fresh scene, thirteen produced text
files and `Navigation` produced a binary one. Extracting the data to
`Scenes/Levels/SC101_ConferenceHall/NavMesh-Navigation.asset` — which is what baking from the
NavMeshSurface inspector produces anyway; the embedded copy was the anomaly — made the scene
serialize as text. No Editor-UI step was required.
> Project Settings ▸ Editor ▸ Asset Serialization → set **Mixed**, let it apply, then set
> **Force Text** again. Changing the value in the dialog is what triggers the reserialize pass.
Then verify from the terminal — you want `%YAML 1.1`:
```bash
head -c 20 NightclubArcadia/Assets/Scenes/DialogueTest.unity
```
Worth knowing: this will reserialize **every** asset the setting touches, so expect a wide diff.
That is now safe to inspect and revert per-file, because everything is committed.
The scene itself is healthy — it opens with 14 roots (`Room_101`, `Dialogue System`,
`Skill System`, the three UI canvases, `CinemachineBrain`, `CinemachineCamera`, `Navigation`,
`CameraAnchor`, `CM_Reveal_Room101`, `RevealVolume_Room101`, `Directional Light`, `UI System`).
Its md5 was verified unchanged across every failed attempt. The cost of it staying binary is
that it is **not diffable and not mergeable** — which is an argument for bringing Phase 3's
scene split forward, since the split rebuilds these files anyway.
The scene is now `Assets/Scenes/Levels/SC101_ConferenceHall.unity`, 43KB of YAML, diffable and
mergeable, with the UnityYAMLMerge driver configured (Phase 1). The generalised lesson is
trap 1 in `CLAUDE.md`.
### 0.2 The Unity CLI — finding corrected
@@ -393,17 +384,25 @@ Effort marks are rough: **S** ≲1h, **M** a half day, **L** a day or more.
0.4 is the one open item and it does not block Phase 1 or 2.
### Phase 1 — repo hygiene (no Unity changes)
### Phase 1 — repo hygiene ✅ DONE (2026-08-25)
| # | Step | Detail | Effort |
|---|---|---|---|
| 1.1 | Add `.gitattributes` | Unity YAML merge driver for `*.unity` / `*.prefab` / `*.asset`; LFS for `Assets/Art/**` binaries; `* text=auto eol=lf` | S |
| 1.2 | Untrack `tools/YarnCheck/{bin,obj}` | `git rm -r --cached`, add ignore rules | S |
| 1.3 | Remove committed `.DS_Store` files, confirm ignore rule | S |
| 1.4 | Add a `Makefile` or `scripts/` wrapping the `CLAUDE.md` commands | `make test`, `make yarncheck`, `make build` | S |
| # | Step | Status |
|---|---|---|
| 1.1 | `.gitattributes` | done — UnityYAMLMerge driver, LF pinned, binaries marked |
| 1.2 | Untrack `tools/YarnCheck/{bin,obj}` | **no-op** — already ignored, never tracked |
| 1.3 | Remove committed `.DS_Store` | **no-op** — already ignored, never tracked |
| 1.4 | Makefile wrapping the CLAUDE.md commands | done, plus `tools/ci/report_tests.py` |
`.gitattributes` before any restructuring is deliberate — the moment work happens on a branch,
scene merges without a merge driver will corrupt files.
1.2 and 1.3 were wrong in the original plan: both were listed from a filesystem `find`, not from
`git ls-files`. Nothing was ever tracked, and `.gitignore` already covers both.
Line endings were renormalized in one deliberate pass (74 files, all CRLF→LF, `git diff -w`
empty) rather than left to surface as phantom diffs later. LFS was **not** enabled: the remote is
self-hosted and its LFS support is unverified, and turning the filter on against a server that
lacks it breaks pushing. The rules and the migration step are recorded in `.gitattributes`.
`make merge-driver` configures UnityYAMLMerge. Note the binary is at `Contents/Helpers/`, not
`Contents/Tools/` as most guides claim — there is no `Tools` directory in Unity 6 on macOS.
### Phase 2 — writing pipeline ✅ DONE (2026-08-25)
@@ -441,20 +440,51 @@ scene spec could not honestly certify its own tone field. `writing/STYLE.md` §9
sheet with four fields explicitly unset. Filling them is about half an hour of decisions and it
unblocks a check that has been stuck for a while.
### Phase 3 — scene restructure (the risky part)
### Phase 3 — scene restructure ✅ DONE (2026-08-25)
| # | Step | Detail | Effort |
|---|---|---|---|
| 3.1 | Delete template leftovers | `SampleScene.unity`, `Readme.asset`, `TutorialInfo/`; fix `EditorBuildSettings` | S |
| 3.2 | Create `Bootstrap.unity` + `Systems/Systems.unity` | Systems empty at first | M |
| 3.3 | **Copy** `DialogueTest.unity` → `Levels/SC101_ConferenceHall.unity` | Copy, do not move. Keep the original until 3.6. | S |
| 3.4 | Move systems objects out of the level scene into `Systems.unity` | dialogue runner, UI layer, skills, commentary, camera director | L |
| 3.5 | Wire additive loading in `Scripts/Core/` | Bootstrap → Systems → level | M |
| 3.6 | Verify parity, then delete `DialogueTest.unity` | full playthrough of SC-101 vs. pre-migration behaviour | M |
| 3.7 | Retarget `YarnDemoSetup.cs` or retire it | it hardcodes `Assets/Scenes/DialogueTest.unity` | S |
| # | Step | Status |
|---|---|---|
| 3.1 | Delete template leftovers | done — SampleScene, Readme.asset, TutorialInfo |
| 3.2 | Create `Bootstrap` + `Systems/Systems` | done, both text |
| 3.3 | ~~Copy~~ **rebuild** `DialogueTest` → `Levels/SC101_ConferenceHall` | done — see below |
| 3.4 | Move systems objects into `Systems.unity` | done — 10 roots incl. the player |
| 3.5 | Additive loading in `Scripts/Core/` | done — `GameBootstrap`, `SceneServices` |
| 3.6 | Verify parity, delete `DialogueTest.unity` | done — verified by diff, then deleted |
| 3.7 | Retarget `YarnDemoSetup.cs` | done — all four setup menus share `ScenePaths` |
Step 3.4 is where things break. Do it in small commits — one system per commit, playtest between
each. Do **not** batch it.
Validated: EditMode **42/42**, PlayMode **5/5**, YarnCheck 9 files / 35 nodes.
#### Phase 3 — what changed against the plan
**The binary scene is fixed, and the cause was not what §0.1 assumed.** The `NavMeshSurface` held
its baked `NavMeshData` *embedded in the scene*. That type prefers binary serialization, and one
such object forces the entire file to binary regardless of Force Text — which is why toggling the
setting, re-saving, and saving to a new path all did nothing. Bisecting the roots one at a time
found it: thirteen saved as text alone, `Navigation` did not. Extracting the data to
`Scenes/Levels/SC101_ConferenceHall/NavMesh-Navigation.asset` — what baking from the inspector
produces anyway — made the scene text. **No Editor-UI step was needed after all.**
So 3.3 became a *rebuild*, not a copy: saving the binary scene to a new path reproduces binary, so
all 14 roots were moved into a fresh scene instead. Verified roots 14→14, cross-root references
25→25, zero missing scripts, render settings carried across by hand.
**The player moved to Systems.** It had been parented under `Room_101`, i.e. under level geometry.
It is persistent content, and moving it removes most of the cross-scene breakage by itself.
**The split nulled exactly seven references**, measured by auditing the pre-split scene from git
and diffing: both NPCs' `dialogueRunner`, `dialogueUI` and `player`, plus the reveal camera's
tracking target. A first, naive audit reported 210 — those turned out to be pre-existing Unity
defaults (`Image.m_Material` and friends), which is why the before/after baseline mattered.
**A real bug surfaced, and only PlayMode could see it.** The player's `NavMeshAgent` is in Systems
while the NavMesh is baked into the level, so during load the agent exists off-mesh and
`ResetPath` logs an error. `ClickToMoveController` now guards on `agent.isOnNavMesh`. This is
exactly the class of failure an EditMode suite cannot reach, and it is the argument for having
written `BootstrapTests` rather than eyeballing the scene.
**Phase 4.2 came early.** `Assets/Tests/PlayMode` now exists. Two of its checks use reflection,
because an asmdef assembly cannot reference `Assembly-CSharp` where `NPCStandIn` and the
`Cinematics` namespace live — Phase 4.1 (per-area asmdefs) is what removes that.
### Phase 4 — code structure