Derive each player's zen from the experience they actually gained

The money amount of a monster drop is computed as "gained experience + 7", but
in a party it was neither derived from, nor distributed like, that experience.

DistributeExperienceAfterKillAsync returned a party total without the game rate
and the personal experience rates, while AddExpAfterKillAsync returned a value
which had them applied. Both landed in the same variable in OnDeathAsync, so the
party number was orders of magnitude smaller. That mismatch was worked around by
recalculating the killer's solo experience for money purposes, which pinned the
party pool to a solo-sized amount: with the pool then split by member count, a
party of three received about a third of the solo zen each, while its experience
pool grows with the party size.

AwardExperienceAsync already computes the per member experience with all rates
applied, it just discarded it. It now returns that value, the distribution
returns the per member breakdown, and the money is split proportionally to it.
The workaround is gone, the pool follows the experience, and each member's zen
matches their own level, their own rates and their own master/normal branch. The
units lost to the integer division are handed to the shares which were cut the
most, one each, so no member is systematically favoured over many kills.

Two defects around the money rate are fixed on the way:

- MoneyAmountRate was applied for the killer when the drop was created and again
  for the receiver when a party picked it up, so a rate of 3.0 paid a party 9x.
  On the "money straight into the inventory" path for a solo killer it was not
  applied at all. It is now applied exactly once, for the receiver.
- ClampMoneyOnPickup was only honoured for solo pick ups. A party member at
  MaximumInventoryMoney silently lost their share, because the drop was consumed
  as soon as any other member could take one. The clamp now runs on the shared
  payout path, after the money rate, so it clamps the amount actually credited.

Money without a per player breakdown, such as the fixed amount of an item box,
keeps being split equally. Shares of players who are no longer eligible are
redistributed among the remaining ones instead of being lost.

(cherry picked from commit 53f25aca08e690255571f2bb6c796bb7c10934a2)

src/GameLogic/DroppedMoney.cs

@@ -6,7 +6,6 @@ namespace MUnique.OpenMU.GameLogic;
 
 using System.Diagnostics;
 using System.Threading;
-using MUnique.OpenMU.GameLogic.Attributes;
 using MUnique.OpenMU.Pathfinding;
 using Nito.AsyncEx;
 
@@ -20,6 +19,8 @@ public sealed class DroppedMoney : AsyncDisposable, ILocateable
     /// </summary>
     private readonly AsyncLock _pickupLock;
 
+    private readonly IReadOnlyList<MoneyShare> _shares;
+
     private Timer? _removeTimer;
 
     private bool _availableToPick = true;
@@ -30,9 +31,14 @@ public sealed class DroppedMoney : AsyncDisposable, ILocateable
     /// <param name="amount">The amount.</param>
     /// <param name="position">The position where the item was dropped on the map.</param>
     /// <param name="map">The map.</param>
-    public DroppedMoney(uint amount, Point position, GameMap map)
+    /// <param name="shares">
+    /// The part of the money which is reserved for each player, matching the experience they gained from the kill.
+    /// When it's empty - for example for money from an item box - the money is split equally instead.
+    /// </param>
+    public DroppedMoney(uint amount, Point position, GameMap map, IReadOnlyList<MoneyShare>? shares = null)
     {
         this.Amount = amount;
+        this._shares = shares ?? [];
         this._pickupLock = new();
         this.Position = position;
         this.CurrentMap = map;
@@ -131,54 +137,28 @@ public sealed class DroppedMoney : AsyncDisposable, ILocateable
     /// <returns><c>True</c>, if at least one player received money; Otherwise, <c>false</c>.</returns>
     private bool TryGiveMoneyTo(Player player)
     {
-        if (player.Party is { } party)
+        if (player.Party is not { } party)
         {
-            var partyMembers = party.PartyList
-                .OfType<Player>()
-                .Where(p => p.CurrentMap == player.CurrentMap && !p.IsAtSafezone() && p.Attributes is { })
-                .ToList();
-
-            if (partyMembers.Count == 0)
+            if (!MoneyDistribution.TryPay(player, this.Amount))
             {
-                player.Logger.LogDebug("No party member could receive the money, Player {0}, Money {1}", player, this);
+                player.Logger.LogDebug("Money could not be added to the inventory, 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;
+            return true;
         }
 
-        var amountToAdd = (int)this.Amount;
-        if (player.GameContext?.Configuration?.ClampMoneyOnPickup ?? false)
+        var shares = this._shares.Count > 0
+            ? this._shares
+            : MoneyDistribution.CreateEqualShares(this.Amount, party.PartyList.OfType<Player>().ToList());
+
+        var received = MoneyDistribution.TryPayShares(shares, member => party.IsEligibleForMoney(member, player));
+        if (!received)
         {
-            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;
-            }
+            player.Logger.LogDebug("No party member could take the money, Player {0}, Money {1}", player, this);
         }
 
-        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;
+        return received;
     }
 
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "VSTHRD100:Avoid async void methods", Justification = "Catching all Exceptions.")]

src/GameLogic/ExperienceShare.cs

@@ -0,0 +1,12 @@
+// <copyright file="ExperienceShare.cs" company="MUnique">
+// Licensed under the MIT License. See LICENSE file in the project root for full license information.
+// </copyright>
+
+namespace MUnique.OpenMU.GameLogic;
+
+/// <summary>
+/// The experience which a single player gained from a kill.
+/// </summary>
+/// <param name="Player">The player which gained the experience.</param>
+/// <param name="Experience">The gained experience, with all experience rates already applied.</param>
+public readonly record struct ExperienceShare(Player Player, int Experience);

src/GameLogic/MoneyDistribution.cs

@@ -0,0 +1,223 @@
+// <copyright file="MoneyDistribution.cs" company="MUnique">
+// Licensed under the MIT License. See LICENSE file in the project root for full license information.
+// </copyright>
+
+namespace MUnique.OpenMU.GameLogic;
+
+using MUnique.OpenMU.GameLogic.Attributes;
+
+/// <summary>
+/// Splits a money drop between players and hands it over to them.
+/// </summary>
+/// <remarks>
+/// This is the single place where <see cref="Stats.MoneyAmountRate"/> and
+/// <see cref="DataModel.Configuration.GameConfiguration.ClampMoneyOnPickup"/> are applied, so that
+/// every path - dropped on the ground, added directly, solo or in a party - treats them the same way.
+/// </remarks>
+internal static class MoneyDistribution
+{
+    /// <summary>
+    /// Splits the money proportionally to the experience each player gained from the kill.
+    /// </summary>
+    /// <param name="amount">The total amount of money to split.</param>
+    /// <param name="experienceShares">The experience gained per player.</param>
+    /// <returns>The part of the money which is reserved for each player.</returns>
+    public static IReadOnlyList<MoneyShare> CreateShares(uint amount, IReadOnlyList<ExperienceShare> experienceShares)
+    {
+        if (experienceShares.Count == 0)
+        {
+            return [];
+        }
+
+        if (experienceShares.Count == 1)
+        {
+            return [new MoneyShare(experienceShares[0].Player, amount)];
+        }
+
+        var weights = new long[experienceShares.Count];
+        for (int i = 0; i < experienceShares.Count; i++)
+        {
+            weights[i] = experienceShares[i].Experience;
+        }
+
+        var parts = SplitByWeight(amount, weights);
+        var shares = new MoneyShare[experienceShares.Count];
+        for (int i = 0; i < experienceShares.Count; i++)
+        {
+            shares[i] = new MoneyShare(experienceShares[i].Player, parts[i]);
+        }
+
+        return shares;
+    }
+
+    /// <summary>
+    /// Splits the money equally, which is used when no per player experience is known - for example
+    /// for the fixed money amount of an item box.
+    /// </summary>
+    /// <param name="amount">The total amount of money to split.</param>
+    /// <param name="players">The players to split it between.</param>
+    /// <returns>The part of the money which is reserved for each player.</returns>
+    public static IReadOnlyList<MoneyShare> CreateEqualShares(uint amount, IReadOnlyList<Player> players)
+    {
+        if (players.Count == 0)
+        {
+            return [];
+        }
+
+        var parts = SplitByWeight(amount, new long[players.Count]);
+        var shares = new MoneyShare[players.Count];
+        for (int i = 0; i < players.Count; i++)
+        {
+            shares[i] = new MoneyShare(players[i], parts[i]);
+        }
+
+        return shares;
+    }
+
+    /// <summary>
+    /// Hands the shares over to the players which are still eligible. The shares of players which
+    /// are not eligible anymore are re-distributed between the remaining ones.
+    /// </summary>
+    /// <param name="shares">The shares.</param>
+    /// <param name="isEligible">Determines whether a player may still receive their share.</param>
+    /// <returns><c>True</c>, if at least one player received money; Otherwise, <c>false</c>.</returns>
+    public static bool TryPayShares(IReadOnlyList<MoneyShare> shares, Func<Player, bool> isEligible)
+    {
+        var eligible = new List<MoneyShare>(shares.Count);
+        uint forfeited = 0;
+        foreach (var share in shares)
+        {
+            if (isEligible(share.Player))
+            {
+                eligible.Add(share);
+            }
+            else
+            {
+                forfeited += share.Amount;
+            }
+        }
+
+        if (eligible.Count == 0)
+        {
+            return false;
+        }
+
+        var extra = new uint[eligible.Count];
+        if (forfeited > 0)
+        {
+            var weights = new long[eligible.Count];
+            for (int i = 0; i < eligible.Count; i++)
+            {
+                weights[i] = eligible[i].Amount;
+            }
+
+            extra = SplitByWeight(forfeited, weights);
+        }
+
+        var received = false;
+        for (int i = 0; i < eligible.Count; i++)
+        {
+            received |= TryPay(eligible[i].Player, eligible[i].Amount + extra[i]);
+        }
+
+        return received;
+    }
+
+    /// <summary>
+    /// Adds the money to the inventory of the player, applying their <see cref="Stats.MoneyAmountRate"/>
+    /// and the configured pick up clamp.
+    /// </summary>
+    /// <param name="player">The player which should receive the money.</param>
+    /// <param name="amount">The amount, before the money rate of the player is applied.</param>
+    /// <returns><c>True</c>, if the player received money; Otherwise, <c>false</c>.</returns>
+    public static bool TryPay(Player player, uint amount)
+    {
+        if (amount == 0)
+        {
+            return false;
+        }
+
+        // The rate is applied in double precision: a float multiplication would round money amounts
+        // above the ~16.7M the float mantissa can represent exactly, before the cast to long.
+        var scaled = (long)(amount * (double)(player.Attributes?[Stats.MoneyAmountRate] ?? 1.0f));
+        if (scaled <= 0)
+        {
+            return false;
+        }
+
+        var amountToAdd = (int)Math.Min(scaled, int.MaxValue);
+        if (player.GameContext?.Configuration?.ClampMoneyOnPickup ?? false)
+        {
+            var maximumMoney = player.GameContext?.Configuration?.MaximumInventoryMoney ?? int.MaxValue;
+            amountToAdd = (int)Math.Min(amountToAdd, Math.Max(0, maximumMoney - player.Money));
+            if (amountToAdd <= 0)
+            {
+                return false;
+            }
+        }
+
+        return player.TryAddMoney(amountToAdd);
+    }
+
+    /// <summary>
+    /// Splits an amount proportionally to the given weights. When all weights are zero, it's split equally.
+    /// </summary>
+    private static uint[] SplitByWeight(uint amount, IReadOnlyList<long> weights)
+    {
+        var result = new uint[weights.Count];
+        if (weights.Count == 0 || amount == 0)
+        {
+            return result;
+        }
+
+        long totalWeight = 0;
+        foreach (var weight in weights)
+        {
+            totalWeight += Math.Max(0, weight);
+        }
+
+        var remainders = new long[weights.Count];
+        uint distributed = 0;
+        for (int i = 0; i < weights.Count; i++)
+        {
+            if (totalWeight > 0)
+            {
+                var numerator = (long)amount * Math.Max(0, weights[i]);
+                result[i] = (uint)(numerator / totalWeight);
+                remainders[i] = numerator % totalWeight;
+            }
+            else
+            {
+                result[i] = amount / (uint)weights.Count;
+                remainders[i] = 1;
+            }
+
+            distributed += result[i];
+        }
+
+        // The integer division loses up to one unit per share. Handing all of it to the same
+        // share would systematically favour one player over many kills, so the units go to the
+        // shares which were cut the most, one each (largest remainder method).
+        for (var rest = amount - distributed; rest > 0; rest--)
+        {
+            var pick = -1;
+            for (int i = 0; i < remainders.Length; i++)
+            {
+                if (remainders[i] > 0 && (pick < 0 || remainders[i] > remainders[pick]))
+                {
+                    pick = i;
+                }
+            }
+
+            if (pick < 0)
+            {
+                break;
+            }
+
+            result[pick]++;
+            remainders[pick] = 0;
+        }
+
+        return result;
+    }
+}

src/GameLogic/MoneyShare.cs

@@ -0,0 +1,12 @@
+// <copyright file="MoneyShare.cs" company="MUnique">
+// Licensed under the MIT License. See LICENSE file in the project root for full license information.
+// </copyright>
+
+namespace MUnique.OpenMU.GameLogic;
+
+/// <summary>
+/// The part of a money drop which is reserved for a single player.
+/// </summary>
+/// <param name="Player">The player for which the part is reserved.</param>
+/// <param name="Amount">The amount of money, before the <see cref="Attributes.Stats.MoneyAmountRate"/> of the player is applied.</param>
+public readonly record struct MoneyShare(Player Player, uint Amount);

src/GameLogic/NPC/AttackableNpcBase.cs

@@ -323,7 +323,9 @@ public abstract class AttackableNpcBase : NonPlayerCharacter, IAttackable
         var player = this.GetHitNotificationTarget(attacker);
         if (player is { })
         {
-            int exp = await (player.Party?.DistributeExperienceAfterKillAsync(this, player) ?? player.AddExpAfterKillAsync(this)).ConfigureAwait(false);
+            var experienceShares = player.Party is { } party
+                ? await party.DistributeExperienceAfterKillAsync(this, player).ConfigureAwait(false)
+                : [new ExperienceShare(player, await player.AddExpAfterKillAsync(this).ConfigureAwait(false))];
             if (attacker == player)
             {
                 await player.AfterKilledMonsterAsync().ConfigureAwait(false);
@@ -341,7 +343,7 @@ public abstract class AttackableNpcBase : NonPlayerCharacter, IAttackable
                     selectedCharacter.StateRemainingSeconds -= (int)this.Attributes[Stats.Level];
                 }
 
-                _ = this.DropItemDelayedAsync(player, exp); // don't wait for completion.
+                _ = this.DropItemDelayedAsync(player, experienceShares); // don't wait for completion.
             }
         }
     }
@@ -401,46 +403,44 @@ public abstract class AttackableNpcBase : NonPlayerCharacter, IAttackable
         }
     }
 
-    private async ValueTask HandleMoneyDropAsync(uint amount, Player killer)
+    private async ValueTask HandleMoneyDropAsync(uint amount, Player killer, IReadOnlyList<ExperienceShare> experienceShares)
     {
+        // Each player gets the part of the money which matches the experience they gained from the kill,
+        // so that money follows the same distribution as the experience it is derived from.
+        var shares = MoneyDistribution.CreateShares(amount, experienceShares);
+
         // We don't drop money in Devil Square, etc.
         var shouldDropMoney = killer.GameContext.Configuration.ShouldDropMoney && killer.CurrentMiniGame is null;
         if (!shouldDropMoney)
         {
-            var party = killer.Party;
-            if (party is null)
+            if (killer.Party is { } party)
             {
-                killer.TryAddMoney((int)amount);
+                await party.DistributeMoneyAfterKillAsync(this, killer, shares).ConfigureAwait(false);
             }
             else
             {
-                await party.DistributeMoneyAfterKillAsync(this, killer, amount).ConfigureAwait(false);
+                _ = MoneyDistribution.TryPay(killer, amount);
             }
 
             return;
         }
 
-        var droppedMoney = new DroppedMoney((uint)(amount * (killer.Attributes?[Stats.MoneyAmountRate] ?? 1.0f)), this.Position, this.CurrentMap);
+        var droppedMoney = new DroppedMoney(amount, this.Position, this.CurrentMap, shares);
         await this.CurrentMap.AddAsync(droppedMoney).ConfigureAwait(false);
     }
 
-    private async ValueTask DropItemAsync(int exp, Player killer)
+    private async ValueTask DropItemAsync(IReadOnlyList<ExperienceShare> experienceShares, Player killer)
     {
-        // When the killer is in a party, DistributeExperienceAfterKillAsync returns a
-        // total party experience that does NOT include game rate (ExperienceRate) or
-        // personal experience rate multipliers. Since the money drop amount is
-        // derived from this experience value, party money drops were dramatically
-        // lower than solo drops. We recalculate the experience for money purposes
-        // using the solo formula so money is consistent regardless of party state.
-        if (killer.Party is not null)
+        var exp = 0;
+        foreach (var share in experienceShares)
         {
-            exp = killer.CalculateExpAfterKill(this);
+            exp += share.Experience;
         }
 
         var (generatedItems, droppedMoney) = await this._dropGenerator.GenerateItemDropsAsync(this.Definition, exp, killer).ConfigureAwait(false);
         if (droppedMoney > 0)
         {
-            await this.HandleMoneyDropAsync(droppedMoney.Value, killer).ConfigureAwait(false);
+            await this.HandleMoneyDropAsync(droppedMoney.Value, killer, experienceShares).ConfigureAwait(false);
         }
 
         var firstItem = !droppedMoney.HasValue;
@@ -463,12 +463,12 @@ public abstract class AttackableNpcBase : NonPlayerCharacter, IAttackable
         }
     }
 
-    private async ValueTask DropItemDelayedAsync(Player player, int gainedExp)
+    private async ValueTask DropItemDelayedAsync(Player player, IReadOnlyList<ExperienceShare> experienceShares)
     {
         try
         {
             await Task.Delay(1000).ConfigureAwait(false);
-            await this.DropItemAsync(gainedExp, player).ConfigureAwait(false);
+            await this.DropItemAsync(experienceShares, player).ConfigureAwait(false);
         }
         catch (Exception ex)
         {

src/GameLogic/Party.cs

@@ -201,8 +201,8 @@ public sealed class Party : AsyncDisposable
     /// </summary>
     /// <param name="killedObject">The object that was killed.</param>
     /// <param name="killer">The killer who is a party member.</param>
-    /// <returns>The total experience distributed.</returns>
-    public async ValueTask<int> DistributeExperienceAfterKillAsync(IAttackable killedObject, IObservable killer)
+    /// <returns>The experience which each party member gained, with all experience rates applied.</returns>
+    public async ValueTask<IReadOnlyList<ExperienceShare>> DistributeExperienceAfterKillAsync(IAttackable killedObject, IObservable killer)
     {
         using var l = await this._distributionLock.LockAsync();
         try
@@ -220,34 +220,29 @@ public sealed class Party : AsyncDisposable
     /// </summary>
     /// <param name="killed">The object that was killed.</param>
     /// <param name="killer">The killer who is a party member.</param>
-    /// <param name="amount">The amount of money to distribute.</param>
-    public async ValueTask DistributeMoneyAfterKillAsync(IAttackable killed, IPartyMember killer, uint amount)
+    /// <param name="shares">The part of the money which is reserved for each party member.</param>
+    public ValueTask DistributeMoneyAfterKillAsync(IAttackable killed, IPartyMember killer, IReadOnlyList<MoneyShare> shares)
     {
-        using var l = await this._distributionLock.LockAsync();
-        try
-        {
-            this._logger.LogDebug("Distributing money after killing {name}", killed.GetName());
-            this._distributionList.AddRange(
-                this._partyMembers.OfType<Player>()
-                    .Where(p => p.CurrentMap == killer.CurrentMap
-                                && !p.IsAtSafezone()
-                                && p.Attributes is { }));
+        // No lock is taken here: unlike the experience distribution this no longer touches the shared
+        // _distributionList, and paying out the pre-computed shares is consistent with the lock-free
+        // pick up path in DroppedMoney.
+        this._logger.LogDebug("Distributing money after killing {name}", killed.GetName());
+        _ = MoneyDistribution.TryPayShares(shares, player => this.IsEligibleForMoney(player, killer));
+        return ValueTask.CompletedTask;
+    }
 
-            if (this._distributionList.Count == 0)
-            {
-                return;
-            }
-
-            var moneyPart = amount / this._distributionList.Count;
-            foreach (var player in this._distributionList)
-            {
-                player.TryAddMoney((int)(moneyPart * player.Attributes![Stats.MoneyAmountRate]));
-            }
-        }
-        finally
-        {
-            this._distributionList.Clear();
-        }
+    /// <summary>
+    /// Determines whether the player may receive a part of a money drop of the party.
+    /// </summary>
+    /// <param name="player">The player.</param>
+    /// <param name="killer">The killer who is a party member.</param>
+    /// <returns><c>True</c>, if the player may receive money; Otherwise, <c>false</c>.</returns>
+    internal bool IsEligibleForMoney(Player player, IPartyMember killer)
+    {
+        return this._partyMembers.Contains(player)
+               && player.CurrentMap == killer.CurrentMap
+               && !player.IsAtSafezone()
+               && player.Attributes is { };
     }
 
     /// <summary>
@@ -340,7 +335,7 @@ public sealed class Party : AsyncDisposable
         base.Dispose(disposing);
     }
 
-    private static (int Total, float PerLevel) CalculatePartyExperience(List<Player> recipients, IAttackable killed)
+    private static float CalculatePartyExperiencePerLevel(List<Player> recipients, IAttackable killed)
     {
         var memberCount = recipients.Count;
         var totalLevel = recipients.Sum(p => (int)p.Attributes![Stats.TotalLevel]);
@@ -355,9 +350,8 @@ public sealed class Party : AsyncDisposable
         var randomMinMultiplier = attributes[Stats.RandomExperienceMinMultiplier];
         var randomMaxMultiplier = attributes[Stats.RandomExperienceMaxMultiplier];
         var totalExperience = CalculateTotalExperience(totalBaseExperience, randomMinMultiplier, randomMaxMultiplier);
-        var perLevel = (float)totalExperience / totalLevel;
 
-        return (totalExperience, perLevel);
+        return (float)totalExperience / totalLevel;
     }
 
     private static int CalculateTotalExperience(double totalBaseExperience, float randomMinMultiplier, float randomMaxMultiplier)
@@ -377,7 +371,7 @@ public sealed class Party : AsyncDisposable
         return (int)totalBaseExperience;
     }
 
-    private static async ValueTask AwardExperienceAsync(Player player, float perLevel, IAttackable killed)
+    private static async ValueTask<int> AwardExperienceAsync(Player player, float perLevel, IAttackable killed)
     {
         var attributes = player.Attributes!;
         var isAtMaxLevel = (short)attributes[Stats.Level] == player.GameContext.Configuration.MaximumLevel;
@@ -391,8 +385,10 @@ public sealed class Party : AsyncDisposable
                             * (attributes[Stats.MasterExperienceRate] + attributes[Stats.BonusExperienceRate]));
 
             await player.AddMasterExperienceAsync(exp, killed).ConfigureAwait(false);
+            return exp;
         }
-        else if (!isAtMaxLevel)
+
+        if (!isAtMaxLevel)
         {
             var exp = (int)(perLevel
                             * attributes[Stats.Level]
@@ -400,11 +396,11 @@ public sealed class Party : AsyncDisposable
                             * (attributes[Stats.ExperienceRate] + attributes[Stats.BonusExperienceRate]));
 
             await player.AddExperienceAsync(exp, killed).ConfigureAwait(false);
+            return exp;
         }
-        else
-        {
-            // Player is at max level but has not completed master quest. No experience awarded.
-        }
+
+        // Player is at max level but has not completed master quest. No experience awarded.
+        return 0;
     }
 
     private async ValueTask ExitPartyAsync(IPartyMember member, byte index)
@@ -466,11 +462,11 @@ public sealed class Party : AsyncDisposable
         }
     }
 
-    private async ValueTask<int> InternalDistributeExperienceAfterKillAsync(IAttackable killedObject, IObservable killer)
+    private async ValueTask<IReadOnlyList<ExperienceShare>> InternalDistributeExperienceAfterKillAsync(IAttackable killedObject, IObservable killer)
     {
         if (killedObject.IsSummonedMonster)
         {
-            return 0;
+            return [];
         }
 
         using (await killer.ObserverLock.ReaderLockAsync().ConfigureAwait(false))
@@ -483,17 +479,20 @@ public sealed class Party : AsyncDisposable
 
         if (this._distributionList.Count == 0)
         {
-            return 0;
+            return [];
         }
 
-        var (total, perLevel) = CalculatePartyExperience(this._distributionList, killedObject);
+        var perLevel = CalculatePartyExperiencePerLevel(this._distributionList, killedObject);
 
+        // The shares are copied into their own list, because _distributionList is reused and cleared by the caller.
+        var shares = new List<ExperienceShare>(this._distributionList.Count);
         foreach (var player in this._distributionList)
         {
-            await AwardExperienceAsync(player, perLevel, killedObject).ConfigureAwait(false);
+            var experience = await AwardExperienceAsync(player, perLevel, killedObject).ConfigureAwait(false);
+            shares.Add(new ExperienceShare(player, experience));
         }
 
-        return total;
+        return shares;
     }
 
     private async ValueTask UpdateNearbyCountAsync()

tests/MUnique.OpenMU.GameLogic.Benchmarks/PartyBenchmarks.cs

@@ -71,7 +71,8 @@ public class PartyBenchmarks
     [Benchmark]
     public async ValueTask DistributeMoneyAfterKillAsync()
     {
-        await _party.DistributeMoneyAfterKillAsync(_killedObject, _killer, 10000);
+        var shares = _players.Select(p => new MoneyShare(p, 10000 / (uint)_players.Count)).ToList();
+        await _party.DistributeMoneyAfterKillAsync(_killedObject, _killer, shares);
     }
 
     /// <summary>

tests/MUnique.OpenMU.Tests/MoneyDistributionTest.cs

@@ -0,0 +1,266 @@
+// <copyright file="MoneyDistributionTest.cs" company="MUnique">
+// Licensed under the MIT License. See LICENSE file in the project root for full license information.
+// </copyright>
+
+namespace MUnique.OpenMU.Tests;
+
+using Microsoft.Extensions.Logging.Abstractions;
+using MUnique.OpenMU.AttributeSystem;
+using MUnique.OpenMU.GameLogic;
+using MUnique.OpenMU.GameLogic.Attributes;
+using MUnique.OpenMU.Pathfinding;
+using NUnit.Framework;
+
+/// <summary>
+/// Tests for the distribution of a money drop between the players which earned it.
+/// </summary>
+[TestFixture]
+public class MoneyDistributionTest
+{
+    private static readonly Point DropPosition = new(100, 100);
+
+    /// <summary>
+    /// Tests that the money is split proportionally to the experience each player gained,
+    /// because the money amount is derived from that experience.
+    /// </summary>
+    [Test]
+    public async Task SharesFollowExperienceProportionsAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+
+        var shares = MoneyDistribution.CreateShares(1000, [new ExperienceShare(first, 800), new ExperienceShare(second, 200)]);
+
+        Assert.That(shares.Count, Is.EqualTo(2));
+        Assert.That(shares[0].Amount, Is.EqualTo(800u));
+        Assert.That(shares[1].Amount, Is.EqualTo(200u));
+    }
+
+    /// <summary>
+    /// Tests that the remainder of the integer division is not lost, so the shares always add up
+    /// to the dropped amount.
+    /// </summary>
+    [Test]
+    public async Task SharesAddUpToTheDroppedAmountAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        var third = await PlayerTestHelper.CreatePlayerAsync(first.GameContext).ConfigureAwait(false);
+
+        var shares = MoneyDistribution.CreateShares(10, [new ExperienceShare(first, 1), new ExperienceShare(second, 1), new ExperienceShare(third, 1)]);
+
+        Assert.That(shares.Sum(s => (long)s.Amount), Is.EqualTo(10));
+    }
+
+    /// <summary>
+    /// Tests that the units lost to the integer division are spread over the shares which were cut
+    /// the most, instead of always landing on the same player - which would favour them over many kills.
+    /// </summary>
+    [Test]
+    public async Task RemainderIsSpreadInsteadOfAlwaysGoingToTheSamePlayerAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        var third = await PlayerTestHelper.CreatePlayerAsync(first.GameContext).ConfigureAwait(false);
+
+        // 52 split between three equal contributors: 17 each leaves a remainder of 1.
+        var shares = MoneyDistribution.CreateShares(52, [new ExperienceShare(first, 9), new ExperienceShare(second, 9), new ExperienceShare(third, 9)]);
+
+        Assert.That(shares.Sum(s => (long)s.Amount), Is.EqualTo(52));
+        Assert.That(shares.Max(s => s.Amount) - shares.Min(s => s.Amount), Is.LessThanOrEqualTo(1u));
+    }
+
+    /// <summary>
+    /// Tests that a player who gained no experience from the kill gets no money from it.
+    /// </summary>
+    [Test]
+    public async Task PlayerWithoutExperienceGetsNoShareAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+
+        var shares = MoneyDistribution.CreateShares(8, [new ExperienceShare(first, 0), new ExperienceShare(second, 1)]);
+
+        Assert.That(shares[0].Amount, Is.EqualTo(0u));
+        Assert.That(shares[1].Amount, Is.EqualTo(8u));
+    }
+
+    /// <summary>
+    /// Tests that a player without a party gets the money multiplied by their money rate.
+    /// </summary>
+    [Test]
+    public async Task SoloPickUpAppliesMoneyRateAsync()
+    {
+        var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false);
+        player.GameContext.Configuration.MaximumInventoryMoney = int.MaxValue;
+        player.Money = 0;
+        SetMoneyRate(player, 2.0f);
+
+        var money = new DroppedMoney(100, DropPosition, player.CurrentMap!);
+
+        Assert.That(await money.TryPickUpByAsync(player).ConfigureAwait(false), Is.True);
+        Assert.That(player.Money, Is.EqualTo(200));
+    }
+
+    /// <summary>
+    /// Tests that the pick up clamp is calculated on the amount *after* the money rate was applied.
+    /// Clamping the raw amount would let the player exceed the maximum inventory money.
+    /// </summary>
+    [Test]
+    public async Task SoloPickUpClampsAfterApplyingMoneyRateAsync()
+    {
+        var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false);
+        player.GameContext.Configuration.MaximumInventoryMoney = 150;
+        player.GameContext.Configuration.ClampMoneyOnPickup = true;
+        player.Money = 0;
+        SetMoneyRate(player, 2.0f);
+
+        var money = new DroppedMoney(100, DropPosition, player.CurrentMap!);
+
+        Assert.That(await money.TryPickUpByAsync(player).ConfigureAwait(false), Is.True);
+        Assert.That(player.Money, Is.EqualTo(150));
+    }
+
+    /// <summary>
+    /// Tests that every party member receives exactly the share which was reserved for them.
+    /// </summary>
+    [Test]
+    public async Task PartyPickUpPaysReservedSharesAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        await CreatePartyAsync(first, second).ConfigureAwait(false);
+
+        var money = new DroppedMoney(1000, DropPosition, first.CurrentMap!, [new MoneyShare(first, 800), new MoneyShare(second, 200)]);
+
+        Assert.That(await money.TryPickUpByAsync(first).ConfigureAwait(false), Is.True);
+        Assert.That(first.Money, Is.EqualTo(800));
+        Assert.That(second.Money, Is.EqualTo(200));
+    }
+
+    /// <summary>
+    /// Tests that the money rate of a party member is applied exactly once, on their own share.
+    /// It used to be applied twice: once for the killer when the drop was created, and once for
+    /// the receiver when it was picked up.
+    /// </summary>
+    [Test]
+    public async Task PartyPickUpAppliesEachMembersOwnRateOnceAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        await CreatePartyAsync(first, second).ConfigureAwait(false);
+        SetMoneyRate(first, 2.0f);
+
+        var money = new DroppedMoney(200, DropPosition, first.CurrentMap!, [new MoneyShare(first, 100), new MoneyShare(second, 100)]);
+
+        Assert.That(await money.TryPickUpByAsync(first).ConfigureAwait(false), Is.True);
+        Assert.That(first.Money, Is.EqualTo(200));
+        Assert.That(second.Money, Is.EqualTo(100));
+    }
+
+    /// <summary>
+    /// Tests that the share of a player who is not eligible anymore - because they left the party,
+    /// logged out or went to another map - is given to the remaining members instead of being lost.
+    /// </summary>
+    [Test]
+    public async Task PartyPickUpRedistributesShareOfIneligiblePlayerAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        await CreatePartyAsync(first).ConfigureAwait(false);
+
+        // 'second' never joined the party, so it can't receive its share.
+        var money = new DroppedMoney(200, DropPosition, first.CurrentMap!, [new MoneyShare(first, 100), new MoneyShare(second, 100)]);
+
+        Assert.That(await money.TryPickUpByAsync(first).ConfigureAwait(false), Is.True);
+        Assert.That(first.Money, Is.EqualTo(200));
+        Assert.That(second.Money, Is.EqualTo(0));
+    }
+
+    /// <summary>
+    /// Tests that the pick up clamp is honoured for each party member separately. A member at the
+    /// money limit used to silently lose their share, because the clamp was only applied for solo pick ups.
+    /// </summary>
+    [Test]
+    public async Task PartyPickUpHonoursClampPerMemberAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        await CreatePartyAsync(first, second).ConfigureAwait(false);
+        first.GameContext.Configuration.MaximumInventoryMoney = 150;
+        first.GameContext.Configuration.ClampMoneyOnPickup = true;
+        first.Money = 100;
+        second.Money = 0;
+
+        var money = new DroppedMoney(200, DropPosition, first.CurrentMap!, [new MoneyShare(first, 100), new MoneyShare(second, 100)]);
+
+        Assert.That(await money.TryPickUpByAsync(first).ConfigureAwait(false), Is.True);
+        Assert.That(first.Money, Is.EqualTo(150));
+        Assert.That(second.Money, Is.EqualTo(100));
+    }
+
+    /// <summary>
+    /// Tests that money without reserved shares - for example the fixed amount of an item box -
+    /// is still split equally between the party members.
+    /// </summary>
+    [Test]
+    public async Task MoneyWithoutSharesIsSplitEquallyAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        await CreatePartyAsync(first, second).ConfigureAwait(false);
+
+        var money = new DroppedMoney(200, DropPosition, first.CurrentMap!);
+
+        Assert.That(await money.TryPickUpByAsync(first).ConfigureAwait(false), Is.True);
+        Assert.That(first.Money, Is.EqualTo(100));
+        Assert.That(second.Money, Is.EqualTo(100));
+    }
+
+    /// <summary>
+    /// Tests that the drop is not consumed when no party member could take anything, so it stays
+    /// available instead of being lost.
+    /// </summary>
+    [Test]
+    public async Task PartyPickUpKeepsMoneyAvailableWhenNobodyCanTakeItAsync()
+    {
+        var (first, second) = await CreateTwoPlayersAsync().ConfigureAwait(false);
+        await CreatePartyAsync(first, second).ConfigureAwait(false);
+        first.GameContext.Configuration.MaximumInventoryMoney = 100;
+        first.Money = 100;
+        second.Money = 100;
+
+        var money = new DroppedMoney(200, DropPosition, first.CurrentMap!, [new MoneyShare(first, 100), new MoneyShare(second, 100)]);
+
+        Assert.That(await money.TryPickUpByAsync(first).ConfigureAwait(false), Is.False);
+        Assert.That(first.Money, Is.EqualTo(100));
+        Assert.That(second.Money, Is.EqualTo(100));
+    }
+
+    private static void SetMoneyRate(Player player, float rate)
+    {
+        player.Attributes!.AddElement(new SimpleElement(rate, AggregateType.Multiplicate), Stats.MoneyAmountRate);
+        Assert.That(
+            player.Attributes[Stats.MoneyAmountRate],
+            Is.EqualTo(rate).Within(0.001f),
+            "test setup: the money rate was not applied as expected");
+    }
+
+    private static async ValueTask<Party> CreatePartyAsync(params Player[] members)
+    {
+        var party = new Party(new PartyManager(5, new NullLogger<Party>()), 5, new NullLogger<Party>());
+        foreach (var member in members)
+        {
+            await party.AddAsync(member).ConfigureAwait(false);
+        }
+
+        return party;
+    }
+
+    private static async ValueTask<(Player First, Player Second)> CreateTwoPlayersAsync()
+    {
+        var first = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false);
+        first.GameContext.Configuration.MaximumInventoryMoney = int.MaxValue;
+        first.Money = 0;
+
+        var second = await PlayerTestHelper.CreatePlayerAsync(first.GameContext).ConfigureAwait(false);
+        second.Money = 0;
+
+        Assert.That(first.CurrentMap, Is.Not.Null, "test setup: the players need a map");
+        Assert.That(second.CurrentMap, Is.SameAs(first.CurrentMap), "test setup: the players need to be on the same map");
+        Assert.That(first.IsAtSafezone(), Is.False, "test setup: the players must not be at a safezone");
+
+        return (first, second);
+    }
+}