fix(movement+anim+session): clothing dedup, motion wire format, jump-skill default

Three separate fixes landed today, each addressing a specific bug the
user observed during live play:

1. NPC clothing changes by camera angle (InstancedMeshRenderer)
   - Group key was (GfxObjId) only, so every humanoid NPC using the
     same body mesh piled into one instance group; only the first
     instance's texture was used for the entire DrawInstanced batch,
     so which NPC's palette "won" changed as frustum culling and
     iteration order shuffled entries.
   - Now keyed by (GfxObjId, PaletteHash ^ SurfaceOverridesHash)
     so only compatible instances batch; each unique appearance gets
     its own draw call. Perf hit is small (humanoid NPCs each emit
     one more draw call); visually every NPC is now stable.

2. GpuWorldState dedup on respawn
   - Server re-sends CreateObject for the same guid on visibility
     refresh / landblock crossing / appearance update. AppendLiveEntity
     was blindly appending each time, so GpuWorldState accumulated
     multiple copies of the same entity, each with its own
     PaletteOverride / MeshRefs. That alone wasn't the clothing bug
     (that was #1) but it would have caused other overlap problems
     downstream.
   - Added RemoveEntityByServerGuid + WorldGameState.RemoveById;
     OnLiveEntitySpawnedLocked calls both before creating the new
     entity so respawns replace cleanly.

3. Motion wire format — run animation sync with retail observers
   - ACE's MovementData constructor only computes interpState.ForwardSpeed
     on the WalkForward/WalkBackwards branch; every other ForwardCommand
     falls into `else` and passes through WITHOUT speed set, giving
     observers speed=0. Sending RunForward directly meant retail
     clients saw us "run in place" while position drifted forward.
   - Wire: always WalkForward + HoldKey.Run for running. ACE
     auto-upgrades to RunForward with creature.GetRunRate() for
     broadcast — correct command + correct speed at observers.
   - Added per-axis FORWARD_HOLD_KEY / SIDE_STEP_HOLD_KEY /
     TURN_HOLD_KEY so every active axis carries HoldKey.Run when
     running (matches holtburger's build_motion_state_raw_motion_state).
   - Added LocalAnimationCommand to MovementResult so our own
     client still plays the RunForward cycle locally while the wire
     stays WalkForward. Wire vs. local animation command are now
     decoupled.
   - Walk-backward wire command changed from WalkForward@-0.65 to
     WalkBackward@1.0 (holtburger pattern).
   - Strafe speed changed from 0.5 to 1.0 on wire AND local physics
     (matches retail sidestep pace).

4. Jump height default + env-var tuning
   - Default jumpSkill bumped from 100 → 200 (jump ≈ 3m at full
     charge, closer to retail feel for a mid-level character).
   - ACDREAM_RUN_SKILL and ACDREAM_JUMP_SKILL env vars now override
     the defaults so the user can tune per-character until we parse
     PlayerDescription and plumb real skill values through.

5. JustLanded signal on MovementResult
   - Tracks airborne→grounded transition so future animation code
     can fire the landing cycle when we land. Just a bool flag for
     now — no consumer yet (the proper action-queue path will use it).

Not in this commit: jump animation itself. An earlier attempt to
SetCycle(Jump=0x2500003b) fed an Action-type motion into the SubState
cycle resolver, which produced a "torso" mis-render. Reverted. The
proper fix is porting the retail motion action-queue semantics into
AnimationSequencer — see docs/research/deepdives/r03-motion-animation.md
for the spec. That's the next session's work.

470 tests pass, build clean.
This commit is contained in:
Erik 2026-04-18 15:01:32 +02:00
parent d951304875
commit 3308cddda7
6 changed files with 272 additions and 31 deletions

View file

@ -704,6 +704,26 @@ public sealed class GameWindow : IDisposable
{
_liveSpawnReceived++;
// De-dup: the server re-sends CreateObject for the same guid in
// several situations (visibility refresh, landblock crossing,
// appearance update). Without cleanup the OLD copy remains in
// GpuWorldState + WorldGameState + _animatedEntities, so the
// renderer draws both copies overlapped — producing the
// "NPC clothing changes when I turn the camera" bug because the
// depth test arbitrates between overlapping duplicates each frame.
//
// For a respawn, drop the previous rendering state here before we
// build the new one. `_entitiesByServerGuid` is the canonical map,
// its value is the live WorldEntity we need to dispose.
if (_entitiesByServerGuid.TryGetValue(spawn.Guid, out var existingEntity))
{
_worldState.RemoveEntityByServerGuid(spawn.Guid);
_worldGameState.RemoveById(existingEntity.Id);
_animatedEntities.Remove(existingEntity.Id);
// Physics collision registry entry is keyed by local id too.
_physicsEngine.ShadowObjects.Deregister(existingEntity.Id);
}
// Log every spawn that arrives so we can inventory what the server
// sends (including the ones we can't render yet). The Name field
// is the critical one — we can grep the log for "Nullified Statue
@ -2530,6 +2550,16 @@ public sealed class GameWindow : IDisposable
if (result.MotionStateChanged)
{
// HoldKey axis values — retail enum (holtburger types.rs HoldKey):
// Invalid = 0, None = 1, Run = 2
// Retail always sends CURRENT_HOLD_KEY (and uses the same
// value for every active per-axis hold key — see
// holtburger's build_motion_state_raw_motion_state).
// When the player is running forward, 2=Run; otherwise 1=None.
const uint HoldKeyNone = 1u;
const uint HoldKeyRun = 2u;
uint axisHoldKey = result.IsRunning ? HoldKeyRun : HoldKeyNone;
var seq = _liveSession.NextGameActionSequence();
var body = AcDream.Core.Net.Messages.MoveToState.Build(
gameActionSequence: seq,
@ -2539,7 +2569,10 @@ public sealed class GameWindow : IDisposable
sidestepSpeed: result.SidestepSpeed,
turnCommand: result.TurnCommand,
turnSpeed: result.TurnSpeed,
holdKey: result.ForwardCommand == 0x44000007u ? 1u : (uint?)null,
holdKey: axisHoldKey, // always present
forwardHoldKey: result.ForwardCommand.HasValue ? axisHoldKey : (uint?)null,
sidestepHoldKey: result.SidestepCommand.HasValue ? axisHoldKey : (uint?)null,
turnHoldKey: result.TurnCommand.HasValue ? axisHoldKey : (uint?)null,
cellId: wireCellId,
position: wirePos,
rotation: wireRot,
@ -2998,8 +3031,19 @@ public sealed class GameWindow : IDisposable
// Determine the animation command: forward takes priority, then sidestep,
// then turn, then idle (Ready 0x41000003).
//
// Note: AC's Jump (0x2500003b) is an Action motion (mask 0x25000000),
// NOT a SubState cycle. Feeding it to the MotionTable resolver via
// SetCycle produces a failed cycle lookup — which mis-renders the
// character. Proper action playback needs a separate sequencer path
// that honors the motion table's action queue; that's deferred.
// For now the player stays in whatever cycle was active when they
// jumped (usually walk/run or Ready) — animation wise it's wrong but
// at least the character doesn't implode.
uint animCommand;
if (result.ForwardCommand is { } fwd)
if (result.LocalAnimationCommand is { } localCmd)
animCommand = localCmd;
else if (result.ForwardCommand is { } fwd)
animCommand = fwd;
else if (result.SidestepCommand is { } ss)
animCommand = ss;