-
Notifications
You must be signed in to change notification settings - Fork 513
Fix player position desync during movement #770
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,11 @@ namespace MUnique.OpenMU.GameLogic.NPC; | |
| /// </summary> | ||
| public sealed class Monster : AttackableNpcBase, IAttackable, IAttacker, ISupportWalk, IMovable, ISummonable | ||
| { | ||
| private const short IcedEffectNumber = 0x38; | ||
| private const short BlowOfDestructionEffectNumber = 0x56; | ||
| private const double IcedMovementSpeedFactor = 0.5; | ||
| private const double BlowOfDestructionMovementSpeedFactor = 0.33; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a new |
||
|
|
||
| private readonly AsyncLock _moveLock = new(); | ||
| private readonly INpcIntelligence _intelligence; | ||
| private readonly Walker _walker; | ||
|
|
@@ -59,7 +64,7 @@ public Monster(MonsterSpawnArea spawnInfo, MonsterDefinition stats, GameMap map, | |
| : base(spawnInfo, stats, map, eventStateProvider, dropGenerator, plugInManager) | ||
| { | ||
| this._pathFinderPool = pathFinderPool; | ||
| this._walker = new Walker(this, () => this.StepDelay); | ||
| this._walker = new Walker(this, this.GetStepDelay); | ||
| this._intelligence = npcIntelligence; | ||
|
|
||
| (this._skillPowerUp, this._skillPowerUpDuration, this._skillPowerUpTarget) = this.CreateMagicEffectPowerUp(); | ||
|
|
@@ -87,7 +92,7 @@ public Monster(MonsterSpawnArea spawnInfo, MonsterDefinition stats, GameMap map, | |
| public Point WalkTarget => this._walker.CurrentTarget; | ||
|
|
||
| /// <inheritdoc/> | ||
| public TimeSpan StepDelay => this.Definition.MoveDelay; | ||
| public TimeSpan StepDelay => this.GetStepDelay(null); | ||
|
|
||
| /// <inheritdoc/> | ||
| /// <remarks>Monsters don't do combos.</remarks> | ||
|
|
@@ -356,6 +361,30 @@ private static WalkingStep GetStep(PathResultNode node) | |
| }; | ||
| } | ||
|
|
||
| private TimeSpan GetStepDelay(WalkingStep? step) | ||
| { | ||
| var tileDistance = step is { } walkingStep ? walkingStep.From.EuclideanDistanceTo(walkingStep.To) : 1.0; | ||
| var delayMilliseconds = this.Definition.MoveDelay.TotalMilliseconds * Math.Max(1.0, tileDistance); | ||
| delayMilliseconds /= this.GetMovementSpeedFactor(); | ||
|
|
||
| return TimeSpan.FromMilliseconds(delayMilliseconds); | ||
| } | ||
|
|
||
| private double GetMovementSpeedFactor() | ||
| { | ||
| if (this.MagicEffectList.ActiveEffects.ContainsKey(IcedEffectNumber)) | ||
| { | ||
| return IcedMovementSpeedFactor; | ||
| } | ||
|
|
||
| if (this.MagicEffectList.ActiveEffects.ContainsKey(BlowOfDestructionEffectNumber)) | ||
| { | ||
| return BlowOfDestructionMovementSpeedFactor; | ||
| } | ||
|
itayalroy marked this conversation as resolved.
|
||
|
|
||
| return 1.0; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates the magic effect power up for the given skill of a monster. | ||
| /// </summary> | ||
|
|
@@ -401,4 +430,4 @@ private void ValidatePath(Memory<WalkingStep> steps) | |
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -41,6 +42,17 @@ namespace MUnique.OpenMU.GameLogic; | |
| /// </summary> | ||
| public class Player : AsyncDisposable, IBucketMapObserver, IAttackable, IAttacker, ITrader, IPartyMember, IRotatable, IHasBucketInformation, ISupportWalk, IMovable, ILoggerOwner<Player> | ||
| { | ||
| private const short IcedEffectNumber = 0x38; | ||
| private const short BlowOfDestructionEffectNumber = 0x56; | ||
|
itayalroy marked this conversation as resolved.
itayalroy marked this conversation as resolved.
|
||
| private const double IcedMovementSpeedFactor = 0.5; | ||
| private const double BlowOfDestructionMovementSpeedFactor = 0.33; | ||
|
Comment on lines
+45
to
+48
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as for the monster class. |
||
| private const byte RunningGearMinimumLevel = 5; | ||
| private const ushort AtlansMapNumber = 7; | ||
| private const ushort Kalima1MapNumber = 24; | ||
| private const ushort Kalima6MapNumber = 29; | ||
| private const ushort Kalima7MapNumber = 36; | ||
| private const ushort Doppelgaenger3MapNumber = 67; | ||
|
|
||
| private static readonly MagicEffectDefinition GMEffect = new GMMagicEffectDefinition | ||
| { | ||
| InformObservers = true, | ||
|
|
@@ -148,7 +160,7 @@ public Player(IGameContext gameContext) | |
| public bool IsWalking => this._walker.CurrentTarget != default; | ||
|
|
||
| /// <inheritdoc /> | ||
| public TimeSpan StepDelay => this.GetStepDelay(); | ||
| public TimeSpan StepDelay => this.GetStepDelay(null); | ||
|
|
||
| /// <inheritdoc /> | ||
| public Point WalkTarget => this._walker.CurrentTarget; | ||
|
|
@@ -715,6 +727,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) | ||
| { | ||
|
|
@@ -1408,6 +1425,17 @@ public async ValueTask WalkToAsync(Point target, Memory<WalkingStep> steps) | |
| /// <inheritdoc /> | ||
| public ValueTask StopWalkingAsync() => this._walker.StopAsync(); | ||
|
|
||
| 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"/>. | ||
| /// </summary> | ||
|
|
@@ -2107,19 +2135,135 @@ private async ValueTask RegenerateHeroStateAsync() | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the step delay depending on the equipped items. | ||
| /// Gets the step delay depending on the equipped items and current movement effects. | ||
| /// </summary> | ||
| /// <param name="step">The walking step for which the delay is calculated.</param> | ||
| /// <returns>The current step delay, depending on equipped items.</returns> | ||
| private TimeSpan GetStepDelay() | ||
| private TimeSpan GetStepDelay(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; | ||
| if (this.IsInClientSafezone(position)) | ||
| { | ||
| return this.ApplyMovementEffects(walkSpeed); | ||
| } | ||
|
|
||
| return this.ApplyMovementEffects(this.GetMountedOrRunningSpeed(walkSpeed)); | ||
| } | ||
|
|
||
| private double ApplyMovementEffects(double speed) | ||
| { | ||
| if (this.MagicEffectList.ActiveEffects.ContainsKey(IcedEffectNumber)) | ||
| { | ||
| return speed * IcedMovementSpeedFactor; | ||
| } | ||
|
|
||
| if (this.MagicEffectList.ActiveEffects.ContainsKey(BlowOfDestructionEffectNumber)) | ||
| { | ||
| return speed * BlowOfDestructionMovementSpeedFactor; | ||
| } | ||
|
itayalroy marked this conversation as resolved.
|
||
|
|
||
| return speed; | ||
| } | ||
|
|
||
| private double GetMountedOrRunningSpeed(double walkSpeed) | ||
| { | ||
| const double runSpeed = 15.0; | ||
| const double fastWingSpeed = 16.0; | ||
| const double horseOrFenrirRunSpeed = 17.0; | ||
| const double excellentFenrirRunSpeed = 19.0; | ||
|
|
||
| var pet = this.Inventory?.GetItem(InventoryConstants.PetSlot); | ||
| if (this.IsItem(pet, 13, 37)) | ||
| { | ||
| // Wings | ||
| return TimeSpan.FromMilliseconds(300); | ||
| if (this.HasFenrirMovementOption(pet)) | ||
| { | ||
| return excellentFenrirRunSpeed; | ||
| } | ||
|
|
||
| return horseOrFenrirRunSpeed; | ||
| } | ||
|
|
||
| if (this.IsItem(pet, 13, 4)) | ||
| { | ||
| return horseOrFenrirRunSpeed; | ||
| } | ||
|
|
||
| var wings = this.Inventory?.GetItem(InventoryConstants.WingsSlot); | ||
| if (this.HasEquippedWings(wings) | ||
| || this.IsItem(pet, 13, 2) | ||
| || this.IsItem(pet, 13, 3)) | ||
| { | ||
| return this.GetWingMovementSpeed(wings, runSpeed, fastWingSpeed); | ||
| } | ||
|
|
||
| // TODO: Consider pets etc. | ||
| return TimeSpan.FromMilliseconds(500); | ||
| return this.HasRunningGear() ? runSpeed : walkSpeed; | ||
| } | ||
|
Comment on lines
+2180
to
+2212
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imho, there are too many hardcoded values and special logic. The item definitions could hold something like a movement speed, e.g. in new And about atlans/kalima: You could add another new |
||
|
|
||
| private double GetWingMovementSpeed(Item? wings, double runSpeed, double fastWingSpeed) | ||
| { | ||
| return this.IsFastWing(wings) ? fastWingSpeed : runSpeed; | ||
| } | ||
|
|
||
| private bool IsInClientSafezone(Point? position = null) | ||
| { | ||
| var checkedPosition = position ?? this.Position; | ||
| return this.CurrentMap?.Terrain.SafezoneMap[checkedPosition.X, checkedPosition.Y] ?? false; | ||
| } | ||
|
|
||
| private bool HasEquippedWings(Item? item) | ||
| { | ||
| return item is { Durability: > 0.0 } | ||
| && item.ItemSlot == InventoryConstants.WingsSlot; | ||
| } | ||
|
|
||
| private bool IsFastWing(Item? item) | ||
| { | ||
| return this.IsItem(item, 12, 5) | ||
| || this.IsItem(item, 12, 36); | ||
| } | ||
|
|
||
| private bool HasRunningGear() | ||
| { | ||
| var slot = this.IsSwimmingMovementMap() | ||
| ? InventoryConstants.GlovesSlot | ||
| : InventoryConstants.BootsSlot; | ||
| return this.Inventory?.GetItem(slot) is { Durability: > 0.0, Level: >= RunningGearMinimumLevel }; | ||
| } | ||
|
|
||
| private bool IsSwimmingMovementMap() | ||
| { | ||
| return this.CurrentMap?.MapId is AtlansMapNumber | ||
| or >= Kalima1MapNumber and <= Kalima6MapNumber | ||
| or Kalima7MapNumber | ||
| or Doppelgaenger3MapNumber; | ||
| } | ||
|
|
||
| private 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 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() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is recommended to use the
MagicEffectNumberenum instead of hardcodedshortconstants for better maintainability and to ensure consistency with the rest of the project.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MagicEffectNumber enum lives in Persistence.Initialization, inaccessible to GameLogic. I leave moving effect numbers to shared code to a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can check the iced-status by
this.AttributeSystem[Stats.IsIced].