resolve cross-scene dependencies at the point of use, not in Awake

Fixes "[SceneServices] Bartender: could not resolve dialogueRunner". Resolving a cross-scene dependency in Awake is a race by construction. The dialogue runner lives in Systems; an NPC lives in the level. Open the level on its own, or let the scenes load in the other order, and the NPC wakes with no Systems scene in sight. Worse, the failed lookup was cached into the serialized field, so it stayed broken for the rest of the session rather than recovering once Systems appeared. NPCStandIn and DialogueInteractable now resolve lazily through a property and cache only a successful result, so any load order works. Awake still does a silent best-effort pass, which keeps the common case free. SceneServices splits in two accordingly. TryResolve is silent and is what speculative callers use; Resolve logs an error and belongs only where the dependency is genuinely required. Both now include inactive objects — a system parked inactive is still the object we mean, and excluding it reported "not found" for something sitting in the hierarchy. The error text now says where the runner lives and how to open the scenes, instead of asking a question. BootstrapTests only ever covered the happy order, which is why it stayed green while this was broken. SceneOrderTests loads the level BEFORE Systems and holds that NPCs still reach the runner. It also asserts it is reproducing the race — it counts the NPCs whose Awake-time resolution failed and fails if that is zero, so the test cannot quietly start passing for the wrong reason. It currently reports 2 of 2, which is the bug this commit fixes. EditMode 42/42, PlayMode 7/7, YarnCheck 9 files / 35 nodes. Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-25 21:42:54 +02:00
parent 2a20bd68fe
commit ec4619d9dc
6 changed files with 170 additions and 19 deletions
@@ -482,9 +482,18 @@ should not be — see `docs/restructure-plan.md`.

 2. **Serialized references cannot cross a scene boundary.** Unity nulls them. The split cost
   exactly seven: both NPCs' `dialogueRunner`, `dialogueUI` and `player`, plus the reveal
-   camera's tracking target. All are resolved at runtime now — `SceneServices.Resolve` for the
-   first, `CinemachineFollowsPlayer` for the last. Anything new that a level object needs from
-   Systems must follow that pattern, and `BootstrapTests` is where you prove it works.
+   camera's tracking target. All are resolved at runtime now — `SceneServices` for the first,
+   `CinemachineFollowsPlayer` for the last.
+
+   **Resolve at the point of use, never in `Awake`.** A level object can wake up before the
+   scene holding its dependency exists — open the level on its own, or in the wrong order, and
+   `Awake` runs with no Systems scene in sight. That produced
+   `[SceneServices] Bartender: could not resolve dialogueRunner`, and because the result was
+   cached once, it stayed broken for the whole session. `SceneServices.TryResolve` is the silent
+   lookup for speculative calls; `SceneServices.Resolve` logs and belongs only where the
+   dependency is genuinely needed. `NPCStandIn.ResolvedDialogueRunner` is the pattern to copy.
+   `SceneOrderTests` holds the property, and asserts it is actually reproducing the race rather
+   than passing for free.

 3. **The player's NavMeshAgent outlives the NavMesh.** The player is in Systems, the NavMesh is
   baked into the level, so there is a window during load where the agent exists and is not on a
@@ -17,22 +17,40 @@ namespace NightclubArcadia.Core
    /// </summary>
    public static class SceneServices
    {
-        /// <summary>Inspector value if set, otherwise the first one in any loaded scene.</summary>
-        public static T Resolve<T>(T assigned, Object context, string field) where T : Object
+        /// <summary>
+        /// Inspector value if set, otherwise the first one in any loaded scene. Silent:
+        /// returns null when nothing is found.
+        ///
+        /// Use this when the answer may legitimately not exist yet — during Awake, for
+        /// instance, when the scene holding it may not have loaded.
+        /// </summary>
+        public static T TryResolve<T>(T assigned) where T : Object
        {
            if (assigned != null)
            {
                return assigned;
            }

-            var found = Object.FindFirstObjectByType<T>();
+            // Include inactive: a system parked inactive at startup is still the object
+            // we mean, and excluding it produces a confusing "not found" for something
+            // sitting right there in the hierarchy.
+            return Object.FindFirstObjectByType<T>(FindObjectsInactive.Include);
+        }
+
+        /// <summary>
+        /// As TryResolve, but logs an error when nothing is found. Only call this at the
+        /// point the dependency is actually needed — calling it during Awake reports a
+        /// failure that may simply be a scene that has not finished loading.
+        /// </summary>
+        public static T Resolve<T>(T assigned, Object context, string field) where T : Object
+        {
+            var found = TryResolve(assigned);
            if (found == null)
            {
                Debug.LogError(
                    $"[SceneServices] {context?.name}: could not resolve {field} ({typeof(T).Name}). " +
                    "Is the Systems scene loaded?", context);
            }
-
            return found;
        }

@@ -21,8 +21,9 @@ namespace NightclubArcadia.Dialogue

        void Awake()
        {
-            // Same cross-scene resolution as NPCStandIn: the runner is in Systems.
-            dialogueRunner = SceneServices.Resolve(dialogueRunner, this, nameof(dialogueRunner));
+            // Best-effort and silent; the real lookup happens at the point of use, since
+            // Systems may not have loaded when this level object awakes. See Runner.
+            dialogueRunner = SceneServices.TryResolve(dialogueRunner);

            interactAction = new InputAction("Interact");
            interactAction.AddBinding("<Keyboard>/e");
@@ -36,6 +37,15 @@ namespace NightclubArcadia.Dialogue
            };
        }

+        /// <summary>
+        /// The dialogue runner, resolved on demand rather than in Awake, so that load
+        /// order between this level scene and Systems does not matter. Cached once found.
+        /// </summary>
+        DialogueRunner Runner =>
+            dialogueRunner != null
+                ? dialogueRunner
+                : dialogueRunner = SceneServices.TryResolve<DialogueRunner>(null);
+
        void OnEnable() => interactAction?.Enable();
        void OnDisable() => interactAction?.Disable();

@@ -57,6 +67,16 @@ namespace NightclubArcadia.Dialogue

        async void StartConversation()
        {
+            var runner = Runner;
+            if (runner == null)
+            {
+                Debug.LogError(
+                    $"[DialogueInteractable] {name}: no DialogueRunner in any loaded scene. " +
+                    "It lives in Systems.unity — open Bootstrap, or Tools → Nightclub Arcadia → " +
+                    "Open Game Scenes.", this);
+                return;
+            }
+
            // Player controls are locked by DialogueTransitionController off the
            // runner's onDialogueStart event — nothing to do for that here.
            if (DialogueUIVisibility.Show(dialogueUI))
@@ -67,7 +87,7 @@ namespace NightclubArcadia.Dialogue
                await Awaitable.NextFrameAsync();
            }

-            await dialogueRunner.StartDialogue(yarnNodeName);
+            await runner.StartDialogue(yarnNodeName);
        }
    }
 }
@@ -54,9 +54,9 @@ public class NPCStandIn : MonoBehaviour
        rend = GetComponent<Renderer>();
        rend.material.color = npcColor;

-        // The dialogue runner lives in the Systems scene, so this reference cannot be
-        // serialized from a level scene. An Inspector value still wins if one is set.
-        dialogueRunner = SceneServices.Resolve(dialogueRunner, this, nameof(dialogueRunner));
+        // Best-effort only, and silent. The runner lives in the Systems scene, which may
+        // not have loaded yet when this level's objects awake — see ResolvedDialogueRunner.
+        dialogueRunner = SceneServices.TryResolve(dialogueRunner);

        mainCam = Camera.main;

@@ -78,6 +78,20 @@ public class NPCStandIn : MonoBehaviour
        };
    }

+    /// <summary>
+    /// The dialogue runner, resolved on demand.
+    ///
+    /// Not resolved once in Awake: the runner lives in the Systems scene, and an NPC in
+    /// a level scene can wake up before that scene exists — loading the level on its own,
+    /// or in a different order, used to log "could not resolve dialogueRunner" and then
+    /// stay broken for the rest of the session. Looking it up at the point of use makes
+    /// load order irrelevant, and the result is cached, so this costs one search.
+    /// </summary>
+    public DialogueRunner ResolvedDialogueRunner =>
+        dialogueRunner != null
+            ? dialogueRunner
+            : dialogueRunner = SceneServices.TryResolve<DialogueRunner>(null);
+
    void OnEnable() => interactAction?.Enable();
    void OnDisable() => interactAction?.Disable();

@@ -153,9 +167,13 @@ public class NPCStandIn : MonoBehaviour

    public void Interact()
    {
-        if (dialogueRunner == null)
+        var runner = ResolvedDialogueRunner;
+        if (runner == null)
        {
-            Debug.LogWarning($"[NPCStandIn] No DialogueRunner assigned on {npcName}");
+            Debug.LogError(
+                $"[NPCStandIn] {npcName}: no DialogueRunner in any loaded scene. " +
+                "It lives in Systems.unity — open Bootstrap, or Tools → Nightclub Arcadia → " +
+                "Open Game Scenes.", this);
            return;
        }

@@ -164,7 +182,7 @@ public class NPCStandIn : MonoBehaviour
            return;
        }

-        if (!dialogueRunner.IsDialogueRunning)
+        if (!runner.IsDialogueRunning)
        {
            StartDialogueAsync();
        }
@@ -182,7 +200,7 @@ public class NPCStandIn : MonoBehaviour
            await Awaitable.NextFrameAsync();
        }

-        await dialogueRunner.StartDialogue(yarnStartNode);
+        await ResolvedDialogueRunner.StartDialogue(yarnStartNode);
    }

    /// <summary>
@@ -197,10 +215,16 @@ public class NPCStandIn : MonoBehaviour
            return dialogueUI;
        }

-        var canvas = dialogueRunner.GetComponentInChildren<Canvas>(true);
+        var runner = ResolvedDialogueRunner;
+        if (runner == null)
+        {
+            return null;
+        }
+
+        var canvas = runner.GetComponentInChildren<Canvas>(true);
        if (canvas == null)
        {
-            Debug.LogWarning($"[NPCStandIn] No Canvas found under {dialogueRunner.name} for {npcName}");
+            Debug.LogWarning($"[NPCStandIn] No Canvas found under {runner.name} for {npcName}");
            return null;
        }

@@ -0,0 +1,69 @@
+using System.Collections;
+using System.Linq;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.SceneManagement;
+using UnityEngine.TestTools;
+
+namespace NightclubArcadia.PlayMode.Tests
+{
+    /// <summary>
+    /// 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:
+    ///
+    ///   [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.
+    /// </summary>
+    public class SceneOrderTests
+    {
+        const string Systems = "Systems";
+        const string Level = "SC101_ConferenceHall";
+
+        [UnityTearDown]
+        public IEnumerator TearDown()
+        {
+            yield return SceneManager.LoadSceneAsync("Bootstrap", LoadSceneMode.Single);
+        }
+
+        [UnityTest]
+        public IEnumerator LevelLoadedBeforeSystems_NpcsStillResolveTheirRunner()
+        {
+            // 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++)
+            {
+                yield return null;
+            }
+
+            var npcs = Object.FindObjectsByType<NPCStandIn>(
+                FindObjectsInactive.Include, FindObjectsSortMode.None);
+            Assert.IsNotEmpty(npcs, "Expected NPCs 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.");
+
+            foreach (var npc in npcs)
+            {
+                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.");
+            }
+        }
+    }
+}
@@ -0,0 +1,11 @@
+fileFormatVersion: 2
+guid: 959e3d8621d544168b89e4952a7668e0
+MonoImporter:
+  externalObjects: {}
+  serializedVersion: 2
+  defaultReferences: []
+  executionOrder: 0
+  icon: {instanceID: 0}
+  userData: 
+  assetBundleName: 
+  assetBundleVariant: