give the runtime code assemblies, a build entry point, and fix two names
No project code is in Assembly-CSharp any more. Five assemblies: Core (no dependencies), Locomotion.Math, Skills, Game (everything else under Scripts), and StarterAssets. Game is one assembly rather than one per area, which is what the plan assumed. Measuring the dependency graph first found three cycles, all through UI — CharacterPanelView reaches into Dialogue, PlayerControlLock into Player, and UILayerBootstrap into Cinematics — plus two edges a using-scan cannot see, because NPCStandIn is in the global namespace. Assemblies cannot be circular, so splitting further means relocating those three files. That is a code-movement task, not an asmdef task, and nothing needs it yet. StarterAssets had to get an assembly of its own. It had none, so it lived in Assembly-CSharp, and an asmdef assembly cannot reference the predefined assemblies — four runtime files use it, and all four would have stopped compiling. The payoff is immediate: BootstrapTests no longer needs reflection to reach NPCStandIn and CinemachineFollowsPlayer, which is exactly why Phase 3 wanted this. Assets/Editor/PlayerBuild.cs is the headless build entry point. It reads the enabled scenes, asserts Bootstrap is scene 0 because the player opens scene 0 on launch, honours the -buildOutput the CLI forwards from -o, and calls EditorApplication.Exit(1) on anything short of Succeeded — without which Unity exits 0 on a build that produced nothing and CI goes green on it. Verified end to end: a 172MB .app in about two minutes. Two renames, both different from what the plan described. DialougueSkillComparison.cs contains a class called SkillFunctions — the filename never matched the type, so it is now SkillFunctions.cs. The two .yarn files with spaces in their names lost the spaces rather than becoming kebab-case, which would have made them inconsistent with Bartender.yarn and SC101.yarn; spaces were the actual problem. Both renames preserved their .meta GUIDs, and the yarnproject graph keys and the two character sheets were updated. EditMode 42/42, PlayMode 5/5, YarnCheck 9 files / 35 nodes, make build green. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
@@ -10,7 +10,7 @@ Companion reading: `docs/skill-system-refactor-plan.md`, `docs/candidate-system-
|
||||
|
||||
---
|
||||
|
||||
## 0. Status — Phases 0, 1, 2 and 3 are complete
|
||||
## 0. Status — Phases 0–4 are complete
|
||||
|
||||
Executed 2026-08-25. The working tree is clean and tagged `pre-restructure`.
|
||||
|
||||
@@ -19,7 +19,7 @@ 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.
|
||||
|
||||
Phases 1, 2 and 3 are also done — see §4. Only Phase 4 (code structure) and Phase 5 (replacing the NPC scaffold) remain proposal.
|
||||
Phases 1 through 4 are also done — see §4. Only Phase 5 (replacing the NPC scaffold) remains proposal.
|
||||
|
||||
### 0.1 The binary scene — RESOLVED in Phase 3
|
||||
|
||||
@@ -486,14 +486,54 @@ written `BootstrapTests` rather than eyeballing the scene.
|
||||
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
|
||||
### Phase 4 — code structure ✅ DONE (2026-08-25)
|
||||
|
||||
| # | Step | Detail | Effort |
|
||||
|---|---|---|---|
|
||||
| 4.1 | Add asmdefs per area | Camera, Player, Interaction, Dialogue, UI, NPCs, Core | M |
|
||||
| 4.2 | Add `Assets/Tests/PlayMode/` + asmdef | first test: bootstrap loads and reaches playable state | M |
|
||||
| 4.3 | Add `Assets/Editor/BuildPipeline.cs` | `-executeMethod` entry, `EditorApplication.Exit(1)` on failure (`CLAUDE.md` §3.4) | M |
|
||||
| 4.4 | Rename batch | `DialougueSkillComparison.cs` → `DialogueSkillComparison.cs`; kebab-case `.yarn` filenames. Rename **in the Unity Editor** so `.meta` GUIDs are preserved. | S |
|
||||
| # | Step | Status |
|
||||
|---|---|---|
|
||||
| 4.1 | Assemblies per area | done — **as 3 assemblies, not 7**; see below |
|
||||
| 4.2 | `Assets/Tests/PlayMode/` + asmdef | done early, in Phase 3 |
|
||||
| 4.3 | `Assets/Editor/PlayerBuild.cs` | done and verified — a real 172MB `.app` |
|
||||
| 4.4 | Rename batch | done — both renames differed from the plan; see below |
|
||||
|
||||
Validated: EditMode **42/42**, PlayMode **5/5**, YarnCheck 9 files / 35 nodes, `make build` green.
|
||||
|
||||
#### Phase 4 — what changed against the plan
|
||||
|
||||
**One-asmdef-per-area is not achievable without moving code.** The plan assumed seven assemblies
|
||||
(Camera, Player, Interaction, Dialogue, UI, NPCs, Core). Measuring the dependency graph first
|
||||
found three cycles, all through UI:
|
||||
|
||||
| Cycle | Caused by |
|
||||
|---|---|
|
||||
| UI ↔ Dialogue | `UI/Menu/CharacterPanelView.cs` → `NightclubArcadia.Dialogue` |
|
||||
| UI ↔ Player | `UI/PlayerControlLock.cs` → `NightclubArcadia.Player` |
|
||||
| UI ↔ Camera | `UI/UILayerBootstrap.cs` → `NightclubArcadia.Cinematics` |
|
||||
|
||||
There are also two edges a `using`-scan cannot see, because the types are in the global
|
||||
namespace: `Interaction` → `NPCStandIn` and `Dialogue` → `NPCStandIn`.
|
||||
|
||||
Assemblies cannot be circular, so the split landed as **Core / Game / Skills / Locomotion.Math**
|
||||
plus a new **StarterAssets** assembly. `Game` is the cyclic cluster kept whole. Splitting it
|
||||
further is a code-movement task — relocate those three files — not an asmdef task, and it is
|
||||
not worth doing until something needs it.
|
||||
|
||||
**StarterAssets had to get an assembly too.** It had none, so it was in `Assembly-CSharp`, and an
|
||||
asmdef assembly cannot reference the predefined assemblies. Four runtime files use it. Without
|
||||
giving it one, moving our code into assemblies would have broken all four.
|
||||
|
||||
**The payoff was immediate:** `BootstrapTests` no longer needs reflection. It references
|
||||
`NPCStandIn` and `CinemachineFollowsPlayer` directly, which is what Phase 3 flagged as the reason
|
||||
to do this.
|
||||
|
||||
**Both renames in 4.4 were not what the plan described.**
|
||||
|
||||
- `DialougueSkillComparison.cs` contains a class called `SkillFunctions`. The filename never
|
||||
matched the type, so the fix was `SkillFunctions.cs`, not `DialogueSkillComparison.cs`. It is
|
||||
attached to nothing in any scene and its GUID was preserved through the rename.
|
||||
- The `.yarn` files were renamed to remove **spaces**, not to kebab-case: `BradfordKane.yarn`,
|
||||
`ChevalierCassianThal.yarn`. Kebab-case would have made them inconsistent with `Bartender.yarn`,
|
||||
`Common.yarn` and `SC101.yarn`, and spaces were the actual problem. The `.yarnproject` graph
|
||||
keys and the two character sheets in `writing/` were updated to match.
|
||||
|
||||
### Phase 5 — replace the scaffold
|
||||
|
||||
|
||||
Reference in New Issue
Block a user