WaylanderK
Recruit
Summary:
The donation perks, Paid in Promise and Giving Hands, allow players to donate items that results in XP being added to their party's troops. However, the process of applying the donated XP is flawed. When distributing the XP, it always subtracts the full upgrade cost, ignoring the XP the troop already has. This results in not all troops being ready for upgrade when donating the maximum that the inventory screen allows. It also does not handle leftover xp properly.
Bug present since before 1.0.0.
How to Reproduce:
Bug when troops have experience:
The cause:
I debugged the code and found the origin of the issues in
The function in its current state does not handle existing xp and does not handle leftover xp properly. I made my own version and made tests to expose the edge cases that the original function fails to handle.
Have you used cheats and if so which: None
The donation perks, Paid in Promise and Giving Hands, allow players to donate items that results in XP being added to their party's troops. However, the process of applying the donated XP is flawed. When distributing the XP, it always subtracts the full upgrade cost, ignoring the XP the troop already has. This results in not all troops being ready for upgrade when donating the maximum that the inventory screen allows. It also does not handle leftover xp properly.
Bug present since before 1.0.0.
How to Reproduce:
Bug when troops have experience:
- Have a donation perk
- Have two troops in party where one has non-zero XP
- Donate the max amount the inventory screen allows
- Observe the last troop not being ready for upgrade
The cause:
I debugged the code and found the origin of the issues in
CampaignSystem.Helpers.MobilePartyHelper.PartyAddSharedXp. This is the current code:
C#:
public static void PartyAddSharedXp(MobileParty party, float xpToDistribute)
{
TroopRoster memberRoster = party.MemberRoster;
PartyTroopUpgradeModel troopUpgradeModel = Campaign.Current.Models.PartyTroopUpgradeModel;
int num = 0;
for (int index = 0; index < memberRoster.Count; ++index)
{
TroopRosterElement elementCopyAtIndex = memberRoster.GetElementCopyAtIndex(index);
if (troopUpgradeModel.CanTroopGainXp(party.Party, elementCopyAtIndex.Character))
num += MobilePartyHelper.GetShareWeight(ref elementCopyAtIndex);
}
for (int index = 0; index < memberRoster.Count && (double) xpToDistribute >= 1.0 && num > 0; ++index)
{
TroopRosterElement elementCopyAtIndex = memberRoster.GetElementCopyAtIndex(index);
if (troopUpgradeModel.CanTroopGainXp(party.Party, elementCopyAtIndex.Character))
{
int shareWeight = MobilePartyHelper.GetShareWeight(ref elementCopyAtIndex);
int xpAmount = MathF.Floor(xpToDistribute * (float) shareWeight / (float) num);
memberRoster.AddXpToTroopAtIndex(xpAmount, index);
xpToDistribute -= (float) xpAmount;
num -= shareWeight;
}
}
}The function in its current state does not handle existing xp and does not handle leftover xp properly. I made my own version and made tests to expose the edge cases that the original function fails to handle.
C#:
class TroopInfo {
public int MaxGainableXp { get; set; }
public int MemberRosterIndex { get; set; }
public int Weight { get; set; }
}
public static void PartyAddSharedXp(MobileParty party, float xpToDistribute) {
TroopRoster memberRoster = party.MemberRoster;
PartyTroopUpgradeModel troopUpgradeModel = Campaign.Current.Models.PartyTroopUpgradeModel;
int totalXpToDistribute = (int)xpToDistribute;
float totalShares = 0;
var troopsWithXpToGain = new List<TroopInfo>();
for (int index = 0; index < memberRoster.Count; ++index) {
TroopRosterElement troop = memberRoster.GetElementCopyAtIndex(index);
if (!troopUpgradeModel.IsTroopUpgradeable(party.Party, troop.Character)) {
continue;
}
var upgradeCost = troop.Character.UpgradeTargets
.Select(upgradeTarget => troopUpgradeModel.GetXpCostForUpgrade(party.Party, troop.Character, upgradeTarget))
.Max();
var maxGainableXp = troop.Number * upgradeCost - troop.Xp;
if (maxGainableXp <= 0) {
continue;
}
var weight = MobilePartyHelper.GetShareWeight(ref troop);
troopsWithXpToGain.Add(new TroopInfo {
MaxGainableXp = maxGainableXp,
MemberRosterIndex = index,
Weight = weight,
});
totalShares += weight;
}
bool hasAnyTroopBeenMaxed = true;
while (totalXpToDistribute > 0 && hasAnyTroopBeenMaxed) {
hasAnyTroopBeenMaxed = false;
int xpRemaining = totalXpToDistribute;
int decreaseShares = 0;
for (int index = 0; index < troopsWithXpToGain.Count; index++) {
var troopInfo = troopsWithXpToGain[index];
float shareWeight = troopInfo.Weight / totalShares;
// Get the maximum xp that can be given to the troop. Cannot be more than share or remaining xp allows
int awardedXp = Math.Min(MathF.Floor(totalXpToDistribute * shareWeight), xpRemaining);
if (awardedXp >= troopInfo.MaxGainableXp) {
awardedXp = troopInfo.MaxGainableXp;
decreaseShares += troopInfo.Weight;
troopsWithXpToGain.RemoveAt(index--);
hasAnyTroopBeenMaxed = true;
}
memberRoster.AddXpToTroopAtIndex(awardedXp, troopInfo.MemberRosterIndex);
troopInfo.MaxGainableXp -= awardedXp;
xpRemaining -= awardedXp;
}
totalShares -= decreaseShares;
totalXpToDistribute = xpRemaining;
}
// Distribute leftover xp, needed to deal with rounding errors
int leftoverDistributionIndex = 0;
while (totalXpToDistribute > 0 && leftoverDistributionIndex < troopsWithXpToGain.Count) {
var troopInfo = troopsWithXpToGain[leftoverDistributionIndex++];
var xpToAward = Math.Min(troopInfo.MaxGainableXp, totalXpToDistribute);
memberRoster.AddXpToTroopAtIndex(xpToAward, troopInfo.MemberRosterIndex);
totalXpToDistribute -= xpToAward;
}
}
C#:
using NUnit.Framework;
using System;
using System.Collections.Generic;
using System.Linq;
/// <summary>
/// Test multiple different scenarios that PartyAddSharedXp should be able to handle. The OriginalTroopXpDistributor
/// is the function that can be found in game with the Bannerlord specific code removed. The FixedTroopXpDistributor
/// is my own proposal that handles the edge cases that the original fails at.
///
/// Change the _distributor field to test the fixed or original version.
/// </summary>
[TestFixture]
public class PartyAddSharedXpTests {
// Switch between OriginalTroopXpDistributor and FixedTroopXpDistributor
private readonly TroopXpDistributor _distributor = new OriginalTroopXpDistributor();
[Test]
public void DistributesASingleXp() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 1, UpgradeCost = 50, Xp = 0 },
new Troop { Tier = 1, Amount = 1, UpgradeCost = 50, Xp = 0 },
};
_distributor.DistributeXP(1, troops);
Assert.Multiple(() => {
// Original gives one xp less to first and one more to second, this is fine
Assert.AreEqual(1, troops[0].Xp);
Assert.AreEqual(0, troops[1].Xp);
});
}
[Test]
public void HandlesAFullXpTroop() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 10, UpgradeCost = 100, Xp = 1000 },
new Troop { Tier = 1, Amount = 5, UpgradeCost = 100, Xp = 0 },
new Troop { Tier = 1, Amount = 5, UpgradeCost = 100, Xp = 0 },
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
Assert.AreEqual(1000, troops[0].Xp);
Assert.AreEqual(50, troops[1].Xp);
Assert.AreEqual(50, troops[2].Xp);
});
}
[Test]
public void DistributingMaxXpWithSameTierSameCostInexperiencedTroopsShouldMaxThemAll() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 1, UpgradeCost = 50, Xp = 0 },
new Troop { Tier = 1, Amount = 1, UpgradeCost = 50, Xp = 0 },
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
Assert.AreEqual(50, troops[0].Xp);
Assert.AreEqual(50, troops[1].Xp);
});
}
[Test]
public void DistributingMaxXpWithSameTierDifferentCostInexperiencedTroopsShouldMaxThemAll() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 1, UpgradeCost = 50, Xp = 0 },
new Troop { Tier = 1, Amount = 1, UpgradeCost = 100, Xp = 0 },
};
_distributor.DistributeXP(150, troops);
// Original discards leftover xp
Assert.Multiple(() => {
Assert.AreEqual(50, troops[0].Xp);
Assert.AreEqual(100, troops[1].Xp);
});
}
[Test]
public void DistributingMaxXpWithMixedTierInexperiencedTroopsShouldMaxThemAll() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 1, UpgradeCost = 50, Xp = 0 },
new Troop { Tier = 2, Amount = 1, UpgradeCost = 100, Xp = 0 },
};
_distributor.DistributeXP(150, troops);
Assert.Multiple(() => {
Assert.AreEqual(50, troops[0].Xp);
Assert.AreEqual(100, troops[1].Xp);
});
}
[Test]
public void DistributingMaxXpWithExperiencedTroopsShouldMaxThemAll() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 1, UpgradeCost = 50, Xp = 25 },
new Troop { Tier = 1, Amount = 1, UpgradeCost = 100, Xp = 5 },
};
_distributor.DistributeXP(120, troops);
Assert.Multiple(() => {
Assert.AreEqual(50, troops[0].Xp);
Assert.AreEqual(100, troops[1].Xp);
});
}
[Test]
public void DistributingMaxXpWithMixedExperiencedTroopsShouldMaxThemAll() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 3, UpgradeCost = 50, Xp = 25 },
new Troop { Tier = 2, Amount = 3, UpgradeCost = 100, Xp = 10 },
new Troop { Tier = 4, Amount = 3, UpgradeCost = 1000, Xp = 500 },
};
_distributor.DistributeXP(2915, troops);
Assert.Multiple(() => {
Assert.AreEqual(troops[0].UpgradeCost * troops[0].Amount, troops[0].Xp);
Assert.AreEqual(troops[1].UpgradeCost * troops[1].Amount, troops[1].Xp);
Assert.AreEqual(troops[2].UpgradeCost * troops[2].Amount, troops[2].Xp);
});
}
[Test]
public void DistributingSomeXpWithSameTierSameAmountInexperiencedTroopsShouldApplyCorrectly() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 1, UpgradeCost = 100, Xp = 0 },
new Troop { Tier = 1, Amount = 1, UpgradeCost = 100, Xp = 0 },
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
Assert.AreEqual(50, troops[0].Xp);
Assert.AreEqual(50, troops[1].Xp);
});
}
[Test]
public void DistributingSomeXpWithSameTierDifferentAmountInexperiencedTroopsShouldApplyCorrectly() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 10, UpgradeCost = 100, Xp = 0 },
new Troop { Tier = 1, Amount = 90, UpgradeCost = 100, Xp = 0 },
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
// The original is off by one here
Assert.AreEqual(10, troops[0].Xp);
Assert.AreEqual(90, troops[1].Xp);
});
}
[Test]
public void DistributingSomeXpWithDifferentTierDifferentAmountInexperiencedTroopsShouldApplyCorrectly() {
var troops = new List<Troop> {
new Troop { Tier = 2, Amount = 5, UpgradeCost = 50, Xp = 0 },
new Troop { Tier = 9, Amount = 10, UpgradeCost = 100, Xp = 0 },
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
Assert.AreEqual(10, troops[0].Xp);
Assert.AreEqual(90, troops[1].Xp);
});
}
[Test]
public void DistributingSomeXpWithSameTierExperiencedTroopsShouldApplyCorrectly() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 10, UpgradeCost = 100, Xp = 99 },
new Troop { Tier = 1, Amount = 90, UpgradeCost = 100, Xp = 0 },
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
// The original is off by one here
Assert.AreEqual(10 + 99, troops[0].Xp);
Assert.AreEqual(90, troops[1].Xp);
});
}
[Test]
public void DistributingSomeXpWithMixedExperiencedTroopsShouldApplyCorrectly() {
var troops = new List<Troop> {
new Troop { Tier = 2, Amount = 5, UpgradeCost = 100, Xp = 25 },
new Troop { Tier = 9, Amount = 10, UpgradeCost = 1000, Xp = 80 },
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
Assert.AreEqual(35, troops[0].Xp);
Assert.AreEqual(170, troops[1].Xp);
});
}
[Test]
public void DistributedXpShouldBeWeighted() {
var troops = new List<Troop> {
new Troop { Tier = 1, Amount = 10, UpgradeCost = 100, Xp = 50 },
new Troop { Tier = 2, Amount = 5, UpgradeCost = 275, Xp = 25 }
};
_distributor.DistributeXP(100, troops);
Assert.Multiple(() => {
Assert.AreEqual(100, troops[0].Xp);
Assert.AreEqual(75, troops[1].Xp);
});
}
[Test]
public void DistributedXpShouldBeWeightedWithFianAndFalxMan() {
var troops = new List<Troop> {
new Troop { Tier = 5, Amount = 1, UpgradeCost = 1700, Xp = 0 },
new Troop { Tier = 4, Amount = 1, UpgradeCost = 1300, Xp = 0 }
};
_distributor.DistributeXP(2400, troops);
Assert.Multiple(() => {
// Original gives one xp less to first and one more to second, this is fine
Assert.AreEqual(1334, troops[0].Xp);
Assert.AreEqual(1066, troops[1].Xp);
});
}
[Test]
public void ShouldEndWhenXpIsLeftoverAndNotOverAdd() {
var troops = new List<Troop> { new Troop { Tier = 1, Amount = 1, UpgradeCost = 100, Xp = 0 }, };
_distributor.DistributeXP(1000, troops);
Assert.AreEqual(100, troops[0].Xp);
}
[Test]
public void DistributesComplexExample() {
var troops = new List<Troop> {
new() { Tier = 5, Amount = 7, UpgradeCost = 100, Xp = 55 },
new() { Tier = 3, Amount = 7, UpgradeCost = 33, Xp = 66 },
new() { Tier = 4, Amount = 7, UpgradeCost = 22, Xp = 48 },
new() { Tier = 7, Amount = 1, UpgradeCost = 18, Xp = 5 },
};
_distributor.DistributeXP(1500, troops);
Assert.Multiple(() => {
Assert.AreEqual(7 * 100, troops[0].Xp);
Assert.AreEqual(7 * 33, troops[1].Xp);
Assert.AreEqual(7 * 22, troops[2].Xp);
Assert.AreEqual(1 * 18, troops[3].Xp);
});
}
[Test]
public void HandlesComplexSharesCorrectlyWithExistingExperience() {
var troops = new List<Troop> {
new() { Tier = 5, Amount = 7, UpgradeCost = 100, Xp = 55 },
new() { Tier = 3, Amount = 7, UpgradeCost = 33, Xp = 66 },
};
_distributor.DistributeXP(708, troops);
Assert.Multiple(() => {
Assert.AreEqual(598, troops[0].Xp);
Assert.AreEqual(7 * 33, troops[1].Xp);
});
}
[Test]
public void HandlesSharesCorrectlyWithExistingExperience() {
var troops = new List<Troop> {
new() { Tier = 5, Amount = 3, UpgradeCost = 1500, Xp = 550 },
new() { Tier = 3, Amount = 3, UpgradeCost = 1300, Xp = 667 },
};
_distributor.DistributeXP(7000, troops);
Assert.Multiple(() => {
Assert.AreEqual(4500, troops[0].Xp);
Assert.AreEqual(3717, troops[1].Xp);
});
}
[Test]
public void SmallWeightsShouldNotResultInLostExperience() {
var troops = new List<Troop> {
new() { Tier = 6, Amount = 500, UpgradeCost = 1700, Xp = 500 * 1700 },
new() { Tier = 1, Amount = 1, UpgradeCost = 1500, Xp = 0 },
};
_distributor.DistributeXP(1500, troops);
Assert.Multiple(() => {
Assert.AreEqual(500 * 1700, troops[0].Xp);
Assert.AreEqual(1500, troops[1].Xp);
});
}
[Test]
public void CompareToFixedDistributor() {
// Easy-of-use function to compare output of current distributor (see _distributor) with the fixed version.
var testCases = new List<TestCase> {
new() {
Troops = () => new List<Troop> {
new() { Tier = 1, Amount = 5, UpgradeCost = 1500, Xp = 0 },
new() { Tier = 6, Amount = 3, UpgradeCost = 1300, Xp = 0 },
},
XpToDistribute = 10183,
},
new() {
Troops = () => new List<Troop> {
new() { Tier = 5, Amount = 7, UpgradeCost = 100, Xp = 55 },
new() { Tier = 3, Amount = 7, UpgradeCost = 33, Xp = 66 },
new() { Tier = 4, Amount = 7, UpgradeCost = 22, Xp = 48 },
new() { Tier = 7, Amount = 1, UpgradeCost = 18, Xp = 5 },
},
XpToDistribute = 1500,
},
new() {
Troops = () => new List<Troop> {
new() { Tier = 5, Amount = 7, UpgradeCost = 100, Xp = 55 },
new() { Tier = 3, Amount = 7, UpgradeCost = 33, Xp = 66 },
new() { Tier = 4, Amount = 7, UpgradeCost = 22, Xp = 48 },
new() { Tier = 7, Amount = 1, UpgradeCost = 18, Xp = 5 },
},
XpToDistribute = 1500,
},
};
var fixedDistributor = new FixedTroopXpDistributor();
var originalDistributor = _distributor;
foreach (TestCase testCase in testCases) {
var fixedTroops = testCase.Troops();
fixedDistributor.DistributeXP(testCase.XpToDistribute, fixedTroops);
var originalTroops = testCase.Troops();
originalDistributor.DistributeXP(testCase.XpToDistribute, originalTroops);
Assert.That(originalTroops.Select(t => t.Xp), Is.EquivalentTo(fixedTroops.Select(t => t.Xp)));
}
}
}
public class Troop {
public int Amount { get; set; }
public int UpgradeCost { get; set; }
public int Tier { get; set; }
public int Xp { get; set; }
}
public abstract class TroopXpDistributor {
public abstract void DistributeXP(int totalXpToDistribute, List<Troop> troops);
public int GetTroopWeight(Troop troop) {
return troop.Tier * troop.Amount;
}
public static int Floor(float f) => (int)Math.Floor((double)f);
}
/// <summary>
/// Version found in game without any Bannerlord specifics such as MemberRoster.
/// </summary>
public class OriginalTroopXpDistributor : TroopXpDistributor {
public override void DistributeXP(int totalXpToDistribute, List<Troop> troops) {
int totalShares = 0;
foreach (var troop in troops) {
totalShares += this.GetTroopWeight(troop);
}
for (int index = 0; index < troops.Count && totalXpToDistribute >= 1.0 && totalShares > 0; index++) {
Troop troop = troops[index];
var shareWeight = this.GetTroopWeight(troop);
int xpAmount = Floor(totalXpToDistribute * (float)shareWeight / (float)totalShares);
troop.Xp += xpAmount;
totalXpToDistribute -= xpAmount;
totalShares -= shareWeight;
}
}
}
/// <summary>
/// Handle edge cases with the simplest code to comprehend and the least amount of resources, both memory and
/// execution time.
/// </summary>
public class FixedTroopXpDistributor : TroopXpDistributor {
class TroopInfo {
public Troop Troop { get; set; }
public int Weight { get; set; }
public int MaxGainableXp { get; set; }
}
public override void DistributeXP(int totalXpToDistribute, List<Troop> troops) {
float totalShares = 0;
var troopsWithXpToGain = new List<TroopInfo>();
foreach (var troop in troops) {
var maxGainableXp = troop.Amount * troop.UpgradeCost - troop.Xp;
if (maxGainableXp == 0) {
continue;
}
var weight = this.GetTroopWeight(troop);
troopsWithXpToGain.Add(new TroopInfo { Troop = troop, MaxGainableXp = maxGainableXp, Weight = weight, });
totalShares += weight;
}
bool hasAnyTroopBeenMaxed = true;
while (totalXpToDistribute > 0 && hasAnyTroopBeenMaxed) {
hasAnyTroopBeenMaxed = false;
int xpRemaining = totalXpToDistribute;
int decreaseShares = 0;
for (int index = 0; index < troopsWithXpToGain.Count; index++) {
var troopInfo = troopsWithXpToGain[index];
float shareWeight = troopInfo.Weight / totalShares;
// Get the maximum xp that can be given to the troop. Cannot be more than share or remaining xp allows
int awardedXp = Math.Min(Floor(totalXpToDistribute * shareWeight), xpRemaining);
if (awardedXp >= troopInfo.MaxGainableXp) {
awardedXp = troopInfo.MaxGainableXp;
decreaseShares += troopInfo.Weight;
troopsWithXpToGain.RemoveAt(index--);
hasAnyTroopBeenMaxed = true;
}
troopInfo.Troop.Xp += awardedXp;
troopInfo.MaxGainableXp -= awardedXp;
xpRemaining -= awardedXp;
}
totalShares -= decreaseShares;
totalXpToDistribute = xpRemaining;
}
// Distribute leftover xp, needed to deal with rounding errors
int leftoverDistributionIndex = 0;
while (totalXpToDistribute > 0 && leftoverDistributionIndex < troopsWithXpToGain.Count) {
var troopInfo = troopsWithXpToGain[leftoverDistributionIndex++];
var xpToAward = Math.Min(troopInfo.MaxGainableXp, totalXpToDistribute);
troopInfo.Troop.Xp += xpToAward;
totalXpToDistribute -= xpToAward;
}
}
}
/// <summary>
/// Used in CompareToFixedDistributor.
/// </summary>
class TestCase {
public Func<List<Troop>> Troops { get; set; }
public int XpToDistribute { get; set; }
}
Code:
CompareToFixedDistributor
Expected: equivalent to < 6283, 3900 >
But was: < 2213, 7970 >
DistributedXpShouldBeWeighted
Expected: 100
But was: 50
Expected: 75
But was: 100
DistributingMaxXpWithExperiencedTroopsShouldMaxThemAll
Expected: 50
But was: 85
Expected: 100
But was: 65
DistributingMaxXpWithMixedExperiencedTroopsShouldMaxThemAll
Expected: 150
But was: 441
Expected: 300
But was: 843
DistributesComplexExample
Expected: 700
But was: 576
Expected: 231
But was: 367
Expected: 154
But was: 542
Expected: 18
But was: 184
DistributingMaxXpWithExperiencedTroopsShouldMaxThemAll
Expected: 50
But was: 60
Expected: 100
But was: 85
DistributingMaxXpWithMixedExperiencedTroopsShouldMaxThemAll
Expected: 150
But was: 416
Expected: 300
But was: 841
DistributingSomeXpWithMixedExperiencedTroopsShouldApplyCorrectly
Expected: 35
But was: 10
Expected: 170
But was: 115
DistributingSomeXpWithSameTierExperiencedTroopsShouldApplyCorrectly
Expected: 109
But was: 10
Expected: 90
But was: 189
HandlesAFullXpTroop
Expected: 1000
But was: 50
Expected: 50
But was: 525
Expected: 50
But was: 525
HandlesComplexSharesCorrectlyWithExistingExperience
Expected: 598
But was: 442
Expected: 231
But was: 321
HandlesSharesCorrectlyWithExistingExperience
Expected: 4500
But was: 4375
Expected: 3717
But was: 3175
ShouldEndWhenXpIsLeftoverAndNotOverAdd (Not really an issue since it is clamped in AddXpToTroopAtIndex)
Expected: 100
But was: 1000
SmallWeightsShouldNotResultInLostExperience
Expected: 850000
But was: 1499
Expected: 1500
But was: 850001Have you used cheats and if so which: None
Last edited:
