replace the NPC scaffold with data-driven prefabs, and retire the builders

NPCStandIn was written to be dropped on a sphere and carried its identity in
per-instance public fields. The cost of that showed up in the scene itself: of
the two spheres, one was the Bartender and the other was named "Bartender (1)"
in the hierarchy while being configured as the Chair — data changed, name never
was. Nothing outside the scene could see what existed, and two copies could
disagree.

Identity now lives in an InteractableDefinition asset and the scene only places
a prefab. The migration matched the old objects by yarn node rather than by
name, which is the only reason the chair ended up as the chair.

InteractableDefinition rather than the planned "character definition": it covers
a character and a talkable object equally, so Bartender.prefab sits under
Prefabs/Characters and Chair.prefab under Prefabs/Interactables.

Three components collapse into one. DialogueInteractable was in no scene at all
— a dead trigger-zone variant of the same idea. ClickInteractableBridge existed
only to adapt NPCStandIn without modifying it, a constraint that died with
NPCStandIn; DialogueInteractor implements IClickInteractable itself. The parts
worth keeping were lifted out rather than discarded: WorldSpaceLabel and
InteractionPrompt are now normal components on prefab children, so a label can
be seen and positioned in the Editor instead of existing only at runtime.
InputDeviceTracker moved into NightclubArcadia.Core and gained a namespace,
leaving no global-namespace runtime types.

Characters get the same pipeline as Skills and Candidates:
Assets/Characters/characters.json is the source of truth, CharacterSetup
regenerates the definition assets on script reload and removes orphans. Adding
an NPC is a JSON entry plus a prefab — no code, and no Unity for the data half.

Phase 5.4 deletes the one-shot builders rather than disabling them. UILayerSetup,
PlayerControlSetup, UILayerSetupAutoRun and YarnDemoSetup assumed the
single-scene layout, had already destroyed a level scene and turned Systems
binary once each, and after NPCStandIn went they no longer compiled. The prefabs
they used to generate are authored and committed; the wiring knowledge is in
those prefabs and in git history. The asset generators — Skill, Candidate,
Character — are untouched and still work.

EditMode 42/42, PlayMode 7/7, YarnCheck 9 files / 35 nodes. All three scenes
still text.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
2026-08-25 21:53:06 +02:00
co-authored by Claude Opus 5
parent ec4619d9dc
commit 9a0618fd31
54 changed files with 2292 additions and 2195 deletions
@@ -2,6 +2,7 @@ using System.Collections;
using System.Linq;
using NUnit.Framework;
using NightclubArcadia.Cinematics;
using NightclubArcadia.Interaction;
using Unity.Cinemachine;
using UnityEngine;
using UnityEngine.SceneManagement;
@@ -100,16 +101,18 @@ namespace NightclubArcadia.PlayMode.Tests
}
[Test]
public void EveryNpc_ResolvedItsDialogueRunner()
public void EveryInteractor_ResolvedItsDialogueRunner()
{
var npcs = Object.FindObjectsByType<NPCStandIn>(FindObjectsInactive.Include, FindObjectsSortMode.None);
Assert.IsNotEmpty(npcs, "Expected NPCs in the level scene");
var interactors = Object.FindObjectsByType<DialogueInteractor>(
FindObjectsInactive.Include, FindObjectsSortMode.None);
Assert.IsNotEmpty(interactors, "Expected interactors in the level scene");
foreach (var npc in npcs)
foreach (var interactor in interactors)
{
Assert.IsNotNull(npc.dialogueRunner,
$"'{npc.name}' did not resolve a DialogueRunner. It lives in the Systems scene, " +
"so the serialized reference is gone and SceneServices.Resolve has to find it.");
Assert.IsNotNull(interactor.ResolvedRunner,
$"'{interactor.name}' did not resolve a DialogueRunner. It lives in the " +
"Systems scene, so the serialized reference is gone and SceneServices has " +
"to find it.");
}
}
@@ -1,9 +1,10 @@
using System.Collections;
using System.Linq;
using NUnit.Framework;
using NightclubArcadia.Interaction;
using UnityEngine;
using UnityEngine.SceneManagement;
using UnityEngine.TestTools;
using Yarn.Unity;
namespace NightclubArcadia.PlayMode.Tests
{
@@ -11,16 +12,15 @@ namespace NightclubArcadia.PlayMode.Tests
/// The level must survive being loaded BEFORE the systems it depends on.
///
/// BootstrapTests only covers the happy order — Bootstrap loads Systems, then the
/// level, so by the time an NPC awakes the dialogue runner is already there. The
/// Editor does not guarantee that. Open the level on its own, or open scenes in a
/// different order, press Play, and NPCStandIn.Awake runs with no Systems scene in
/// sight. That produced:
/// level, so by the time an interactor needs the dialogue runner it is already there.
/// The Editor does not guarantee that. Open the level on its own, press Play, and
/// nothing in it can reach the runner. Resolving in Awake and caching the result
/// produced exactly that, permanently, for the whole session:
///
/// [SceneServices] Bartender: could not resolve dialogueRunner
///
/// Resolving a cross-scene dependency in Awake is a race by construction. The fix
/// is to resolve at the point of use instead, so any load order works — and this is
/// the test that holds that property.
/// DialogueInteractor resolves at the point of use and caches only success, so load
/// order stops mattering. This is the test that holds that.
/// </summary>
public class SceneOrderTests
{
@@ -34,35 +34,36 @@ namespace NightclubArcadia.PlayMode.Tests
}
[UnityTest]
public IEnumerator LevelLoadedBeforeSystems_NpcsStillResolveTheirRunner()
public IEnumerator LevelLoadedBeforeSystems_InteractorsStillResolveTheRunner()
{
// deliberately the wrong way round
yield return SceneManager.LoadSceneAsync(Level, LoadSceneMode.Single);
yield return SceneManager.LoadSceneAsync(Systems, LoadSceneMode.Additive);
for (var i = 0; i < 3; i++)
for (var i = 0; i < 2; i++)
{
yield return null;
}
var npcs = Object.FindObjectsByType<NPCStandIn>(
var interactors = Object.FindObjectsByType<DialogueInteractor>(
FindObjectsInactive.Include, FindObjectsSortMode.None);
Assert.IsNotEmpty(npcs, "Expected NPCs in the level scene");
Assert.IsNotEmpty(interactors, "Expected interactors in the level scene");
// Read the serialized field BEFORE touching the lazy property, which caches
// into it. If this is null, Awake really did run with no Systems scene — i.e.
// the race this test exists for is genuinely reproduced here, and the
// assertion below is not passing for free.
var eager = npcs.Count(n => n.dialogueRunner == null);
Debug.Log($"[Order] NPCs whose Awake-time resolution failed: {eager}/{npcs.Length}");
Assert.Greater(eager, 0,
"Awake-time resolution succeeded, so this test is not reproducing the race " +
"it was written for. Rewrite it before trusting it.");
// Non-vacuity: with only the level loaded there must genuinely be no runner
// to find. If this ever fails, the dependency stopped being cross-scene and
// the rest of this test proves nothing.
Assert.IsNull(Object.FindFirstObjectByType<DialogueRunner>(FindObjectsInactive.Include),
"A DialogueRunner exists with only the level loaded, so this test is no " +
"longer exercising cross-scene resolution. Rewrite it before trusting it.");
foreach (var npc in npcs)
yield return SceneManager.LoadSceneAsync(Systems, LoadSceneMode.Additive);
for (var i = 0; i < 2; i++)
{
Assert.IsNotNull(npc.ResolvedDialogueRunner,
$"'{npc.name}' could not reach the DialogueRunner when the level loaded " +
"before Systems. Cross-scene resolution must not depend on load order.");
yield return null;
}
foreach (var interactor in interactors)
{
Assert.IsNotNull(interactor.ResolvedRunner,
$"'{interactor.name}' could not reach the DialogueRunner when the level " +
"loaded before Systems. Cross-scene resolution must not depend on load order.");
}
}
}