Skip to content

Synchronize field battle horse animations - #2300

Merged
ShoT-UPfps merged 42 commits into
developmentfrom
fix-field-battle-animation-desync
Aug 1, 2026
Merged

Synchronize field battle horse animations#2300
ShoT-UPfps merged 42 commits into
developmentfrom
fix-field-battle-animation-desync

Conversation

@ShoT-UPfps

@ShoT-UPfps ShoT-UPfps commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • All new classes have class-level documentation comments, if there are any at all
  • Tests for the changes have been added (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation update
  • Other... Please describe:

What is the current behavior?

Cavalry horses can stop moving at their destination on one client while continuing to play a walking animation on another.

What is the new behavior?

Cavalry horse movement animations now settle consistently on every client when horses stop, and resume at the authoritative playback speed when they move again.

Battle diagnostics can also focus the camera on a synchronized mount and capture its rendered head-pose timeline for desync evidence.

Bot Changelog Entry

  • Fixed cavalry horses continuing to walk in place for non-host players after stopping in field battles.

Remote mount packets did not carry rendered playback speed or clear the authoritative no-action state, so stopped puppets could keep walking in place. Replicate and apply the owner gait speed and stopped state while preserving legitimate idle animations.
Cap the staged cavalry at zero speed while held or turning, then restore normal speed for movement orders so combat AI cannot invalidate the animation oracle.
Use Bannerlord action-defined blending when the authoritative client installs a synthetic stationary mount turn. This keeps small facing corrections from snapping the horse pose while preserving the advancing replicated turn phase.
@ShoT-UPfps
ShoT-UPfps marked this pull request as ready for review August 1, 2026 01:37
Resolve the movement batching conflict so synthetic mount-turn state and progress still reach remote clients while unchanged movement remains filtered.
The live-test camera hook runs during ordinary mission pre-display ticks. Use reference null checks so mock missions do not initialize Bannerlord's unavailable native object bridge.

@garrettluskey garrettluskey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core fix is sound — zeroing puppet input when the owner is stationary and syncing the gait playback speed is the right call for the walk-in-place desync, and the test coverage is thorough. But there are two correctness holes in the remote synthetic-turn lifecycle and some wire/perf costs on the movement hot path I want addressed before this merges. Details inline.

Also flagging scope: BattleDebugCommands grew by ~645 lines of camera/pose-capture tooling that the PR description doesn't mention. I'm fine keeping the tooling, but it's more than half the production diff of a "bug fix" PR — give it a sentence in the description so the changelog and reviewers aren't surprised.

Comment thread source/Missions/Agents/Handlers/AgentMovementHandler.cs
Comment thread source/Missions/Agents/Handlers/AgentMovementHandler.cs Outdated
Comment thread source/Missions/Agents/Packets/AgentMountData.cs Outdated
Comment thread source/Missions/Agents/Packets/AgentMountData.cs Outdated
Comment thread source/Missions/Agents/Packets/AgentMountData.cs Outdated
Comment thread source/Missions/Agents/Handlers/AgentVisualActionAccessor.cs Outdated
Comment thread source/Missions/CoopMissionController.cs Outdated

@garrettluskey garrettluskey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed e00cc43..50a37ec. All seven requested changes are in and each got a regression test — verified in code, not just the replies:

  • Remote replay now drops turn state once the mount or rider is locally controlled, with an authority-transfer test covering it.
  • Resume-during-grace takes the incoming progress; test pins it at 0.1 after a resumed turn.
  • Channel 0 goes back through GetActionNameWithCode and fails closed — the unknown-index test asserting zero SetActionChannel calls is the right trade, since the harness can't resolve names and the positive path was only E2E-testable while the code cheated with raw indices. Matches what TryResolveActionTransition already does.
  • Field 19 ships as index+1 with the private wire property, and the DeepClone round-trip proves the encoding.
  • Moving mounts skip the skeleton read entirely.
  • TrySetAction narrows to NullReferenceException.
  • The debug tick lives in a self-registering MissionBehavior; the controller no longer references BattleDebugCommands.

Build, unit, and all eight E2E shards are green. CodeFactor is red — glance at it before merging, but I'm not blocking on a linter.

Two take-it-or-leave-it notes inline; neither needs another round.

Comment thread source/Missions/Agents/Handlers/AgentMovementHandler.cs
Comment thread source/Missions/Agents/Packets/AgentMountData.cs Outdated
…le-animation-desync

# Conflicts:
#	source/Missions/Agents/Packets/AgentMountData.cs
#	source/Missions/Battles/BattleDebugCommands.cs
#	source/Missions/CoopMissionController.cs
@ShoT-UPfps
ShoT-UPfps merged commit 36819cb into development Aug 1, 2026
19 of 21 checks passed
@ShoT-UPfps
ShoT-UPfps deleted the fix-field-battle-animation-desync branch August 1, 2026 13:16
@github-project-automation github-project-automation Bot moved this from Todo to Done in Sync Backlog Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants