Merge pull request #841 from nolt/bot-behaviour

Make the server-side bots hold up as a population

(cherry picked from commit 88535b63a958fee9803d5fc3a4bdac5a3318d221)

tests/MUnique.OpenMU.Tests/BotHuntingGroundTest.cs

@@ -0,0 +1,49 @@
+// <copyright file="BotHuntingGroundTest.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 MUnique.OpenMU.DataModel.Configuration;
+using MUnique.OpenMU.GameLogic.Bots;
+using MUnique.OpenMU.Pathfinding;
+using NUnit.Framework;
+
+/// <summary>
+/// Tests for the way a bot picks the spot it hunts on.
+/// </summary>
+[TestFixture]
+public class BotHuntingGroundTest
+{
+    /// <summary>
+    /// A hunting ground on the far side of the map stays a realistic choice. Bots enter a map through the
+    /// same gate and rate every ground from that one spot, so a steep distance penalty made all of them
+    /// queue up on the few spawns next to the entrance.
+    /// </summary>
+    [Test]
+    public void DistantGroundKeepsARealisticShare()
+    {
+        var atTheGate = new MonsterSpawnArea { X1 = 200, X2 = 200, Y1 = 200, Y2 = 200, Quantity = 1 };
+        var acrossTheMap = new MonsterSpawnArea { X1 = 33, X2 = 33, Y1 = 95, Y2 = 95, Quantity = 1 };
+        var gate = new Point(222, 211);
+
+        var near = BotNavigator.GroundWeight(atTheGate, gate);
+        var far = BotNavigator.GroundWeight(acrossTheMap, gate);
+
+        Assert.That(near, Is.GreaterThan(far), "closer grounds are still preferred");
+        Assert.That(near, Is.LessThan(far * 10), "but not to the point where the rest of the map never gets picked");
+    }
+
+    /// <summary>
+    /// The number of monsters an area holds still counts.
+    /// </summary>
+    [Test]
+    public void DenserGroundOutweighsThinnerOneAtTheSameDistance()
+    {
+        var thin = new MonsterSpawnArea { X1 = 100, X2 = 100, Y1 = 100, Y2 = 100, Quantity = 1 };
+        var dense = new MonsterSpawnArea { X1 = 100, X2 = 100, Y1 = 100, Y2 = 100, Quantity = 20 };
+        var from = new Point(150, 150);
+
+        Assert.That(BotNavigator.GroundWeight(dense, from), Is.GreaterThan(BotNavigator.GroundWeight(thin, from)));
+    }
+}

tests/MUnique.OpenMU.Tests/BotJewelHandlerTest.cs

@@ -35,12 +35,11 @@ public class BotJewelHandlerTest
         var bless = await AddJewelAsync(player, FirstBackpackSlot, ItemConstants.JewelOfBless).ConfigureAwait(false);
         await AddJewelAsync(player, FirstBackpackSlot + 1, ItemConstants.JewelOfSoul).ConfigureAwait(false);
 
-        var plan = BotJewelHandler.PlanNextUse(player, false);
+        var plan = BotJewelHandler.PlanNextUse(player, 2, 1, 1);
 
         Assert.That(plan, Is.Not.Null);
         Assert.That(plan!.Value.Jewel, Is.SameAs(bless));
         Assert.That(plan.Value.Target, Is.SameAs(weakPiece));
-        Assert.That(plan.Value.IsLife, Is.False);
     }
 
     /// <summary>
@@ -59,7 +58,7 @@ public class BotJewelHandlerTest
             await AddJewelAsync(player, (byte)(FirstBackpackSlot + i), ItemConstants.JewelOfSoul).ConfigureAwait(false);
         }
 
-        var plan = BotJewelHandler.PlanNextUse(player, false);
+        var plan = BotJewelHandler.PlanNextUse(player, 2, 1, 1);
 
         Assert.That(plan.HasValue, Is.EqualTo(expectsUse));
     }
@@ -76,7 +75,7 @@ public class BotJewelHandlerTest
         await AddJewelAsync(player, FirstBackpackSlot, ItemConstants.JewelOfSoul).ConfigureAwait(false);
         await AddJewelAsync(player, FirstBackpackSlot + 1, ItemConstants.JewelOfSoul).ConfigureAwait(false);
 
-        var plan = BotJewelHandler.PlanNextUse(player, false);
+        var plan = BotJewelHandler.PlanNextUse(player, 2, 1, 1);
 
         Assert.That(plan, Is.Not.Null);
         Assert.That(plan!.Value.Target, Is.SameAs(luckyPiece));
@@ -100,7 +99,7 @@ public class BotJewelHandlerTest
         await AddJewelAsync(player, FirstBackpackSlot, ItemConstants.JewelOfSoul).ConfigureAwait(false);
         await AddJewelAsync(player, FirstBackpackSlot + 1, ItemConstants.JewelOfSoul).ConfigureAwait(false);
 
-        var plan = BotJewelHandler.PlanNextUse(player, false);
+        var plan = BotJewelHandler.PlanNextUse(player, 2, 1, 1);
 
         Assert.That(plan.HasValue, Is.EqualTo(expectsUse));
     }
@@ -116,19 +115,43 @@ public class BotJewelHandlerTest
     {
         var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false);
         var target = await AddEquippedItemAsync(player, InventoryConstants.LeftHandSlot, 9).ConfigureAwait(false);
-        await AddJewelAsync(player, FirstBackpackSlot, ItemConstants.JewelOfLife).ConfigureAwait(false);
+        var life = await AddJewelAsync(player, FirstBackpackSlot, ItemConstants.JewelOfLife).ConfigureAwait(false);
         await AddJewelAsync(player, FirstBackpackSlot + 1, ItemConstants.JewelOfLife).ConfigureAwait(false);
 
-        var plan = BotJewelHandler.PlanNextUse(player, lifeAlreadyUsed);
+        var plan = BotJewelHandler.PlanNextUse(player, 2, 1, lifeAlreadyUsed ? 0 : 1);
 
         Assert.That(plan.HasValue, Is.EqualTo(expectsUse));
         if (expectsUse)
         {
             Assert.That(plan!.Value.Target, Is.SameAs(target));
-            Assert.That(plan.Value.IsLife, Is.True);
+            Assert.That(plan.Value.Jewel, Is.SameAs(life));
         }
     }
 
+    /// <summary>
+    /// Each jewel kind has its own budget: a spent Bless budget must not keep the Soul rule from
+    /// being reached. With one shared budget the Bless rule - which has a target whenever any equipped
+    /// piece is below +6 - consumed every use of every trip, and Souls and Lifes piled up unused.
+    /// </summary>
+    [Test]
+    public async ValueTask SpentBlessBudgetDoesNotStarveSoulAsync()
+    {
+        var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false);
+
+        // A Bless target (below +6) and a Soul target (+6) are available at the same time.
+        await AddEquippedItemAsync(player, InventoryConstants.RightHandSlot, 2).ConfigureAwait(false);
+        var soulTarget = await AddEquippedItemAsync(player, InventoryConstants.LeftHandSlot, 6).ConfigureAwait(false);
+        await AddJewelAsync(player, FirstBackpackSlot, ItemConstants.JewelOfBless).ConfigureAwait(false);
+        var soul = await AddJewelAsync(player, FirstBackpackSlot + 1, ItemConstants.JewelOfSoul).ConfigureAwait(false);
+        await AddJewelAsync(player, FirstBackpackSlot + 2, ItemConstants.JewelOfSoul).ConfigureAwait(false);
+
+        var plan = BotJewelHandler.PlanNextUse(player, 0, 1, 1);
+
+        Assert.That(plan, Is.Not.Null);
+        Assert.That(plan!.Value.Jewel, Is.SameAs(soul));
+        Assert.That(plan.Value.Target, Is.SameAs(soulTarget));
+    }
+
     /// <summary>
     /// Without any applicable jewel or target, nothing is planned.
     /// </summary>
@@ -138,7 +161,7 @@ public class BotJewelHandlerTest
         var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false);
         await AddEquippedItemAsync(player, InventoryConstants.LeftHandSlot, 2).ConfigureAwait(false);
 
-        var plan = BotJewelHandler.PlanNextUse(player, false);
+        var plan = BotJewelHandler.PlanNextUse(player, 2, 1, 1);
 
         Assert.That(plan, Is.Null);
     }

tests/MUnique.OpenMU.Tests/BotServerPartitionTest.cs

@@ -87,6 +87,38 @@ public class BotServerPartitionTest
         Assert.That(BotServerPartition.Split(capacities, 2, 150).Partition.IsGenerator, Is.False);
     }
 
+    /// <summary>
+    /// A deployment which is configured to hold NO bots still has a server which generates - and, above
+    /// all, purges - the population. Deciding the role by the number of accounts instead would mean that
+    /// "delete all bots" does nothing at all on exactly the setting which asks for no bots.
+    /// </summary>
+    [Test]
+    public void TheFirstServerActsEvenWithoutAnyAccounts()
+    {
+        List<(byte ServerId, int Capacity)> capacities = [(0, 50), (1, 50)];
+
+        var (first, assigned) = BotServerPartition.Split(capacities, 0, 0);
+        var (second, _) = BotServerPartition.Split(capacities, 1, 0);
+
+        Assert.That(assigned, Is.EqualTo(0));
+        Assert.That(first.AccountCount, Is.EqualTo(0));
+        Assert.That(first.IsGenerator, Is.True);
+        Assert.That(second.IsGenerator, Is.False);
+    }
+
+    /// <summary>
+    /// The role is held by one server even when the deployment has no bot capacity left at all, so a
+    /// purge still finds someone to carry it out.
+    /// </summary>
+    [Test]
+    public void TheFirstServerActsWithoutAnyCapacity()
+    {
+        List<(byte ServerId, int Capacity)> capacities = [(0, 0), (1, 0)];
+
+        Assert.That(BotServerPartition.Split(capacities, 0, 100).Partition.IsGenerator, Is.True);
+        Assert.That(BotServerPartition.Split(capacities, 1, 100).Partition.IsGenerator, Is.False);
+    }
+
     /// <summary>
     /// The servers may have different player limits; the shares follow their capacity.
     /// </summary>

tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs

@@ -0,0 +1,126 @@
+// <copyright file="BotSkillRepertoireTest.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 MUnique.OpenMU.DataModel.Configuration;
+using MUnique.OpenMU.GameLogic.Bots;
+using NUnit.Framework;
+
+/// <summary>
+/// Tests for the skills a bot may learn - the gate which decides what it owns to fight with.
+/// </summary>
+[TestFixture]
+public class BotSkillRepertoireTest
+{
+    /// <summary>
+    /// Tests that the castle siege skills are refused. They carry the highest damage numbers of their
+    /// classes, so a "strongest first" rule walks straight into them, and the game activates them
+    /// nowhere but the siege map.
+    /// </summary>
+    /// <param name="skillNumber">The number of the castle siege skill.</param>
+    [TestCase((short)44)]  // Crescent Moon Slash
+    [TestCase((short)45)]  // Lance
+    [TestCase((short)46)]  // Starfall
+    [TestCase((short)57)]  // Spiral Slash
+    [TestCase((short)73)]  // Mana Rays
+    [TestCase((short)74)]  // Fire Blast
+    [TestCase((short)269)] // Charge
+    public void CastleSiegeSkillIsNotLearned(short skillNumber)
+    {
+        var skill = CreateAttackSkill(skillNumber, attackDamage: 120);
+
+        Assert.That(BotProgression.IsBotLearnableSkill(skill), Is.False);
+    }
+
+    /// <summary>
+    /// Tests that the skills of the siege roles stay out as well. Stun is the one that matters: it is an
+    /// area skill with no damage and a single hit, which is exactly what Twisting Slash looks like, so
+    /// nothing but its number tells the two apart.
+    /// </summary>
+    [Test]
+    public void SiegeRoleSkillIsNotLearned()
+    {
+        var stun = CreateAttackSkill(67, attackDamage: 0, skillType: SkillType.AreaSkillAutomaticHits);
+
+        Assert.That(BotProgression.IsBotLearnableSkill(stun), Is.False);
+    }
+
+    /// <summary>
+    /// Tests that a skill which carries no flat damage bonus but strikes several times is learned. The
+    /// Rage Fighter's whole arsenal is built that way - its damage comes from the weapon - so judging by
+    /// <see cref="Skill.AttackDamage"/> alone left the class with nothing to fight with.
+    /// </summary>
+    [Test]
+    public void MultiHitSkillWithoutFlatDamageIsLearned()
+    {
+        var killingBlow = CreateAttackSkill(260, attackDamage: 0, hits: 4);
+
+        Assert.That(BotProgression.IsBotLearnableSkill(killingBlow), Is.True);
+    }
+
+    /// <summary>
+    /// Tests that an area skill is learned even without a flat damage bonus: its worth is the number of
+    /// monsters it catches, not a bonus per hit.
+    /// </summary>
+    [Test]
+    public void AreaSkillWithoutFlatDamageIsLearned()
+    {
+        var twistingSlash = CreateAttackSkill(41, attackDamage: 0, skillType: SkillType.AreaSkillAutomaticHits);
+
+        Assert.That(BotProgression.IsBotLearnableSkill(twistingSlash), Is.True);
+    }
+
+    /// <summary>
+    /// Tests that a single-hit skill without a damage bonus stays out: it is worth no more than a plain
+    /// attack while costing mana. These are the combo swings, which the combo handler drives separately.
+    /// </summary>
+    [Test]
+    public void PlainSingleHitSkillWithoutDamageIsNotLearned()
+    {
+        var lunge = CreateAttackSkill(20, attackDamage: 0);
+
+        Assert.That(BotProgression.IsBotLearnableSkill(lunge), Is.False);
+    }
+
+    /// <summary>
+    /// Tests that an ordinary attack skill is still learned.
+    /// </summary>
+    [Test]
+    public void OrdinaryAttackSkillIsLearned()
+    {
+        var evilSpirit = CreateAttackSkill(9, attackDamage: 45);
+
+        Assert.That(BotProgression.IsBotLearnableSkill(evilSpirit), Is.True);
+    }
+
+    /// <summary>
+    /// Tests that a pet's skill is recognized as one. Plasma Storm belongs to the Fenrir, but its damage
+    /// attribute is derived from the character's own stats, so it looks strong on a character riding
+    /// nothing - and being the longest ranged skill most classes own, it won the tie-break for the whole
+    /// population until the pet was checked for.
+    /// </summary>
+    [Test]
+    public void PetSkillIsRecognizedAsOne()
+    {
+        var plasmaStorm = CreateAttackSkill(76, attackDamage: 60, damageType: DamageType.Fenrir);
+        var strikeOfDestruction = CreateAttackSkill(232, attackDamage: 110);
+
+        Assert.That(BotProgression.RequiresPet(plasmaStorm), Is.True);
+        Assert.That(BotProgression.RequiresPet(strikeOfDestruction), Is.False);
+    }
+
+    private static Skill CreateAttackSkill(short number, int attackDamage, byte hits = 1, SkillType skillType = SkillType.DirectHit, DamageType damageType = DamageType.Physical)
+    {
+        return new Skill
+        {
+            Number = number,
+            AttackDamage = attackDamage,
+            NumberOfHitsPerAttack = hits,
+            SkillType = skillType,
+            DamageType = damageType,
+            Range = 6,
+        };
+    }
+}