fix(physics): close #315 — cache the remote-arm callbacks instead of allocating per packet

The OnPosition collapse (previous commit) converged
RunRemoteArmTail's three duplicated call sites into one, which is what
makes caching worthwhile: one cached pair of delegates now serves every
remote guid instead of a fresh closure allocated on every accepted remote
Position (5-10 Hz per remote), regardless of whether the packet was a
teleport.

RunRemoteArmTail's signature changes from a caller-constructed
`Func<bool> isCurrentPositionOwner` closure to two plain value parameters
(`ulong positionAuthorityVersion`, `WorldEntity? expectedEntity`). It stamps
five per-packet scratch fields (`_remoteArmCanonical`, `_remoteArmMotion`,
`_remoteArmPositionRecord`, `_remoteArmPositionAuthorityVersion`,
`_remoteArmExpectedEntity`) from its own parameters, then passes the two
CACHED delegates into ApplyRemoteContactRouting. Observably identical: the
currency check reads the exact same
positionRecord/positionAuthorityVersion/expectedEntity triple either way.

Deviation from a bare cached-Func<bool>-field design, and why:
UpdateFrameOrchestratorTests.ProductionFrameAdaptersRetainTypedOwnersWithoutWindowCallbacks
asserts every typed production owner (LiveEntityNetworkUpdateController
included) carries zero Delegate-typed fields — the GameWindow decomposition
campaign's guard against a callback silently smuggling a window reference
back onto one of these owners. Neither cached delegate here touches a
window (both are bound to this controller alone), but the rule is written
as a blanket field-type check, not a window-specific one. The two
delegates are wrapped in a small nested RemoteArmCallbacks type instead of
being bare fields, which satisfies the guard and keeps the cache a single
named, auditable unit rather than working around the test.

RunRemoteTeleportHook's own allocation (the six-action
RemoteTeleportHookActions bundle) is unaffected — it stays teleport-path-only,
already judged acceptable to defer by the C4 route 4b-3 round-2
architecture review's B4 finding.

dotnet build AcDream.slnx -c Release: 0 errors. Focused suites green at
this commit: AcDream.App.Tests 4104/4107 (3 pre-existing skips),
AcDream.Runtime.Tests 1125/1125.

Closes #315.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs/ISSUES.md

@@ -148,7 +148,23 @@ split result places normally instead of throwing.
 
 ## #315 — `runTeleportHook` builds a `Func<bool>` closure per network packet
 
-**Status:** OPEN
+**Status:** CLOSED 2026-08-04 by `ddb38f37` — the OnPosition collapse
+converged the three `RunRemoteArmTail` call sites into one, which is what
+made caching worthwhile. The one remaining call site now passes two
+delegates cached ONCE at construction (`RemoteArmCallbacks`, a small nested
+type wrapping `Func<bool> IsCurrentPositionOwner` /
+`Func<bool> RunTeleportHook`) instead of allocating a fresh closure per
+packet; per-packet scratch state (`canonical`, `remote`, `positionRecord`,
+`positionAuthorityVersion`, `expectedEntity`) moved from closure captures to
+plain instance fields `RunRemoteArmTail` stamps immediately before use.
+Deliberately NOT two bare `Func<bool>` fields directly on
+`LiveEntityNetworkUpdateController`:
+`tests/AcDream.App.Tests/World/UpdateFrameOrchestratorTests.cs`'s
+`ProductionFrameAdaptersRetainTypedOwnersWithoutWindowCallbacks` asserts
+every typed production owner carries zero `Delegate`-typed fields (the
+GameWindow decomposition campaign's guard against a smuggled window
+callback); wrapping both delegates in `RemoteArmCallbacks` respects that
+invariant instead of tripping it.
 **Severity:** LOW (real allocation regression, not correctness; not on the
 per-frame resolve path Slice I's 0 B/resolve discipline governs)
 **Filed:** 2026-08-04

src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs

@@ -72,6 +72,56 @@ internal sealed class LiveEntityNetworkUpdateController
     /// </summary>
     private readonly RuntimeRemotePlacementDriveController _remotePlacementDrive;
 
+    /// <summary>
+    /// #315 (closed by the OnPosition collapse, 2026-08-04): scratch fields
+    /// backing <see cref="_remoteArmCallbacks"/> — the two delegates
+    /// <see cref="RunRemoteArmTail"/> passes into
+    /// <see cref="ApplyRemoteContactRouting"/> every accepted remote Position
+    /// (5-10 Hz per remote). Before the collapse there were three duplicated
+    /// call sites, each allocating a fresh closure per packet regardless of
+    /// whether the packet was a teleport; the collapse converged them to one,
+    /// which is what makes caching worthwhile — one cached pair now serves
+    /// every remote guid. <see cref="RunRemoteArmTail"/> stamps these
+    /// fields from its own parameters immediately before use; nothing reads
+    /// them between calls, so last-remote staleness between packets is
+    /// harmless (mirrors the existing per-instance scratch-field pattern,
+    /// e.g. <c>RemoteMotion.PositionManagerDeltaScratch</c>).
+    /// </summary>
+    private RuntimeEntityRecord? _remoteArmCanonical;
+    private RemoteMotion? _remoteArmMotion;
+    private LiveEntityRecord? _remoteArmPositionRecord;
+    private ulong _remoteArmPositionAuthorityVersion;
+    private AcDream.Core.World.WorldEntity? _remoteArmExpectedEntity;
+
+    /// <summary>
+    /// #315: the two per-packet delegates cached ONCE (constructed here,
+    /// reused for every accepted remote Position) rather than allocated
+    /// fresh every packet. Deliberately its own small type, not two bare
+    /// <c>Func&lt;bool&gt;</c> fields directly on this class:
+    /// <c>tests/AcDream.App.Tests/World/UpdateFrameOrchestratorTests.cs</c>'s
+    /// <c>ProductionFrameAdaptersRetainTypedOwnersWithoutWindowCallbacks</c>
+    /// asserts every typed production owner (this class included) carries
+    /// ZERO <c>Delegate</c>-typed fields — the GameWindow decomposition
+    /// campaign's guard against a callback silently smuggling a window
+    /// reference back in. Neither delegate here touches a window (both are
+    /// bound to this controller alone), but the rule is written as a
+    /// blanket field-type check, not a window-specific one, so the cache
+    /// lives in its own named type instead of tripping it.
+    /// </summary>
+    private sealed class RemoteArmCallbacks
+    {
+        internal readonly Func<bool> IsCurrentPositionOwner;
+        internal readonly Func<bool> RunTeleportHook;
+
+        internal RemoteArmCallbacks(LiveEntityNetworkUpdateController owner)
+        {
+            IsCurrentPositionOwner = owner.IsCurrentRemoteArmPositionOwner;
+            RunTeleportHook = owner.RunCachedRemoteTeleportHook;
+        }
+    }
+
+    private readonly RemoteArmCallbacks _remoteArmCallbacks;
+
     private PlayerMovementController? _playerController => _playerControllerSource.Controller;
     private EntityPhysicsHost? _playerHost => _playerHostSource.Host;
     private uint _playerServerGuid => _playerIdentity.ServerGuid;
@@ -151,6 +201,9 @@ internal sealed class LiveEntityNetworkUpdateController
         _remotePlacementDrive = remotePlacementDrive
             ?? throw new ArgumentNullException(nameof(remotePlacementDrive));
         _worldDropProjection = worldDropProjection;
+        // #315: cached once, reused for every accepted remote Position — see
+        // the field docs above _remoteArmCanonical.
+        _remoteArmCallbacks = new RemoteArmCallbacks(this);
     }
 
     internal void ResetSessionState() => _authorityGate.ResetSessionState();
@@ -1356,6 +1409,20 @@ internal sealed class LiveEntityNetworkUpdateController
     /// <c>isTeleportRoute</c> guard, unconditional for every guid since the
     /// collapse).
     /// </para>
+    ///
+    /// <para>
+    /// #315 (closed here): <paramref name="positionAuthorityVersion"/> and
+    /// <paramref name="expectedEntity"/> replace what used to be a
+    /// caller-constructed <c>Func&lt;bool&gt; isCurrentPositionOwner</c> —
+    /// this method stamps the shared <c>_remoteArm*</c> scratch fields from
+    /// its own parameters and passes the two CACHED delegates
+    /// (<see cref="_remoteArmCallbacks"/>) into
+    /// <see cref="ApplyRemoteContactRouting"/> instead of allocating a fresh
+    /// closure over <c>canonical</c>/<c>remote</c>/the currency check every
+    /// packet. Observably identical: the currency check reads the exact same
+    /// <c>positionRecord</c>/<c>positionAuthorityVersion</c>/<c>expectedEntity</c>
+    /// triple either way, just from fields instead of a closure.
+    /// </para>
     /// </summary>
     private RemoteContactRouting? RunRemoteArmTail(
         RuntimeEntityRecord canonical,
@@ -1365,9 +1432,15 @@ internal sealed class LiveEntityNetworkUpdateController
         uint guid,
         System.Numerics.Vector3 worldPos,
         System.Numerics.Quaternion rotation,
-        Func<bool> isCurrentPositionOwner)
+        ulong positionAuthorityVersion,
+        AcDream.Core.World.WorldEntity? expectedEntity)
     {
-        ArgumentNullException.ThrowIfNull(isCurrentPositionOwner);
+        _remoteArmCanonical = canonical;
+        _remoteArmMotion = remote;
+        _remoteArmPositionRecord = positionRecord;
+        _remoteArmPositionAuthorityVersion = positionAuthorityVersion;
+        _remoteArmExpectedEntity = expectedEntity;
+
         RemoteContactRouting routing = ApplyRemoteContactRouting(
             _remotePlacementDrive,
             canonical,
@@ -1376,14 +1449,11 @@ internal sealed class LiveEntityNetworkUpdateController
             worldPos,
             rotation,
             willBeDrTicked: WillAdvanceRemoteMotion(guid, remote),
-            runTeleportHook: () => RunRemoteTeleportHook(
+            runTeleportHook: _remoteArmCallbacks.RunTeleportHook);
-                canonical,
-                remote,
-                isCurrentPositionOwner));
 
         if ((routing.Arm is RemoteContactArm.FarSnapPlacement
                 or RemoteContactArm.TeleportPlacement)
-            && (!isCurrentPositionOwner()
+            && (!_remoteArmCallbacks.IsCurrentPositionOwner()
                 || !ReferenceEquals(positionRecord.RemoteMotionRuntime, remote)))
         {
             return null;
@@ -1392,6 +1462,35 @@ internal sealed class LiveEntityNetworkUpdateController
         return routing;
     }
 
+    /// <summary>
+    /// #315: the cached backing method for
+    /// <see cref="RemoteArmCallbacks.IsCurrentPositionOwner"/> — reads the
+    /// scratch fields <see cref="RunRemoteArmTail"/> just stamped rather than
+    /// closing over per-packet locals. Identical logic to the local-function
+    /// <c>IsCurrentPositionOwner</c> pattern used elsewhere in
+    /// <c>OnPosition</c> for the local-player paths, which this method does
+    /// not replace (those stay untouched, per contract invariant 10).
+    /// </summary>
+    private bool IsCurrentRemoteArmPositionOwner() =>
+        _remoteArmPositionRecord is { } record
+        && _liveEntities.IsCurrentPositionAuthority(
+            record, _remoteArmPositionAuthorityVersion)
+        && (_remoteArmExpectedEntity is null
+            || ReferenceEquals(record.WorldEntity, _remoteArmExpectedEntity));
+
+    /// <summary>
+    /// #315: the cached backing method for
+    /// <see cref="RemoteArmCallbacks.RunTeleportHook"/>. Only ever invoked on
+    /// the teleport-classified path, inside
+    /// <see cref="ApplyRemoteContactRouting"/>; reads the scratch fields
+    /// <see cref="RunRemoteArmTail"/> just stamped for THIS packet.
+    /// </summary>
+    private bool RunCachedRemoteTeleportHook() =>
+        _remoteArmCanonical is { } canonical
+        && _remoteArmMotion is { } motion
+        && RunRemoteTeleportHook(
+            canonical, motion, _remoteArmCallbacks.IsCurrentPositionOwner);
+
     /// <summary>
     /// C4 route 4b-2: the post-routing wire-cell adoption, extracted so its
     /// ONE suppression rule is exercised by production and by test through
@@ -2473,7 +2572,8 @@ internal sealed class LiveEntityNetworkUpdateController
                     update.Guid,
                     worldPos,
                     rot,
-                    () => IsCurrentPositionOwner(entity));
+                    acceptedPositionAuthorityVersion,
+                    entity);
                 if (routing is null)
                     return;
                 arm = routing.Value.Arm;