diff --git a/src/GameLogic/DroppedMoney.cs b/src/GameLogic/DroppedMoney.cs index 7285d6a..f3372c0 100644 --- a/src/GameLogic/DroppedMoney.cs +++ b/src/GameLogic/DroppedMoney.cs @@ -80,51 +80,16 @@ public sealed class DroppedMoney : AsyncDisposable, ILocateable this._availableToPick = false; } - if (player.Party is { } party) + if (!this.TryGiveMoneyTo(player)) { - var partyMembers = party.PartyList - .OfType() - .Where(p => p.CurrentMap == player.CurrentMap && !p.IsAtSafezone() && p.Attributes is { }) - .ToList(); - - if (partyMembers.Count > 0) + // Nobody got the money, so the drop is released again. Keeping it claimed would leave it + // lying on the map, unpickable for everyone until it expires - and then lost. + using (await this._pickupLock.LockAsync()) { - var share = (int)(this.Amount / partyMembers.Count); - foreach (var member in partyMembers) - { - member.TryAddMoney((int)(share * member.Attributes![Stats.MoneyAmountRate])); - } + this._availableToPick = true; } - } - else - { - var clampMoneyOnPickup = player.GameContext?.Configuration?.ClampMoneyOnPickup ?? false; - if (clampMoneyOnPickup) - { - var maxMoney = player.GameContext?.Configuration?.MaximumInventoryMoney ?? int.MaxValue; - var currentMoney = player.Money; - var amountToAdd = (int)Math.Min(this.Amount, (uint)Math.Max(0, maxMoney - currentMoney)); - if (amountToAdd <= 0) - { - player.Logger.LogDebug("Player is at maximum money limit, Player {0}, Money {1}", player, this); - return false; - } - - if (!player.TryAddMoney(amountToAdd)) - { - player.Logger.LogDebug("Money could not be added to the inventory, Player {0}, Money {1}", player, this); - return false; - } - } - else - { - if (!player.TryAddMoney((int)this.Amount)) - { - player.Logger.LogDebug("Money could not be added to the inventory, Player {0}, Money {1}", player, this); - return false; - } - } + return false; } await this.DisposeAsync().ConfigureAwait(false); @@ -158,6 +123,64 @@ public sealed class DroppedMoney : AsyncDisposable, ILocateable await base.DisposeAsyncCore().ConfigureAwait(false); } + /// + /// Tries to hand the money over to the player, or to its party. Returns false if it could not be + /// given to anyone, e.g. because the receiver is already at the maximum inventory money. + /// + /// The player which picks the money up. + /// True, if at least one player received money; Otherwise, false. + private bool TryGiveMoneyTo(Player player) + { + if (player.Party is { } party) + { + var partyMembers = party.PartyList + .OfType() + .Where(p => p.CurrentMap == player.CurrentMap && !p.IsAtSafezone() && p.Attributes is { }) + .ToList(); + + if (partyMembers.Count == 0) + { + player.Logger.LogDebug("No party member could receive the money, Player {0}, Money {1}", player, this); + return false; + } + + var share = (int)(this.Amount / partyMembers.Count); + var received = false; + foreach (var member in partyMembers) + { + received |= member.TryAddMoney((int)(share * member.Attributes![Stats.MoneyAmountRate])); + } + + if (!received) + { + player.Logger.LogDebug("No party member could take the money, Player {0}, Money {1}", player, this); + } + + return received; + } + + var amountToAdd = (int)this.Amount; + if (player.GameContext?.Configuration?.ClampMoneyOnPickup ?? false) + { + var maxMoney = player.GameContext?.Configuration?.MaximumInventoryMoney ?? int.MaxValue; + amountToAdd = (int)Math.Min(this.Amount, (uint)Math.Max(0, maxMoney - player.Money)); + + if (amountToAdd <= 0) + { + player.Logger.LogDebug("Player is at maximum money limit, Player {0}, Money {1}", player, this); + return false; + } + } + + if (!player.TryAddMoney(amountToAdd)) + { + player.Logger.LogDebug("Money could not be added to the inventory, Player {0}, Money {1}", player, this); + return false; + } + + return true; + } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "VSTHRD100:Avoid async void methods", Justification = "Catching all Exceptions.")] private async void OnTimerTimeout(object? state) { diff --git a/tests/MUnique.OpenMU.Tests/DroppedMoneyTest.cs b/tests/MUnique.OpenMU.Tests/DroppedMoneyTest.cs new file mode 100644 index 0000000..97b1010 --- /dev/null +++ b/tests/MUnique.OpenMU.Tests/DroppedMoneyTest.cs @@ -0,0 +1,65 @@ +// +// Licensed under the MIT License. See LICENSE file in the project root for full license information. +// + +namespace MUnique.OpenMU.Tests; + +using MUnique.OpenMU.GameLogic; +using MUnique.OpenMU.Pathfinding; +using NUnit.Framework; + +/// +/// Tests for the . +/// +[TestFixture] +public class DroppedMoneyTest +{ + private const uint DroppedAmount = 1000; + + /// + /// Tests that a failed pick up doesn't consume the drop, so it's still available for the next player. + /// A player at the maximum inventory money can't take it, and used to claim it nevertheless - which left + /// it lying on the map, unpickable for everyone until it expired. + /// + [Test] + public async Task FailedPickUpKeepsMoneyAvailableAsync() + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + player.GameContext.Configuration.MaximumInventoryMoney = int.MaxValue; + var maximumMoney = player.GameContext.Configuration.MaximumInventoryMoney; + player.Money = maximumMoney; + + var money = new DroppedMoney(DroppedAmount, new Point(100, 100), player.CurrentMap!); + + Assert.That(await money.TryPickUpByAsync(player).ConfigureAwait(false), Is.False); + Assert.That(player.Money, Is.EqualTo(maximumMoney)); + + var otherPlayer = await PlayerTestHelper.CreatePlayerAsync(player.GameContext).ConfigureAwait(false); + otherPlayer.Money = 0; + + Assert.That(await money.TryPickUpByAsync(otherPlayer).ConfigureAwait(false), Is.True); + Assert.That(otherPlayer.Money, Is.EqualTo((int)DroppedAmount)); + } + + /// + /// Tests that a successful pick up still consumes the drop, so it can't be picked up twice. + /// + [Test] + public async Task SuccessfulPickUpConsumesMoneyAsync() + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + player.GameContext.Configuration.MaximumInventoryMoney = int.MaxValue; + player.Money = 0; + + var money = new DroppedMoney(DroppedAmount, new Point(100, 100), player.CurrentMap!); + + Assert.That(await money.TryPickUpByAsync(player).ConfigureAwait(false), Is.True); + Assert.That(player.Money, Is.EqualTo((int)DroppedAmount)); + + var otherPlayer = await PlayerTestHelper.CreatePlayerAsync(player.GameContext).ConfigureAwait(false); + otherPlayer.Money = 0; + + Assert.That(await money.TryPickUpByAsync(otherPlayer).ConfigureAwait(false), Is.False); + Assert.That(otherPlayer.Money, Is.EqualTo(0)); + } +}