Skip to content

Fix player position desync during movement #768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
itayalroy wants to merge 4 commits into from
Closed

Fix player position desync during movement #768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ad12ef
Use projected walks for player movement
itayalroy May 8, 2026
e16b9ab
Block player attacks involving safe zones
itayalroy May 8, 2026
9ac788f
Stop sending walk directions by default
itayalroy May 8, 2026
a59ae7a
Fix walk step ordering for player movement
itayalroy May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/GameLogic/LocateableExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ public static bool IsAtSafezone(this ILocateable obj)
return true;
}

return map.Terrain.SafezoneMap[obj.Position.X, obj.Position.Y];
var position = obj.Position;
return map.Terrain.SafezoneMap[position.X, position.Y];
}
}
}
192 changes: 156 additions & 36 deletions src/GameLogic/Player.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace MUnique.OpenMU.GameLogic;
using System.Threading;
using MUnique.OpenMU.AttributeSystem;
using MUnique.OpenMU.DataModel.Attributes;
using MUnique.OpenMU.DataModel.Configuration.Items;
using MUnique.OpenMU.GameLogic.Attributes;
using MUnique.OpenMU.GameLogic.GuildWar;
using MUnique.OpenMU.GameLogic.MiniGames;
Expand Down Expand Up @@ -52,7 +53,7 @@ public class Player : AsyncDisposable, IBucketMapObserver, IAttackable, IAttacke
private readonly AsyncLock _moveLock = new();
private readonly AsyncLock _experienceLock = new();

private readonly Walker _walker;
private readonly ProjectedWalker _projectedWalker;

private readonly AppearanceDataAdapter _appearanceData;

Expand Down Expand Up @@ -89,7 +90,11 @@ public Player(IGameContext gameContext)
this.GameContext = gameContext;
this.Logger = gameContext.LoggerFactory.CreateLogger<Player>();
this.PersistenceContext = this.GameContext.PersistenceContextProvider.CreateNewPlayerContext(gameContext.Configuration);
this._walker = new Walker(this, this.GetStepDelay);
this._projectedWalker = new ProjectedWalker(
this.GetStoredPosition,
this.SetStoredPosition,
this.GetStepDuration,
this.RaiseAttackableMoved);

this.MagicEffectList = new MagicEffectsList(this);
this._appearanceData = new AppearanceDataAdapter(this);
Expand Down Expand Up @@ -145,13 +150,13 @@ public Player(IGameContext gameContext)
public bool CanWalkOnSafezone => true;

/// <inheritdoc />
public bool IsWalking => this._walker.CurrentTarget != default;
public bool IsWalking => this._projectedWalker.IsWalking;

/// <inheritdoc />
public TimeSpan StepDelay => this.GetStepDelay();
public TimeSpan StepDelay => this.GetStepDuration(null);

/// <inheritdoc />
public Point WalkTarget => this._walker.CurrentTarget;
public Point WalkTarget => this._projectedWalker.CurrentTarget;

/// <summary>
/// Gets a value indicating whether this instance is invisible to other players.
Expand Down Expand Up @@ -380,17 +385,9 @@ private set
/// <inheritdoc/>
public Point Position
{
get => new(this.SelectedCharacter?.PositionX ?? 0, this.SelectedCharacter?.PositionY ?? 0);
get => this._projectedWalker.Position;

set
{
if (this.Position != value && this.SelectedCharacter is { } character)
{
character.PositionX = value.X;
character.PositionY = value.Y;
this.GameContext.PlugInManager?.GetPlugInPoint<IAttackableMovedPlugIn>()?.AttackableMoved(this);
}
}
set => this._projectedWalker.Position = value;
}

/// <summary>
Expand Down Expand Up @@ -715,6 +712,11 @@ public ValueTask ShowBlueMessageAsync(string message)
throw new InvalidOperationException("AttributeSystem not set.");
}

if (this.IsAttackBlockedBySafezone(attacker))
{
return null;
}

if (!this.GameContext.PvpEnabled && this.CurrentMap?.Definition.BattleZone == null &&
this.CurrentMiniGame?.AllowPlayerKilling is false)
{
Expand Down Expand Up @@ -816,7 +818,7 @@ public async Task TeleportAsync(Point target, Skill teleportSkill)
{
await (this.SkillCancelTokenSource?.CancelAsync() ?? Task.CompletedTask).ConfigureAwait(false);

await this._walker.StopAsync().ConfigureAwait(false);
this.StopProjectedWalk();

var previous = this.Position;
this.Position = target;
Expand Down Expand Up @@ -866,7 +868,7 @@ public async Task TeleportToMapAsync(GameMap targetMap, Point targetPoint)
{
await (this.SkillCancelTokenSource?.CancelAsync() ?? Task.CompletedTask).ConfigureAwait(false);

await this._walker.StopAsync().ConfigureAwait(false);
this.StopProjectedWalk();

await this.ForEachWorldObserverAsync<IObjectsOutOfScopePlugIn>(p => p.ObjectsOutOfScopeAsync(this.GetAsEnumerable()), false).ConfigureAwait(false);

Expand Down Expand Up @@ -1331,7 +1333,7 @@ private async ValueTask AddExperienceCoreAsync(int experience, IAttackable? kill
public async ValueTask MoveAsync(Point target)
{
this.Logger.LogDebug("MoveAsync: Player is moving to {0}", target);
await this._walker.StopAsync().ConfigureAwait(false);
this.StopProjectedWalk();
await this.CurrentMap!.MoveAsync(this, target, this._moveLock, MoveType.Instant).ConfigureAwait(false);
this.Logger.LogDebug("MoveAsync: Observer Count: {0}", this.Observers.Count);
}
Expand Down Expand Up @@ -1364,7 +1366,7 @@ public async ValueTask WalkToAsync(Point target, Memory<WalkingStep> steps)
return;
}

await this._walker.StopAsync().ConfigureAwait(false);
this.StopProjectedWalk();

var startPoint = steps.Span[0].From;
var currentPosition = this.Position;
Expand All @@ -1379,34 +1381,85 @@ public async ValueTask WalkToAsync(Point target, Memory<WalkingStep> steps)
return;
}

var canWalkToTarget = currentMap.Terrain.WalkMap[target.X, target.Y];
if (canWalkToTarget)
if (ProjectedWalker.TryValidateWalkSteps(currentMap.Terrain, steps, target, out var invalidPoint))
{
this.Logger.LogDebug("WalkToAsync: Player is walking to {0}", target);

var token = await this._walker.InitializeWalkToAsync(target, steps).ConfigureAwait(false);
this.Position = startPoint;
this.InitializeProjectedWalk(target, steps);
await currentMap.MoveAsync(this, target, this._moveLock, MoveType.Walk).ConfigureAwait(false);
await this._walker.StartWalkAsync(token).ConfigureAwait(false);

this.Logger.LogDebug("WalkToAsync: Observer Count: {0}", this.Observers.Count);
}
else
{
this.Logger.LogWarning("WalkToAsync: Player requested to walk to {0}, but it's not an allowed target", target);
this.Logger.LogWarning("WalkToAsync: Player requested an invalid walk to {0}. Invalid point: {1}", target, invalidPoint);

// We'll send the current coordinates back to the client, so it doesn't appear in the invalid coordinates.
await this.InvokeViewPlugInAsync<IObjectMovedPlugIn>(p => p.ObjectMovedAsync(this, MoveType.Instant)).ConfigureAwait(false);
}
}

/// <inheritdoc />
public ValueTask<int> GetDirectionsAsync(Memory<Direction> directions) => this._walker.GetDirectionsAsync(directions);
public ValueTask<int> GetDirectionsAsync(Memory<Direction> directions) => this._projectedWalker.GetDirectionsAsync(directions);

/// <inheritdoc />
public ValueTask<int> GetStepsAsync(Memory<WalkingStep> steps) => this._walker.GetStepsAsync(steps);
public ValueTask<int> GetStepsAsync(Memory<WalkingStep> steps) => this._projectedWalker.GetStepsAsync(steps);

/// <inheritdoc />
public ValueTask StopWalkingAsync() => this._walker.StopAsync();
public ValueTask StopWalkingAsync()
{
this.StopProjectedWalk();
return ValueTask.CompletedTask;
}

private void InitializeProjectedWalk(Point target, Memory<WalkingStep> steps)
{
this._projectedWalker.Initialize(target, steps);
}

private void StopProjectedWalk()
{
this._projectedWalker.Stop();
}

private Point GetStoredPosition()
{
return new(this.SelectedCharacter?.PositionX ?? 0, this.SelectedCharacter?.PositionY ?? 0);
}

private bool SetStoredPosition(Point position)
{
if (this.SelectedCharacter is not { } character)
{
return false;
}

if (character.PositionX == position.X && character.PositionY == position.Y)
{
return false;
}

character.PositionX = position.X;
character.PositionY = position.Y;
return true;
}

private void RaiseAttackableMoved()
{
this.GameContext.PlugInManager?.GetPlugInPoint<IAttackableMovedPlugIn>()?.AttackableMoved(this);
}

private bool IsAttackBlockedBySafezone(IAttacker attacker)
{
if (this.IsAtSafezone())
{
return true;
}

var attackerPlayer = attacker as Player ?? (attacker as IPlayerSurrogate)?.Owner;
return attackerPlayer?.IsAtSafezone() is true;
}

/// <summary>
/// Regenerates the attributes specified in <see cref="Stats.IntervalRegenerationAttributes"/>.
Expand Down Expand Up @@ -1884,7 +1937,7 @@ protected override async ValueTask DisposeAsyncCore()
await this.RemoveFromCurrentMapAsync().ConfigureAwait(false);
await this._observerToWorldViewAdapter.ClearObservingObjectsListAsync().ConfigureAwait(false);
this._observerToWorldViewAdapter.Dispose();
this._walker.Dispose();
this.StopProjectedWalk();
await this.MagicEffectList.DisposeAsync().ConfigureAwait(false);
this._respawnAfterDeathCts?.Dispose();
(this._viewPlugIns as IDisposable)?.Dispose();
Expand Down Expand Up @@ -1962,7 +2015,7 @@ private async ValueTask<bool> TryRemoveFromCurrentMapAsync(bool willRespawnOnSam

this.IsAlive = false;
this.IsTeleporting = false;
await this._walker.StopAsync().ConfigureAwait(false);
this.StopProjectedWalk();
await this._observerToWorldViewAdapter.ClearObservingObjectsListAsync().ConfigureAwait(false);
if (this.Summon?.Item1 is { IsAlive: true } summon)
{
Expand Down Expand Up @@ -2110,16 +2163,83 @@ private async ValueTask RegenerateHeroStateAsync()
/// Gets the step delay depending on the equipped items.
/// </summary>
/// <returns>The current step delay, depending on equipped items.</returns>
private TimeSpan GetStepDelay()
private TimeSpan GetStepDuration(WalkingStep? step)
{
const double referenceFrameTimeMilliseconds = 40.0;
const double terrainScale = 100.0;

var speed = this.GetClientMovementSpeed(step?.From);
var tileDistance = step is { } walkingStep ? walkingStep.From.EuclideanDistanceTo(walkingStep.To) : 1.0;
var movementMilliseconds = terrainScale * Math.Max(1.0, tileDistance) / speed * referenceFrameTimeMilliseconds;

return TimeSpan.FromMilliseconds(movementMilliseconds);
}

private double GetClientMovementSpeed(Point? position = null)
{
if (this.Inventory?.EquippedItems.Any(item => item.Definition?.ItemSlot?.ItemSlots.Contains(7) ?? false) ?? false)
const double walkSpeed = 12.0;
const double runSpeed = 15.0;
const double fastWingSpeed = 16.0;
const double horseOrFenrirRunSpeed = 17.0;
const double excellentFenrirRunSpeed = 19.0;

if (this.IsInClientSafezone(position))
{
return walkSpeed;
}

var pet = this.Inventory?.GetItem(InventoryConstants.PetSlot);
if (IsItem(pet, 13, 37))
{
return HasFenrirMovementOption(pet) ? excellentFenrirRunSpeed : horseOrFenrirRunSpeed;
}

if (IsItem(pet, 13, 4))
{
// Wings
return TimeSpan.FromMilliseconds(300);
return horseOrFenrirRunSpeed;
}

// TODO: Consider pets etc.
return TimeSpan.FromMilliseconds(500);
var wings = this.Inventory?.GetItem(InventoryConstants.WingsSlot);
if (HasEquippedWings(wings) || IsItem(pet, 13, 2) || IsItem(pet, 13, 3))
{
return IsFastWing(wings) ? fastWingSpeed : runSpeed;
}

return runSpeed;
}

private bool IsInClientSafezone(Point? position = null)
{
var checkedPosition = position ?? this.Position;
return this.CurrentMap?.Terrain.SafezoneMap[checkedPosition.X, checkedPosition.Y] ?? false;
}

private static bool HasEquippedWings(Item? item)
{
return item is { Durability: > 0.0 }
&& item.ItemSlot == InventoryConstants.WingsSlot;
}

private static bool IsFastWing(Item? item)
{
return IsItem(item, 12, 5) // Wings of Dragon
|| IsItem(item, 12, 36); // Wing of Storm
}

private static bool IsItem(Item? item, short group, short number)
{
return item is { Durability: > 0.0 }
&& item.Definition is { } definition
&& definition.Group == group
&& definition.Number == number;
}

private static bool HasFenrirMovementOption(Item? item)
{
return item?.ItemOptions.Any(option =>
option.ItemOption?.OptionType == ItemOptionTypes.BlueFenrir
|| option.ItemOption?.OptionType == ItemOptionTypes.BlackFenrir
|| option.ItemOption?.OptionType == ItemOptionTypes.GoldFenrir) ?? false;
}

private async ValueTask<ExitGate> GetSpawnGateOfCurrentMapAsync()
Expand Down Expand Up @@ -2254,7 +2374,7 @@ private async ValueTask OnDeathAsync(IAttacker? killer)
return;
}

await this._walker.StopAsync().ConfigureAwait(false);
this.StopProjectedWalk();
this.IsAlive = false;
this._respawnAfterDeathCts = new CancellationTokenSource();
await this.ForEachWorldObserverAsync<IObjectGotKilledPlugIn>(p => p.ObjectGotKilledAsync(this, killer), true).ConfigureAwait(false);
Expand Down
Loading