From c172d98f4ee53fd69fb8302ba905e238fe89dffd Mon Sep 17 00:00:00 2001 From: Eduardo <6845999+eduardosmaniotto@users.noreply.github.com> Date: Sat, 5 Sep 2026 13:40:23 -0300 Subject: [PATCH 1/4] Fix bots using skills they should not have access to Bots no longer receive orb/scroll skills magically on level-up; like regular players, they now learn them only by looting the orb/scroll and consuming it through the regular consume action (new BotSkillHandler, hooked into pickup and the navigator's equipment cadence). Generation backfills a plausible kit instead, gated by granting-item obtainability (drop level + item requirements). Mount-bound skills (Impale, Fire Breath, Plasma Storm) are off-limits to bots entirely: never learned, looted, or selected, with no mount detection needed. Fresh-start bots now spawn weapon-only with no armor, like a regular player's new character; veteran starter gear was extracted into BotStarterGearEquipper. Fixes #943. --- docs-website/docs/server-features/bots.md | 13 +- src/GameLogic/Bots/BotGenerator.cs | 179 ++------------- src/GameLogic/Bots/BotNavigator.cs | 5 + src/GameLogic/Bots/BotProgression.cs | 208 +++++++++++++++-- src/GameLogic/Bots/BotSkillHandler.cs | 108 +++++++++ .../Bots/BotSkillProgressionPlugIn.cs | 16 +- src/GameLogic/Bots/BotStarterGearEquipper.cs | 201 +++++++++++++++++ src/GameLogic/Bots/BotStartupProfile.cs | 13 ++ src/GameLogic/Offline/CombatHandler.cs | 7 + src/GameLogic/Offline/ItemPickupHandler.cs | 7 + .../Offline/BotProgressionTests.cs | 211 ++++++++++++++++++ .../Offline/BotStartupProfileTests.cs | 10 +- 12 files changed, 784 insertions(+), 194 deletions(-) create mode 100644 src/GameLogic/Bots/BotSkillHandler.cs create mode 100644 src/GameLogic/Bots/BotStarterGearEquipper.cs diff --git a/docs-website/docs/server-features/bots.md b/docs-website/docs/server-features/bots.md index ce020135d5..419c5fc1ad 100644 --- a/docs-website/docs/server-features/bots.md +++ b/docs-website/docs/server-features/bots.md @@ -143,10 +143,15 @@ range are three tiles at any level. Skills the game only activates during a castle siege are left out, and so are a pet's skills unless the pet is actually equipped: Plasma Storm draws its damage from the Fenrir, but the attribute behind it is derived from the character's own stats, so nothing but the pet slot tells a -mounted character from one riding nothing. Skills are learned against the -game's own requirements — total energy, leadership, character level — at -generation and again on every level-up, and the class buffs are kept up on their -own. +mounted character from one riding nothing. Mount-bound skills stay out entirely +instead: bots neither learn nor use them (even if a looted pet ends up in the +pet slot). Skills are learned like a human +player learns them: a freshly generated bot starts with the skills its start +level and stats entitle it to (a fresh level-1 character almost none, a veteran +a plausible kit up to its level), and from then on it loots skill orbs and +scrolls from the ground and consumes them - never granted magically on level-up. +A scroll which does not drop yet where the bot hunts stays unknown until the bot +gets there. Once learned, the class buffs are kept up on their own. A skill the character cannot currently cast is passed over, in the attack rotation and in the buffs alike. That is not the same as not having learned it: a diff --git a/src/GameLogic/Bots/BotGenerator.cs b/src/GameLogic/Bots/BotGenerator.cs index f338b89e73..1adc890a9a 100644 --- a/src/GameLogic/Bots/BotGenerator.cs +++ b/src/GameLogic/Bots/BotGenerator.cs @@ -9,7 +9,6 @@ namespace MUnique.OpenMU.GameLogic.Bots; using Microsoft.Extensions.Logging; using MUnique.OpenMU.AttributeSystem; using MUnique.OpenMU.DataModel.Configuration; -using MUnique.OpenMU.DataModel.Configuration.Items; using MUnique.OpenMU.DataModel.Entities; using MUnique.OpenMU.GameLogic.Attributes; using MUnique.OpenMU.GameLogic.Resets; @@ -44,24 +43,6 @@ internal sealed class BotGenerator /// Number of inventory extensions (each 4 rows of 8 slots) a bot gets, so loot does not clog its backpack. private const int BotInventoryExtensions = 4; - /// The highest item group that is a melee weapon (0 sword, 1 axe, 2 mace, 3 spear). - private const byte MaxMeleeGroup = 3; - - /// Item group of bows (need ammunition). - private const byte BowGroup = 4; - - /// Item group of staves/sticks (casters). - private const byte StaffGroup = 5; - - /// Item group of body armor; its item number identifies the armor set. - private const byte ArmorGroup = 8; - - /// - /// Armor set numbers tried in thematic order; the first the class is qualified for (by its chest piece) - /// is used: 5 Leather (warriors), 2 Pad (wizards), 10 Vine (elves), 39 Mistery (summoners), then fallbacks. - /// - private static readonly byte[] ArmorSetCandidates = { 5, 2, 10, 39, 6, 0, 4, 8 }; - private readonly IGameContext _gameContext; private readonly ILogger _logger; private readonly BotNameGenerator _nameGenerator = new(); @@ -172,7 +153,7 @@ public async ValueTask EnsureBotsAsync(int numberOfAccounts, int characters var level = profile.GetStartLevel(minLevel, maxLevel); var seededResets = profile.GetSeededResets(maxSeededResets); var name = await this._nameGenerator.GenerateUniqueAsync(context, reservedNames, cancellationToken).ConfigureAwait(false); - this.CreateCharacter(context, account, name, characterClass, level, slot, experienceTable, seededResets, profile.StarterItemLevel, resetConfiguration); + this.CreateCharacter(context, account, name, characterClass, level, slot, experienceTable, seededResets, profile.StarterItemLevel, profile.EquipStarterArmor, resetConfiguration); } // Save per account so a single failure does not roll back already generated accounts, @@ -459,7 +440,7 @@ private async ValueTask TryDeleteBotAccountAsync(string } } - private void CreateCharacter(IPlayerContext context, Account account, string name, CharacterClass characterClass, int level, byte slot, long[] experienceTable, int seededResets, byte starterItemLevel, ResetConfiguration? resetConfiguration) + private void CreateCharacter(IPlayerContext context, Account account, string name, CharacterClass characterClass, int level, byte slot, long[] experienceTable, int seededResets, byte starterItemLevel, bool equipStarterArmor, ResetConfiguration? resetConfiguration) { // A character generated beyond the class evolution level was created as its second-generation // class right away - like a player who completed the class quest long ago. Everything downstream @@ -522,7 +503,18 @@ private void CreateCharacter(IPlayerContext context, Account account, string nam character.Inventory = context.CreateNew(); character.Inventory.Money = StartMoney; - this.EquipStarterGear(context, character, starterItemLevel); + + // A fresh character starts like a regular player's new character - weapon only, no armor - + // and loots its first set like everyone else. Veterans keep the basic set, without which + // they could not survive the maps their start level puts them on. + var starterGear = new BotStarterGearEquipper(context, this._gameContext.Configuration, character, starterItemLevel); + starterGear.EquipWeapon(); + if (equipStarterArmor) + { + starterGear.EquipArmorSet(); + } + + starterGear.AddPotions(); account.Characters.Add(character); } @@ -532,7 +524,10 @@ private void CreateCharacter(IPlayerContext context, Account account, string nam /// as the class's own buffs and heals (e.g. elf Heal/Greater Defense/Greater Damage). Only skills the /// class is qualified for are ever learned, gated by the skills' real learn requirements from the game /// configuration (total energy, leadership, character level, ...) evaluated against the stats the bot - /// was just given - exactly the requirements a human player has to meet for the same skill. + /// was just given - exactly the requirements a human player has to meet for the same skill. Orb and + /// scroll skills additionally require their granting item to be obtainable (see + /// ), so a bot cannot learn a scroll before the + /// monster level where it starts to drop. /// private void LearnClassSkills(IPlayerContext context, Character character, CharacterClass characterClass, int level) { @@ -555,6 +550,7 @@ private void LearnClassSkills(IPlayerContext context, Character character, Chara if (!BotProgression.IsBotLearnableSkill(skill, itemGrantedSkillNumbers) || !skill.QualifiedCharacters.Contains(characterClass) || !BotProgression.MeetsRequirements(skill, GetValue) + || !BotProgression.IsGrantingItemObtainable(skill, this._gameContext.Configuration, characterClass, level, GetValue) || !learnedNumbers.Add(skill.Number)) { continue; @@ -567,142 +563,5 @@ private void LearnClassSkills(IPlayerContext context, Character character, Chara } } - /// - /// Equips the bot with a basic, class-appropriate weapon and armor set (mirrors the low-level test - /// account gear), so it is not naked and punching with its fists. The item level scales modestly - /// with the bot level for a bit more defense/damage without raising the equip requirements too high. - /// - /// The persistence context. - /// The character to equip. - /// The upgrade level of the starter items (level 0 for fresh characters). - private void EquipStarterGear(IPlayerContext context, Character character, byte starterItemLevel) - { - var inventory = character.Inventory!; - var characterClass = character.CharacterClass!; - - // Data-driven, so every class gets gear it is actually QUALIFIED to wear (a Dark Lord must never - // end up in a Pad/wizard set). We pick the most basic options (lowest DropLevel) the class can use: - // - a weapon from the weapon groups (0 sword, 1 axe, 2 mace, 3 spear, 4 bow, 5 staff), - // - the armor set whose chest piece (group 8) has the lowest DropLevel; its NUMBER identifies the set, - // and the equipment type is the GROUP (7 helm, 8 armor, 9 pants, 10 gloves, 11 boots). - // The weapon type follows the bot's BUILD (BotProgression.IsPreferredWeaponGroup - the same rule the - // later upgrades use), so an energy-specked Magic Gladiator starts with a staff instead of a blade. - // The Small Axe is qualified for almost every class, so without this filter casters and archers would - // all end up with one. - bool IsPreferredWeapon(ItemDefinition definition) - => BotProgression.IsPreferredWeaponGroup(characterClass, character.Name, (byte)definition.Group); - - // Ammunition shares the bow group (Bolt/Arrows have DropLevel 0), so without this filter every - // archer would get a bolt stack as its "weapon" and end up punching with its fists. - var weapon = this._gameContext.Configuration.Items - .Where(d => IsPreferredWeapon(d) && !d.IsAmmunition && d.QualifiedCharacters.Contains(characterClass)) - .MinBy(d => d.DropLevel) - ?? this._gameContext.Configuration.Items - .Where(d => d.Group <= StaffGroup && !d.IsAmmunition && d.QualifiedCharacters.Contains(characterClass)) - .MinBy(d => d.DropLevel); - if (weapon is not null) - { - if (weapon.Group == BowGroup) - { - // Bows need ammunition; the arrows go into the left hand. - this.AddEquippedItem(context, inventory, characterClass, InventoryConstants.RightHandSlot, weapon, starterItemLevel); - this.AddAmmunition(context, inventory); - } - else - { - this.AddEquippedItem(context, inventory, characterClass, InventoryConstants.LeftHandSlot, weapon, starterItemLevel); - } - } - - // Choose a thematically appropriate armor set the class can wear, tried in order (warriors -> Leather, - // wizards -> Pad, elves -> Vine, summoners -> Mistery, then fallbacks). Each piece is added only if the - // class is qualified for it, so e.g. the Magic Gladiator keeps the set but skips the helm it can't wear. - foreach (var set in ArmorSetCandidates) - { - if (this._gameContext.Configuration.Items.FirstOrDefault(d => d.Group == ArmorGroup && d.Number == set) is not { } chest - || !chest.QualifiedCharacters.Contains(characterClass)) - { - continue; - } - - this.EquipArmorPiece(context, inventory, characterClass, InventoryConstants.HelmSlot, 7, set, starterItemLevel); - this.EquipArmorPiece(context, inventory, characterClass, InventoryConstants.ArmorSlot, 8, set, starterItemLevel); - this.EquipArmorPiece(context, inventory, characterClass, InventoryConstants.PantsSlot, 9, set, starterItemLevel); - this.EquipArmorPiece(context, inventory, characterClass, InventoryConstants.GlovesSlot, 10, set, starterItemLevel); - this.EquipArmorPiece(context, inventory, characterClass, InventoryConstants.BootsSlot, 11, set, starterItemLevel); - break; - } - - this.AddPotions(context, inventory); - } - - private void AddPotions(IPlayerContext context, ItemStorage inventory) - { - // A stack of Large Healing Potions so the offline HealingHandler has something to drink, and a - // stack of Large Mana Potions so casters can keep casting instead of degrading to weak melee once - // their mana runs dry. The BotNavigator tops both up at runtime, so the bot never runs out. - // Durability holds the stack count. - this.AddPotionStack(context, inventory, 3, InventoryConstants.EquippableSlotsCount); // Large Healing Potion, first backpack slot - this.AddPotionStack(context, inventory, 6, (byte)(InventoryConstants.EquippableSlotsCount + 1)); // Large Mana Potion, second backpack slot - } - - private void AddPotionStack(IPlayerContext context, ItemStorage inventory, byte potionNumber, byte slot) - { - var potion = this._gameContext.Configuration.Items.FirstOrDefault(d => d.Group == 14 && d.Number == potionNumber); - if (potion is null) - { - return; - } - - var item = context.CreateNew(); - item.Definition = potion; - - // Only a handful of charges to start with: fresh bots head to the merchant right away and buy - // their supplies with their starting Zen, kicking off the shopping economy from minute one - // (kept just above the emergency top-up threshold, so the economy path - not the fallback - runs). - item.Durability = Rand.NextInt(10, 16); - item.ItemSlot = slot; - inventory.Items.Add(item); - } - - private void EquipArmorPiece(IPlayerContext context, ItemStorage inventory, CharacterClass characterClass, byte slot, int group, int number, byte starterItemLevel) - { - var definition = this._gameContext.Configuration.Items.FirstOrDefault(d => d.Group == group && d.Number == number); - if (definition is null || !definition.QualifiedCharacters.Contains(characterClass)) - { - return; - } - this.AddEquippedItem(context, inventory, characterClass, slot, definition, starterItemLevel); - } - - private void AddEquippedItem(IPlayerContext context, ItemStorage inventory, CharacterClass characterClass, byte slot, ItemDefinition definition, byte starterItemLevel) - { - if (!definition.QualifiedCharacters.Contains(characterClass)) - { - return; - } - - var item = context.CreateNew(); - item.Definition = definition; - item.Level = starterItemLevel; - item.Durability = definition.Durability; - item.ItemSlot = slot; - inventory.Items.Add(item); - } - - private void AddAmmunition(IPlayerContext context, ItemStorage inventory) - { - var arrows = this._gameContext.Configuration.Items.FirstOrDefault(d => d.Group == 4 && d.Number == 15); - if (arrows is null) - { - return; - } - - var item = context.CreateNew(); - item.Definition = arrows; - item.Durability = 255; - item.ItemSlot = InventoryConstants.LeftHandSlot; - inventory.Items.Add(item); - } } diff --git a/src/GameLogic/Bots/BotNavigator.cs b/src/GameLogic/Bots/BotNavigator.cs index c194f1e28b..2463b1e8c8 100644 --- a/src/GameLogic/Bots/BotNavigator.cs +++ b/src/GameLogic/Bots/BotNavigator.cs @@ -557,6 +557,11 @@ private async ValueTask EvaluateAsync(CancellationToken cancellationToken) this._nextEquipCheckUtc = DateTime.UtcNow + EquipCheckInterval; this._player.PendingBotActions.Enqueue(() => BotEquipmentHandler.TryEquipUpgradesAsync(this._player)); + // Looted skill orbs and scrolls are consumed into new skills like a human would (see + // BotSkillHandler). Queued for the same reason: learning mutates the skill list the combat + // handler may be enumerating on its own timer. + this._player.PendingBotActions.Enqueue(() => BotSkillHandler.TryLearnSkillsAsync(this._player)); + // Wings don't drop, so the loot-driven equipment progression above never provides them; // they are earned at the classic level milestones instead (see BotWingHandler). Queued // for the same reason: equipping mount item power-ups. diff --git a/src/GameLogic/Bots/BotProgression.cs b/src/GameLogic/Bots/BotProgression.cs index 16ba77a486..e96d89ab10 100644 --- a/src/GameLogic/Bots/BotProgression.cs +++ b/src/GameLogic/Bots/BotProgression.cs @@ -4,6 +4,7 @@ namespace MUnique.OpenMU.GameLogic.Bots; +using System.Collections.Concurrent; using MUnique.OpenMU.AttributeSystem; using MUnique.OpenMU.DataModel.Configuration; using MUnique.OpenMU.GameLogic.Attributes; @@ -85,6 +86,16 @@ internal static class BotProgression /// private static readonly short[] ItemOrWeaponBoundSkillNumbers = [270]; + /// + /// Skills which the game only lets a character cast while riding a mount (see the skill numbers in + /// the SkillNumber enum of the initialization assembly). Neither the mount nor, where it + /// applies, the required weapon kind is modeled in the skill data, so nothing else tells a mounted + /// cast from one on foot. Bots never use these skills - even if a looted pet ends up in the pet + /// slot, they are never learned, never looted and never selected (see ) - + /// which keeps the logic free of any mount detection. + /// + private static readonly short[] MountRequiredSkillNumbers = [47, 49, 76]; + /// /// Gets the class the character evolves into at , or null when the /// class has no (in-scope) evolution. @@ -293,27 +304,16 @@ public static bool IsBotLearnableSkill(Skill skill, IReadOnlySet itemGran return false; } - if (CastleSiegeOnlySkillNumbers.Contains(skill.Number) - || ItemOrWeaponBoundSkillNumbers.Contains(skill.Number) - || (itemGrantedSkillNumbers.Contains(skill.Number) && skill.Requirements is not { Count: > 0 })) + if (itemGrantedSkillNumbers.Contains(skill.Number) && skill.Requirements is not { Count: > 0 }) { return false; } - if (IsAttackSkill(skill)) - { - // Worth learning if it adds damage of its own, hits more than once, or hits more than one - // target. Judging by AttackDamage alone would lock a Rage Fighter out of Chain Drive and - // Dragon Roar, which carry a flat bonus of zero and four hits instead, because their damage - // comes from the weapon - which is also how the server pays them out. - return skill.AttackDamage > 0 - || skill.NumberOfHitsPerAttack > 1 - || IsAreaSkill(skill); - } - - return skill.SkillType is SkillType.Buff or SkillType.Regeneration - && skill.MagicEffectDef is not null - && !ExcludedBuffSkillNumbers.Contains(skill.Number); + // Worth learning if it adds damage of its own, hits more than once, or hits more than one + // target. Judging by AttackDamage alone would lock a Rage Fighter out of Chain Drive and + // Dragon Roar, which carry a flat bonus of zero and four hits instead, because their damage + // comes from the weapon - which is also how the server pays them out. + return IsBotLootableSkill(skill); } /// @@ -355,6 +355,170 @@ or SkillType.AreaSkillExplicitHits /// The skill. public static bool RequiresPet(Skill skill) => skill.DamageType == DamageType.Fenrir; + /// + /// Determines whether the skill is bound to a mount and therefore never used by bots. Covers bots + /// which had already learned such a skill before the gate existed, too - + /// the combat handler skips it without any mount detection. + /// + /// The skill. + public static bool RequiresMount(Skill skill) => MountRequiredSkillNumbers.Contains(skill.Number); + + /// + /// Determines whether the skill is one a bot may pick up and learn from a looted orb or scroll, + /// like a human player: an actual attack skill or a castable self/party buff or heal - but never a + /// master skill, a castle-siege-only skill, or a mount-bound skill (never used by bots). Unlike + /// , item-granted skills are welcome here: the orb or scroll in + /// the bot's backpack is the gate, exactly as for a human consuming it. + /// + /// The skill to check. + public static bool IsBotLootableSkill(Skill skill) + { + if (skill.MasterDefinition is not null + || CastleSiegeOnlySkillNumbers.Contains(skill.Number) + || ItemOrWeaponBoundSkillNumbers.Contains(skill.Number) + || MountRequiredSkillNumbers.Contains(skill.Number)) + { + return false; + } + + if (IsAttackSkill(skill)) + { + return skill.AttackDamage > 0 + || skill.NumberOfHitsPerAttack > 1 + || IsAreaSkill(skill); + } + + return skill.SkillType is SkillType.Buff or SkillType.Regeneration + && skill.MagicEffectDef is not null + && !ExcludedBuffSkillNumbers.Contains(skill.Number); + } + + /// + /// Determines whether the bot could plausibly own the item which teaches an orb/scroll skill: the + /// granting item must accept the bot's class, the bot's level must have reached the item's drop + /// level (the monster level where the item starts to drop, so a low-level character hunting where + /// it does not drop yet could not own one), and the bot must meet the item's own level and stat + /// requirements (the same gate a human faces at the consume handler). At least one granting item + /// must pass; a skill with no granting item at all is not item-gated and returns true. + /// + /// The skill whose granting item is checked. + /// The game configuration which defines the items. + /// The bot's current character class. + /// The bot's current character level. + /// Resolves an attribute's current value; null means unknown and fails. + public static bool IsGrantingItemObtainable( + Skill skill, + GameConfiguration gameConfiguration, + CharacterClass characterClass, + int level, + Func getAttributeValue) + { + var grantingItems = GetGrantingItems(gameConfiguration, skill.Number); + if (grantingItems.Count == 0) + { + return true; + } + + return grantingItems.Any(item => IsObtainableGrantingItem(item, characterClass, level, getAttributeValue)); + } + + /// + /// Cache of the items granting each skill, per game configuration. Configurations are effectively + /// immutable at runtime (a reload builds a new instance), so a static cache keyed by the instance + /// is safe; it keeps the per-tick skill selection of hundreds of bots from re-scanning the whole + /// item list for every candidate skill. + /// + private static readonly ConcurrentDictionary>> GrantingItemsCache = new(); + + private static IReadOnlyList GetGrantingItems(GameConfiguration gameConfiguration, short skillNumber) + { + var bySkill = GrantingItemsCache.GetOrAdd( + gameConfiguration, + static config => (config.Items ?? []) + .Where(item => item.Skill is not null) + .GroupBy(item => item.Skill!.Number) + .ToDictionary(group => group.Key, group => group.ToList()) as IReadOnlyDictionary>); + return bySkill.TryGetValue(skillNumber, out var items) ? items : []; + } + + private static bool IsObtainableGrantingItem( + DataModel.Configuration.Items.ItemDefinition item, + CharacterClass characterClass, + int level, + Func getAttributeValue) + { + if (!item.QualifiedCharacters.Contains(characterClass)) + { + return false; + } + + if (level < item.DropLevel) + { + return false; + } + + // The caller's getAttributeValue resolves TOTAL attributes (at generation time from base + // stats via TotalToBaseStat, at runtime from the live attribute graph) - exactly what + // MeetsRequirements expects. Item requirements use the same totals, except scrolls which + // use the *RequirementValue variants, so those are normalized first. Level is resolved + // from the passed level, which is also what the callers map Stats.Level to. + foreach (var requirement in item.Requirements) + { + if (requirement.Attribute is not { } attribute) + { + continue; + } + + if (attribute == Stats.Level) + { + if (level < requirement.MinimumValue) + { + return false; + } + + continue; + } + + var totalAttribute = NormalizeRequirementValue(attribute); + if (getAttributeValue(totalAttribute) is not { } value || value < requirement.MinimumValue) + { + return false; + } + } + + return true; + } + + private static AttributeDefinition NormalizeRequirementValue(AttributeDefinition attribute) + { + if (attribute == Stats.TotalEnergyRequirementValue) + { + return Stats.TotalEnergy; + } + + if (attribute == Stats.TotalStrengthRequirementValue) + { + return Stats.TotalStrength; + } + + if (attribute == Stats.TotalAgilityRequirementValue) + { + return Stats.TotalAgility; + } + + if (attribute == Stats.TotalVitalityRequirementValue) + { + return Stats.TotalVitality; + } + + if (attribute == Stats.TotalLeadershipRequirementValue) + { + return Stats.TotalLeadership; + } + + return attribute; + } + /// /// Determines whether the character meets the skill's learn requirements (the same ones the game /// enforces when casting, e.g. total energy for wizard spells or character level for knight skills). @@ -389,27 +553,27 @@ public static bool MeetsRequirements(Skill skill, FuncThe "total" attribute to map. public static AttributeDefinition? TotalToBaseStat(AttributeDefinition attribute) { - if (attribute == Stats.TotalEnergy) + if (attribute == Stats.TotalEnergy || attribute == Stats.TotalEnergyRequirementValue) { return Stats.BaseEnergy; } - if (attribute == Stats.TotalStrength) + if (attribute == Stats.TotalStrength || attribute == Stats.TotalStrengthRequirementValue) { return Stats.BaseStrength; } - if (attribute == Stats.TotalAgility) + if (attribute == Stats.TotalAgility || attribute == Stats.TotalAgilityRequirementValue) { return Stats.BaseAgility; } - if (attribute == Stats.TotalVitality) + if (attribute == Stats.TotalVitality || attribute == Stats.TotalVitalityRequirementValue) { return Stats.BaseVitality; } - if (attribute == Stats.TotalLeadership) + if (attribute == Stats.TotalLeadership || attribute == Stats.TotalLeadershipRequirementValue) { return Stats.BaseLeadership; } diff --git a/src/GameLogic/Bots/BotSkillHandler.cs b/src/GameLogic/Bots/BotSkillHandler.cs new file mode 100644 index 0000000000..b42b71ac0c --- /dev/null +++ b/src/GameLogic/Bots/BotSkillHandler.cs @@ -0,0 +1,108 @@ +// +// Licensed under the MIT License. See LICENSE file in the project root for full license information. +// + +namespace MUnique.OpenMU.GameLogic.Bots; + +using MUnique.OpenMU.DataModel.Entities; +using MUnique.OpenMU.GameLogic.Offline; +using MUnique.OpenMU.GameLogic.PlayerActions.ItemConsumeActions; + +/// +/// Lets a bot learn skills like a real player: by looting the orb or scroll from the ground and +/// consuming it through the regular - the same validations a human +/// faces (class qualification, the item's level and stat requirements, unknown skill only). Skills +/// are therefore learned when their orb or scroll actually drops where the bot hunts, instead of +/// appearing the moment the bot's stats allow it - a low-level bot can no longer fight with a skill +/// whose orb or scroll does not drop yet where it hunts. +/// +internal static class BotSkillHandler +{ + /// The item group of skill orbs. + private const byte OrbGroup = 12; + + /// The item group of skill scrolls and parchments. + private const byte ScrollGroup = 15; + + private static readonly ItemConsumeAction ConsumeAction = new(); + + /// + /// Determines whether the dropped item teaches the bot a skill it wants: an orb or scroll for a + /// skill the bot does not know yet, is qualified for (skill and item alike), may currently consume + /// (the item's own requirements), and may actually fight with (no siege-only, mount-bound, master + /// or non-combat skills). Like for gear upgrades, the pickup handler asks + /// this before collecting anything from the ground. + /// + /// The bot player which would learn the skill. + /// The dropped item to evaluate. + public static bool WantsSkillItem(Player player, Item item) + { + if (player.SelectedCharacter?.CharacterClass is not { } characterClass + || item.Definition is not { } definition + || definition.Skill is not { } skill) + { + return false; + } + + if (definition.Group != OrbGroup && definition.Group != ScrollGroup) + { + return false; + } + + if (player.SkillList?.ContainsSkill(skill.Number.ToUnsigned()) == true) + { + return false; + } + + if (!skill.QualifiedCharacters.Contains(characterClass) + || !definition.QualifiedCharacters.Contains(characterClass) + || !BotProgression.IsBotLootableSkill(skill)) + { + return false; + } + + // The same gate a human faces when consuming the orb or scroll. + return player.CompliesRequirements(item); + } + + /// + /// Consumes every looted orb and scroll in the bot's backpack which still teaches something new. + /// Runs on the equipment cadence (see BotNavigator) and queued into the AI tick like every + /// other self-mutation, so it never races the combat handler enumerating the skill list. One pass + /// consumes whatever is consumable right now; what the bot cannot consume yet (a requirement it + /// does not meet) was never picked up in the first place. + /// + /// The bot player whose backpack is scanned. + public static async ValueTask TryLearnSkillsAsync(OfflinePlayer player) + { + if (player.Inventory is not { } inventory) + { + return; + } + + // Snapshot, because consuming mutates the item collection while we iterate. + var backpackItems = inventory.Items + .Where(i => i.ItemSlot >= InventoryConstants.EquippableSlotsCount) + .ToList(); + + foreach (var item in backpackItems) + { + if (!WantsSkillItem(player, item)) + { + continue; + } + + var skillName = item.Definition?.Skill?.Name; + var slot = item.ItemSlot; + + // No target item: skill orbs and scrolls are consumed on their own. Byte.MaxValue addresses + // no slot, so the action resolves a null target instead of some unrelated equipped piece. + await ConsumeAction.HandleConsumeRequestAsync(player, slot, byte.MaxValue, FruitUsage.Undefined).ConfigureAwait(false); + + if (inventory.GetItem(slot) != item) + { + player.Logger.LogDebug("Bot '{Name}' learned '{Skill}' from a looted orb or scroll.", player.Name, skillName); + } + } + } +} diff --git a/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs b/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs index d7f67fe6a4..c776470a6d 100644 --- a/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs +++ b/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs @@ -16,10 +16,11 @@ namespace MUnique.OpenMU.GameLogic.Bots; /// /// Grows a server-side bot like a real player when it levels up during play: the earned stat points are /// invested according to the bot's class build (see ), and any -/// class skill whose learn requirements (total energy, leadership, character level, ...) are now met is -/// learned - attack skills as well as the class's own buffs and heals. Skills are only ever learned for -/// the character's own class (), using the same requirements the -/// game enforces for human players, so a grown bot matches a freshly generated one of the same level. +/// non-item class skill whose learn requirements (total energy, leadership, character level, ...) are +/// now met is learned. Orb and scroll skills are excluded on purpose: like a human player, the bot learns +/// those only by looting and consuming the orb or scroll (see ). Skills are +/// only ever learned for the character's own class (), using the +/// same requirements the game enforces for human players. /// [PlugIn] [Display(Name = "Bot skill progression", Description = "Invests level-up stat points and teaches server-side bots new class- and level-appropriate skills as they level up.")] @@ -157,10 +158,15 @@ private async ValueTask LearnNewSkillsAsync(Player player) float? GetValue(AttributeDefinition attribute) => player.Attributes?[attribute]; + // Orb and scroll skills are deliberately NOT granted here: like a human player, the bot learns + // those only by finding the orb or scroll on the ground and consuming it (see BotSkillHandler). + // Handing them out on every level-up is what put scroll skills the bot could not yet own into + // low-level hunting grounds. var itemGrantedSkillNumbers = BotProgression.GetItemGrantedSkillNumbers(player.GameContext.Configuration); foreach (var skill in player.GameContext.Configuration.Skills) { - if (!BotProgression.IsBotLearnableSkill(skill, itemGrantedSkillNumbers) + if (itemGrantedSkillNumbers.Contains(skill.Number) + || !BotProgression.IsBotLearnableSkill(skill, itemGrantedSkillNumbers) || !skill.QualifiedCharacters.Contains(characterClass) || skillList.ContainsSkill((ushort)skill.Number) || !BotProgression.MeetsRequirements(skill, GetValue)) diff --git a/src/GameLogic/Bots/BotStarterGearEquipper.cs b/src/GameLogic/Bots/BotStarterGearEquipper.cs new file mode 100644 index 0000000000..0c9677ea5b --- /dev/null +++ b/src/GameLogic/Bots/BotStarterGearEquipper.cs @@ -0,0 +1,201 @@ +// +// Licensed under the MIT License. See LICENSE file in the project root for full license information. +// + +namespace MUnique.OpenMU.GameLogic.Bots; + +using System.Linq; +using MUnique.OpenMU.DataModel.Configuration.Items; +using MUnique.OpenMU.Persistence; + +/// +/// Equips a freshly generated bot character with its starter gear: a class- and build-appropriate +/// weapon (with ammunition for bows), a basic armor set, and starting potion stacks - everything at +/// the profile's starter item level. One instance outfits one character: the shared context (persistence +/// context, inventory, class, item level) travels in fields, so each step only takes what actually +/// varies per call instead of threading the same parameters through every helper. +/// +internal sealed class BotStarterGearEquipper +{ + /// Item group of bows (need ammunition). + private const byte BowGroup = 4; + + /// Item group of staves/sticks (casters). + private const byte StaffGroup = 5; + + /// Item group of body armor; its item number identifies the armor set. + private const byte ArmorGroup = 8; + + /// + /// Armor set numbers tried in thematic order; the first the class is qualified for (by its chest piece) + /// is used: 5 Leather (warriors), 2 Pad (wizards), 10 Vine (elves), 39 Mistery (summoners), then fallbacks. + /// + private static readonly byte[] ArmorSetCandidates = { 5, 2, 10, 39, 6, 0, 4, 8 }; + + private readonly IPlayerContext _context; + private readonly GameConfiguration _configuration; + private readonly ItemStorage _inventory; + private readonly Character _character; + private readonly CharacterClass _characterClass; + private readonly byte _starterItemLevel; + + /// + /// Initializes a new instance of the class. + /// + /// The persistence context. + /// The game configuration which defines the items. + /// The character to equip. Its inventory and class must be set. + /// The upgrade level of the starter items (level 0 for fresh characters). + public BotStarterGearEquipper(IPlayerContext context, GameConfiguration configuration, Character character, byte starterItemLevel) + { + this._context = context; + this._configuration = configuration; + this._inventory = character.Inventory!; + this._character = character; + this._characterClass = character.CharacterClass!; + this._starterItemLevel = starterItemLevel; + } + + /// + /// Equips a basic, class-appropriate weapon (mirrors the low-level test account gear), so the bot + /// is not punching with its fists. + /// + public void EquipWeapon() + { + // Data-driven, so every class gets a weapon it is actually QUALIFIED to wield. We pick the most + // basic option (lowest DropLevel) the class can use from the weapon groups (0 sword, 1 axe, + // 2 mace, 3 spear, 4 bow, 5 staff). + // The weapon type follows the bot's BUILD (BotProgression.IsPreferredWeaponGroup - the same rule the + // later upgrades use), so an energy-specked Magic Gladiator starts with a staff instead of a blade. + // The Small Axe is qualified for almost every class, so without this filter casters and archers would + // all end up with one. + bool IsPreferredWeapon(ItemDefinition definition) + => BotProgression.IsPreferredWeaponGroup(this._characterClass, this._character.Name, (byte)definition.Group); + + // Ammunition shares the bow group (Bolt/Arrows have DropLevel 0), so without this filter every + // archer would get a bolt stack as its "weapon" and end up punching with its fists. + var weapon = this._configuration.Items + .Where(d => IsPreferredWeapon(d) && !d.IsAmmunition && d.QualifiedCharacters.Contains(this._characterClass)) + .MinBy(d => d.DropLevel) + ?? this._configuration.Items + .Where(d => d.Group <= StaffGroup && !d.IsAmmunition && d.QualifiedCharacters.Contains(this._characterClass)) + .MinBy(d => d.DropLevel); + if (weapon is null) + { + return; + } + + if (weapon.Group == BowGroup) + { + // Bows need ammunition; the arrows go into the left hand. + this.AddEquippedItem(InventoryConstants.RightHandSlot, weapon); + this.AddAmmunition(); + } + else + { + this.AddEquippedItem(InventoryConstants.LeftHandSlot, weapon); + } + } + + /// + /// Equips a basic, class-appropriate armor set (mirrors the low-level test account gear). + /// + public void EquipArmorSet() + { + // Data-driven, so every class gets gear it is actually QUALIFIED to wear (a Dark Lord must never + // end up in a Pad/wizard set). We pick the armor set whose chest piece (group 8) has the lowest + // DropLevel; its NUMBER identifies the set, and the equipment type is the GROUP (7 helm, 8 armor, + // 9 pants, 10 gloves, 11 boots). + // Choose a thematically appropriate armor set the class can wear, tried in order (warriors -> Leather, + // wizards -> Pad, elves -> Vine, summoners -> Mistery, then fallbacks). Each piece is added only if the + // class is qualified for it, so e.g. the Magic Gladiator keeps the set but skips the helm it can't wear. + foreach (var set in ArmorSetCandidates) + { + if (this._configuration.Items.FirstOrDefault(d => d.Group == ArmorGroup && d.Number == set) is not { } chest + || !chest.QualifiedCharacters.Contains(this._characterClass)) + { + continue; + } + + this.EquipArmorPiece(InventoryConstants.HelmSlot, 7, set); + this.EquipArmorPiece(InventoryConstants.ArmorSlot, 8, set); + this.EquipArmorPiece(InventoryConstants.PantsSlot, 9, set); + this.EquipArmorPiece(InventoryConstants.GlovesSlot, 10, set); + this.EquipArmorPiece(InventoryConstants.BootsSlot, 11, set); + break; + } + } + + /// + /// Adds starting potion stacks to the backpack, so the offline HealingHandler has something to drink. + /// + public void AddPotions() + { + // A stack of Large Healing Potions so the offline HealingHandler has something to drink, and a + // stack of Large Mana Potions so casters can keep casting instead of degrading to weak melee once + // their mana runs dry. The BotNavigator tops both up at runtime, so the bot never runs out. + // Durability holds the stack count. + this.AddPotionStack(3, InventoryConstants.EquippableSlotsCount); // Large Healing Potion, first backpack slot + this.AddPotionStack(6, (byte)(InventoryConstants.EquippableSlotsCount + 1)); // Large Mana Potion, second backpack slot + } + + private void EquipArmorPiece(byte slot, int group, int number) + { + var definition = this._configuration.Items.FirstOrDefault(d => d.Group == group && d.Number == number); + if (definition is null || !definition.QualifiedCharacters.Contains(this._characterClass)) + { + return; + } + + this.AddEquippedItem(slot, definition); + } + + private void AddEquippedItem(byte slot, ItemDefinition definition) + { + if (!definition.QualifiedCharacters.Contains(this._characterClass)) + { + return; + } + + var item = this._context.CreateNew(); + item.Definition = definition; + item.Level = this._starterItemLevel; + item.Durability = definition.Durability; + item.ItemSlot = slot; + this._inventory.Items.Add(item); + } + + private void AddAmmunition() + { + var arrows = this._configuration.Items.FirstOrDefault(d => d.Group == 4 && d.Number == 15); + if (arrows is null) + { + return; + } + + var item = this._context.CreateNew(); + item.Definition = arrows; + item.Durability = 255; + item.ItemSlot = InventoryConstants.LeftHandSlot; + this._inventory.Items.Add(item); + } + + private void AddPotionStack(byte potionNumber, byte slot) + { + var potion = this._configuration.Items.FirstOrDefault(d => d.Group == 14 && d.Number == potionNumber); + if (potion is null) + { + return; + } + + var item = this._context.CreateNew(); + item.Definition = potion; + + // Only a handful of charges to start with: fresh bots head to the merchant right away and buy + // their supplies with their starting Zen, kicking off the shopping economy from minute one + // (kept just above the emergency top-up threshold, so the economy path - not the fallback - runs). + item.Durability = Rand.NextInt(10, 16); + item.ItemSlot = slot; + this._inventory.Items.Add(item); + } +} diff --git a/src/GameLogic/Bots/BotStartupProfile.cs b/src/GameLogic/Bots/BotStartupProfile.cs index 0fa2272995..e6bdc3405b 100644 --- a/src/GameLogic/Bots/BotStartupProfile.cs +++ b/src/GameLogic/Bots/BotStartupProfile.cs @@ -30,6 +30,13 @@ internal abstract class BotStartupProfile /// public abstract byte StarterItemLevel { get; } + /// + /// Gets whether a character of this profile is equipped with a starter armor set. Fresh characters + /// start like a regular player's new character - weapon only, no armor - while veterans start with + /// a basic set so they can survive the maps their start level puts them on. + /// + public abstract bool EquipStarterArmor { get; } + /// /// Creates the startup profile corresponding to the /// flag of the bot feature. @@ -71,6 +78,9 @@ private sealed class FreshStartupProfile : BotStartupProfile /// public override byte StarterItemLevel => FreshStarterItemLevel; + /// + public override bool EquipStarterArmor => false; + /// public override int GetStartLevel(int minLevel, int maxLevel) { @@ -122,6 +132,9 @@ private sealed class VeteranStartupProfile : BotStartupProfile /// public override byte StarterItemLevel => VeteranStarterItemLevel; + /// + public override bool EquipStarterArmor => true; + /// public override int GetStartLevel(int minLevel, int maxLevel) { diff --git a/src/GameLogic/Offline/CombatHandler.cs b/src/GameLogic/Offline/CombatHandler.cs index e3f7b890c4..5e55117d6e 100644 --- a/src/GameLogic/Offline/CombatHandler.cs +++ b/src/GameLogic/Offline/CombatHandler.cs @@ -775,6 +775,7 @@ private async ValueTask ExecuteTargetedSkillAttackAsync(IAttackable target, Skil } var ridesFenrir = this.RidesFenrir(); + var isBot = this.IsBot; var candidates = new List<(SkillEntry Entry, float Score)>(); foreach (var entry in skillList.Skills) { @@ -782,6 +783,12 @@ private async ValueTask ExecuteTargetedSkillAttackAsync(IAttackable target, Skil || !BotProgression.IsAttackSkill(skill) || BotProgression.IsCastleSiegeOnly(skill) || (BotProgression.RequiresPet(skill) && !ridesFenrir) + + // Mount-bound skills are never selectable for bots - no mount detection needed, even if + // a looted pet sits in the pet slot. This also covers bots which had already learned + // such a skill before the gate existed. Humans keep their mounted skills: their client + // only enables the cast while riding anyway. + || (isBot && BotProgression.RequiresMount(skill)) || skill.Range == 0 // Same trap as the buffs: a character keeps its skills across a reset but not the level diff --git a/src/GameLogic/Offline/ItemPickupHandler.cs b/src/GameLogic/Offline/ItemPickupHandler.cs index d16cd5bb26..bf8c0651eb 100644 --- a/src/GameLogic/Offline/ItemPickupHandler.cs +++ b/src/GameLogic/Offline/ItemPickupHandler.cs @@ -143,6 +143,13 @@ private bool ShouldPickUp(Item item) return true; } + if (this._player.Account?.IsBot == true && Bots.BotSkillHandler.WantsSkillItem(this._player, item)) + { + // The item is an orb or scroll teaching a skill the bot does not know yet and may currently + // consume - picked up like a human would; the BotSkillHandler consumes it on its next pass. + return true; + } + if (this._config.PickExtraItems && item.Definition is { } definition) { return this._config.ExtraItemNames.Any(name => definition.Name.ToString()?.Contains(name, StringComparison.OrdinalIgnoreCase) ?? false); diff --git a/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs b/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs index 9cbf37a73c..a8648f6289 100644 --- a/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs +++ b/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs @@ -203,6 +203,217 @@ public void IsBotLearnableSkill_CastleSiegeRoleSkill_ReturnsFalse(short skillNum Assert.That(BotProgression.IsBotLearnableSkill(skill, NoItemGrantedSkills), Is.False); } + /// + /// Tests that mount-bound skills are never learned by a bot - a bot fighting with one on foot + /// does something no player can do. + /// + [TestCase((short)47, "Impale", 28)] + [TestCase((short)49, "Fire Breath", 110)] + [TestCase((short)76, "Plasma Storm", 110)] + public void IsBotLearnableSkill_MountRequiredSkill_ReturnsFalse(short skillNumber, string name, int levelRequirement) + { + var skill = new SkillWithRequirements(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = levelRequirement }) + { + Number = skillNumber, + Name = name, + SkillType = SkillType.DirectHit, + AttackDamage = 15, + NumberOfHitsPerAttack = 1, + }; + + Assert.That(BotProgression.IsBotLearnableSkill(skill, NoItemGrantedSkills), Is.False); + Assert.That(BotProgression.RequiresMount(skill), Is.True); + } + + /// + /// Tests that ordinary skills carry no mount requirement. + /// + [Test] + public void RequiresMount_OrdinarySkill_ReturnsFalse() + { + var evilSpirit = new Skill + { + Number = 9, + Name = "Evil Spirit", + SkillType = SkillType.AreaSkillAutomaticHits, + AttackDamage = 45, + NumberOfHitsPerAttack = 1, + }; + + Assert.That(BotProgression.RequiresMount(evilSpirit), Is.False); + } + + /// + /// Tests that a skill with no granting item is not item-gated. + /// + [Test] + public void IsGrantingItemObtainable_NoGrantingItem_ReturnsTrue() + { + var config = new GameConfiguration(); + var characterClass = new CharacterClass(); + var skill = new Skill { Number = 9, Name = "Evil Spirit" }; + + Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 1, _ => 0f), Is.True); + } + + /// + /// Tests that a bot below the granting item's drop level cannot have the skill yet: with the item + /// dropping from level 50 on, a low-level bot with enough energy still must wait. + /// + [Test] + public void IsGrantingItemObtainable_BelowDropLevel_ReturnsFalse() + { + var (config, characterClass, skill) = CreateEvilSpiritSetup(dropLevel: 50); + float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 300f : null; + + Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 30, GetValue), Is.False); + } + + /// + /// Tests that the same bot may learn the skill once it reaches the drop level with the required energy. + /// + [Test] + public void IsGrantingItemObtainable_AtDropLevelWithRequirements_ReturnsTrue() + { + var (config, characterClass, skill) = CreateEvilSpiritSetup(dropLevel: 50); + float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 300f : null; + + Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 50, GetValue), Is.True); + } + + /// + /// Tests that the granting item must accept the bot's class. + /// + [Test] + public void IsGrantingItemObtainable_WrongClass_ReturnsFalse() + { + var (config, _, skill) = CreateEvilSpiritSetup(dropLevel: 50); + var otherClass = new CharacterClass { Number = 4 }; + float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 300f : null; + + Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, otherClass, 50, GetValue), Is.False); + } + + /// + /// Tests that the item's own requirements gate the skill: without the required energy the scroll + /// could not have been consumed, even at the drop level. + /// + [Test] + public void IsGrantingItemObtainable_RequirementsNotMet_ReturnsFalse() + { + var (config, characterClass, skill) = CreateEvilSpiritSetup(dropLevel: 50); + float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 100f : null; + + Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 50, GetValue), Is.False); + } + + /// + /// Tests that the scrolls' *RequirementValue attributes resolve to the same base stats as the + /// skills' totals, so the generation-time lookup finds the bot's stats. + /// + [Test] + public void TotalToBaseStat_RequirementValues_MapToBaseStats() + { + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalEnergyRequirementValue), Is.EqualTo(Stats.BaseEnergy)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalStrengthRequirementValue), Is.EqualTo(Stats.BaseStrength)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalAgilityRequirementValue), Is.EqualTo(Stats.BaseAgility)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalVitalityRequirementValue), Is.EqualTo(Stats.BaseVitality)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalLeadershipRequirementValue), Is.EqualTo(Stats.BaseLeadership)); + } + + private static (GameConfiguration Config, CharacterClass CharacterClass, Skill Skill) CreateEvilSpiritSetup(byte dropLevel) + { + var config = new TestGameConfiguration(); + var characterClass = new CharacterClass { Number = 0 }; + var skill = new Skill { Number = 9, Name = "Evil Spirit" }; + var scroll = new TestItemDefinition + { + Group = 15, + Number = 8, + Name = "Scroll of Evil Spirit", + DropLevel = dropLevel, + Skill = skill, + }; + scroll.QualifiedCharacters.Add(characterClass); + scroll.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergyRequirementValue, MinimumValue = 220 }); + config.Items.Add(scroll); + return (config, characterClass, skill); + } + + private sealed class TestGameConfiguration : GameConfiguration + { + public TestGameConfiguration() + { + this.Items = new List(); + } + } + + private sealed class TestItemDefinition : ItemDefinition + { + public TestItemDefinition() + { + this.Requirements = new List(); + this.QualifiedCharacters = new List(); + this.BasePowerUpAttributes = new List(); + } + } + + /// + /// Tests that mount-bound skills are never lootable: bots never use them, so the orb must stay + /// on the ground. + /// + [TestCase((short)47, "Impale")] + [TestCase((short)49, "Fire Breath")] + [TestCase((short)76, "Plasma Storm")] + public void IsBotLootableSkill_MountRequiredSkill_ReturnsFalse(short skillNumber, string name) + { + var skill = new Skill { Number = skillNumber, Name = name, SkillType = SkillType.DirectHit, AttackDamage = 15, NumberOfHitsPerAttack = 1 }; + + Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); + Assert.That(BotProgression.RequiresMount(skill), Is.True); + } + + /// + /// Tests that an ordinary orb-gated attack skill is lootable - including one the level-up + /// progression would never grant for free because the gate lives on its orb alone (it carries + /// no skill requirements of its own). + /// + [TestCase((short)9, "Evil Spirit", SkillType.AreaSkillAutomaticHits, 45)] + [TestCase((short)41, "Twisting Slash", SkillType.AreaSkillAutomaticHits, 0)] + public void IsBotLootableSkill_OrbGatedAttackSkill_ReturnsTrue(short skillNumber, string name, SkillType skillType, int attackDamage) + { + var skill = new Skill { Number = skillNumber, Name = name, SkillType = skillType, AttackDamage = attackDamage, NumberOfHitsPerAttack = 1 }; + + Assert.That(BotProgression.IsBotLootableSkill(skill), Is.True); + } + + /// + /// Tests that non-combat skills stay out of the loot rotation: summons, excluded buffs and + /// siege-only attacks alike. + /// + [Test] + public void IsBotLootableSkill_NonCombatSkill_ReturnsFalse() + { + var summonGoblin = new Skill { Number = 30, Name = "Summon Goblin", SkillType = SkillType.SummonMonster, AttackDamage = 0 }; + var defense = new Skill { Number = 18, Name = "Defense", SkillType = SkillType.Buff, AttackDamage = 0, MagicEffectDef = new MagicEffectDefinition() }; + var crescentMoon = new Skill { Number = 44, Name = "Crescent Moon Slash", SkillType = SkillType.DirectHit, AttackDamage = 90 }; + + Assert.That(BotProgression.IsBotLootableSkill(summonGoblin), Is.False); + Assert.That(BotProgression.IsBotLootableSkill(defense), Is.False); + Assert.That(BotProgression.IsBotLootableSkill(crescentMoon), Is.False); + } + + /// + /// Tests that a castable class buff with a magic effect is lootable from its orb. + /// + [Test] + public void IsBotLootableSkill_CastableBuff_ReturnsTrue() + { + var greaterDefense = new Skill { Number = 27, Name = "Greater Defense", SkillType = SkillType.Buff, AttackDamage = 0, MagicEffectDef = new MagicEffectDefinition() }; + + Assert.That(BotProgression.IsBotLootableSkill(greaterDefense), Is.True); + } + private sealed class SkillWithRequirements : Skill { public SkillWithRequirements(AttributeRequirement requirement) diff --git a/tests/MUnique.OpenMU.Tests/Offline/BotStartupProfileTests.cs b/tests/MUnique.OpenMU.Tests/Offline/BotStartupProfileTests.cs index 4dfbdfe8c9..33309f7754 100644 --- a/tests/MUnique.OpenMU.Tests/Offline/BotStartupProfileTests.cs +++ b/tests/MUnique.OpenMU.Tests/Offline/BotStartupProfileTests.cs @@ -13,8 +13,9 @@ namespace MUnique.OpenMU.Tests.Offline; public class BotStartupProfileTests { /// - /// Tests that the fresh profile always generates a level-1 character with level-0 starter gear and - /// no reset history, regardless of the veteran level bounds or the configured reset seeding. + /// Tests that the fresh profile always generates a level-1 character with level-0 starter gear, no + /// starter armor (weapon only, like a regular player's new character) and no reset history, + /// regardless of the veteran level bounds or the configured reset seeding. /// [Test] public void FreshProfile_AlwaysStartsAtLevelOne() @@ -28,6 +29,7 @@ public void FreshProfile_AlwaysStartsAtLevelOne() { Assert.That(level, Is.EqualTo(1)); Assert.That(profile.StarterItemLevel, Is.EqualTo(0)); + Assert.That(profile.EquipStarterArmor, Is.False); Assert.That(resets, Is.EqualTo(0)); Assert.That(profile.MinLevel, Is.EqualTo(1)); Assert.That(profile.MaxLevel, Is.EqualTo(1)); @@ -35,7 +37,8 @@ public void FreshProfile_AlwaysStartsAtLevelOne() } /// - /// Tests that the veteran profile rolls a level within the given bounds and keeps the +6 starter gear. + /// Tests that the veteran profile rolls a level within the given bounds and keeps the +6 starter + /// gear including the basic armor set. /// [Test] public void VeteranProfile_LevelStaysWithinBounds() @@ -48,6 +51,7 @@ public void VeteranProfile_LevelStaysWithinBounds() { Assert.That(level, Is.InRange(10, 250)); Assert.That(profile.StarterItemLevel, Is.EqualTo(6)); + Assert.That(profile.EquipStarterArmor, Is.True); }); } From 57ea7a08af9b83f3ecfa87abc47cf3f28cb0f389 Mon Sep 17 00:00:00 2001 From: Eduardo <6845999+eduardosmaniotto@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:03:08 -0300 Subject: [PATCH 2/4] Address review feedback on bot skill-access fix - Keep looted skill orbs out of the junk list, so they survive both selling and slot-pressure discarding until the next learn pass. - Drop the static granting-items cache; generation builds the lookup once per run instead of pinning the configuration graph. - Backfill consumable (orb/scroll) grants only; worn-equipment and pet grants are never written into learned skills. The raw requirement comparison is documented as consumable-only. - Replace IsBotLearnableSkill with IsBotLootableSkill plus per-call-site grant filtering; gate orb pickup behind the upgrade-items toggle. - Document the asymmetric handling of previously over-granted scroll skills in bots.md. - Cover BotSkillHandler and BotStarterGearEquipper with tests; port the grant-rule tests to MayBackfillSkill. --- docs-website/docs/server-features/bots.md | 6 +- src/GameLogic/Bots/BotGenerator.cs | 17 +- src/GameLogic/Bots/BotProgression.cs | 273 +++++++++--------- src/GameLogic/Bots/BotShoppingHandler.cs | 10 +- src/GameLogic/Bots/BotSkillHandler.cs | 8 +- .../Bots/BotSkillProgressionPlugIn.cs | 2 +- src/GameLogic/Offline/ItemPickupHandler.cs | 4 +- .../BotSkillHandlerTest.cs | 133 +++++++++ .../BotSkillRepertoireTest.cs | 13 +- .../BotStarterGearEquipperTest.cs | 160 ++++++++++ .../Offline/BotProgressionTests.cs | 261 ++++++++++------- 11 files changed, 622 insertions(+), 265 deletions(-) create mode 100644 tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs create mode 100644 tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs diff --git a/docs-website/docs/server-features/bots.md b/docs-website/docs/server-features/bots.md index 419c5fc1ad..ae343e565e 100644 --- a/docs-website/docs/server-features/bots.md +++ b/docs-website/docs/server-features/bots.md @@ -151,7 +151,11 @@ level and stats entitle it to (a fresh level-1 character almost none, a veteran a plausible kit up to its level), and from then on it loots skill orbs and scrolls from the ground and consumes them - never granted magically on level-up. A scroll which does not drop yet where the bot hunts stays unknown until the bot -gets there. Once learned, the class buffs are kept up on their own. +gets there, and skill-orb pickup follows the same upgrade-items toggle as gear. +Once learned, the class buffs are kept up on their own. Mount-bound previously +learned skills go quiet on their own (they are never selected), but scroll skills +a character learned too early keep working - regenerating the population with +`Reset bots` is the way to a clean slate. A skill the character cannot currently cast is passed over, in the attack rotation and in the buffs alike. That is not the same as not having learned it: a diff --git a/src/GameLogic/Bots/BotGenerator.cs b/src/GameLogic/Bots/BotGenerator.cs index 1adc890a9a..5d6bec48f3 100644 --- a/src/GameLogic/Bots/BotGenerator.cs +++ b/src/GameLogic/Bots/BotGenerator.cs @@ -524,10 +524,11 @@ private void CreateCharacter(IPlayerContext context, Account account, string nam /// as the class's own buffs and heals (e.g. elf Heal/Greater Defense/Greater Damage). Only skills the /// class is qualified for are ever learned, gated by the skills' real learn requirements from the game /// configuration (total energy, leadership, character level, ...) evaluated against the stats the bot - /// was just given - exactly the requirements a human player has to meet for the same skill. Orb and - /// scroll skills additionally require their granting item to be obtainable (see - /// ), so a bot cannot learn a scroll before the - /// monster level where it starts to drop. + /// was just given - exactly the requirements a human player has to meet for the same skill. Item-bound + /// skills follow the backfill rules (see ): orb/scroll + /// skills only when their granting item is obtainable, so a bot cannot learn a scroll before the + /// monster level where it starts to drop - and never when the skill comes from worn equipment or a + /// pet, which the server grants temporarily on equip instead. /// private void LearnClassSkills(IPlayerContext context, Character character, CharacterClass characterClass, int level) { @@ -544,13 +545,13 @@ private void LearnClassSkills(IPlayerContext context, Character character, Chara } var learnedNumbers = new HashSet(character.LearnedSkills.Select(s => s.Skill!.Number)); - var itemGrantedSkillNumbers = BotProgression.GetItemGrantedSkillNumbers(this._gameContext.Configuration); + var grantingItems = BotProgression.GetGrantingItems(this._gameContext.Configuration); foreach (var skill in this._gameContext.Configuration.Skills) { - if (!BotProgression.IsBotLearnableSkill(skill, itemGrantedSkillNumbers) + if (!BotProgression.IsBotLootableSkill(skill) || !skill.QualifiedCharacters.Contains(characterClass) || !BotProgression.MeetsRequirements(skill, GetValue) - || !BotProgression.IsGrantingItemObtainable(skill, this._gameContext.Configuration, characterClass, level, GetValue) + || !BotProgression.MayBackfillSkill(skill, grantingItems, characterClass, level, GetValue) || !learnedNumbers.Add(skill.Number)) { continue; @@ -562,6 +563,4 @@ private void LearnClassSkills(IPlayerContext context, Character character, Chara character.LearnedSkills.Add(entry); } } - - } diff --git a/src/GameLogic/Bots/BotProgression.cs b/src/GameLogic/Bots/BotProgression.cs index e96d89ab10..7ac9bc8894 100644 --- a/src/GameLogic/Bots/BotProgression.cs +++ b/src/GameLogic/Bots/BotProgression.cs @@ -4,9 +4,9 @@ namespace MUnique.OpenMU.GameLogic.Bots; -using System.Collections.Concurrent; using MUnique.OpenMU.AttributeSystem; using MUnique.OpenMU.DataModel.Configuration; +using MUnique.OpenMU.DataModel.Configuration.Items; using MUnique.OpenMU.GameLogic.Attributes; /// @@ -24,6 +24,12 @@ internal static class BotProgression /// public const int ClassEvolutionLevel = 200; + /// The item group of skill orbs. + internal const byte SkillOrbItemGroup = 12; + + /// The item group of skill scrolls and parchments. + internal const byte SkillScrollItemGroup = 15; + /// /// The character class numbers from the game's data model (CharacterClassNumber lives in the /// initialization assembly which GameLogic does not reference, so the relevant values are mirrored here). @@ -286,35 +292,7 @@ public static int GetVitalityTarget(string characterName) /// The game configuration which defines the items. /// The numbers of all skills which are granted by an item. public static IReadOnlySet GetItemGrantedSkillNumbers(GameConfiguration gameConfiguration) - => gameConfiguration.Items.Where(item => item.Skill is not null).Select(item => item.Skill!.Number).ToHashSet(); - - /// - /// Determines whether the skill is one a bot may learn: an actual attack skill, or a self/party - /// buff or heal with a magic effect (which the offline buff/heal handlers know how to cast). - /// Passive boosts, event skills, enemy debuffs and utility skills are left out. - /// - /// The skill to check. - /// The skills granted through items, see . - public static bool IsBotLearnableSkill(Skill skill, IReadOnlySet itemGrantedSkillNumbers) - { - if (skill.MasterDefinition is not null) - { - // Master skills are never learned for free - they cost the master points earned per master - // level and go through the regular action (see BotMasterHandler), like for a human player. - return false; - } - - if (itemGrantedSkillNumbers.Contains(skill.Number) && skill.Requirements is not { Count: > 0 }) - { - return false; - } - - // Worth learning if it adds damage of its own, hits more than once, or hits more than one - // target. Judging by AttackDamage alone would lock a Rage Fighter out of Chain Drive and - // Dragon Roar, which carry a flat bonus of zero and four hits instead, because their damage - // comes from the weapon - which is also how the server pays them out. - return IsBotLootableSkill(skill); - } + => (gameConfiguration.Items ?? []).Where(item => item.Skill is not null).Select(item => item.Skill!.Number).ToHashSet(); /// /// Determines whether the skill deals damage to a target, as opposed to buffing, summoning or the like. @@ -364,11 +342,13 @@ or SkillType.AreaSkillExplicitHits public static bool RequiresMount(Skill skill) => MountRequiredSkillNumbers.Contains(skill.Number); /// - /// Determines whether the skill is one a bot may pick up and learn from a looted orb or scroll, - /// like a human player: an actual attack skill or a castable self/party buff or heal - but never a - /// master skill, a castle-siege-only skill, or a mount-bound skill (never used by bots). Unlike - /// , item-granted skills are welcome here: the orb or scroll in - /// the bot's backpack is the gate, exactly as for a human consuming it. + /// Determines whether the skill is one a bot may own at all: an actual attack skill, or a + /// self/party buff or heal with a magic effect (which the offline buff/heal handlers know how to + /// cast) - but never a master skill (those go through the regular master action like for a human + /// player, see ), a castle-siege-only skill, or a mount-bound skill + /// (never used by bots). Item-granted skills are welcome here: the orb or scroll in the bot's + /// backpack is the gate, exactly as for a human consuming it. Passive boosts, event skills, enemy + /// debuffs and utility skills are left out. /// /// The skill to check. public static bool IsBotLootableSkill(Skill skill) @@ -381,6 +361,10 @@ public static bool IsBotLootableSkill(Skill skill) return false; } + // Worth owning if it adds damage of its own, hits more than once, or hits more than one + // target. Judging by AttackDamage alone would lock a Rage Fighter out of Chain Drive and + // Dragon Roar, which carry a flat bonus of zero and four hits instead, because their damage + // comes from the weapon - which is also how the server pays them out. if (IsAttackSkill(skill)) { return skill.AttackDamage > 0 @@ -394,129 +378,57 @@ public static bool IsBotLootableSkill(Skill skill) } /// - /// Determines whether the bot could plausibly own the item which teaches an orb/scroll skill: the - /// granting item must accept the bot's class, the bot's level must have reached the item's drop - /// level (the monster level where the item starts to drop, so a low-level character hunting where - /// it does not drop yet could not own one), and the bot must meet the item's own level and stat - /// requirements (the same gate a human faces at the consume handler). At least one granting item - /// must pass; a skill with no granting item at all is not item-gated and returns true. + /// Collects the items granting each skill, for one generation run. Built by the caller alongside + /// and handed into - no + /// static cache: generation runs rarely, and a cache keyed by the configuration would pin the whole + /// object graph for the process lifetime (and go stale against in-place admin-panel edits). /// - /// The skill whose granting item is checked. /// The game configuration which defines the items. - /// The bot's current character class. - /// The bot's current character level. + public static IReadOnlyDictionary> GetGrantingItems(GameConfiguration gameConfiguration) + => (gameConfiguration.Items ?? []) + .Where(item => item.Skill is not null) + .GroupBy(item => item.Skill!.Number) + .ToDictionary(group => group.Key, group => group.ToList()); + + /// + /// Determines whether the skill may be granted when a bot is generated (the backfill of what it + /// would have looted on its way up). A skill with no granting item at all passes; a skill granted + /// only by worn equipment or a pet never passes - those are learned temporarily by equipping the + /// item (which the server handles on its own), so writing them into the learned skills would make + /// them permanent. A consumable (orb/scroll) grant passes only when the skill carries requirements + /// of its own (otherwise the gate lives on the orb alone, and the loot path teaches it) and at + /// least one granting orb or scroll is obtainable: class-qualified, drop level reached, and the + /// item's own requirements met. + /// + /// The skill to check. + /// The granting items by skill number, see . + /// The bot's character class. + /// The bot's character level. /// Resolves an attribute's current value; null means unknown and fails. - public static bool IsGrantingItemObtainable( + public static bool MayBackfillSkill( Skill skill, - GameConfiguration gameConfiguration, + IReadOnlyDictionary> grantingItems, CharacterClass characterClass, int level, Func getAttributeValue) { - var grantingItems = GetGrantingItems(gameConfiguration, skill.Number); - if (grantingItems.Count == 0) + if (!grantingItems.TryGetValue(skill.Number, out var granting) || granting.Count == 0) { return true; } - return grantingItems.Any(item => IsObtainableGrantingItem(item, characterClass, level, getAttributeValue)); - } - - /// - /// Cache of the items granting each skill, per game configuration. Configurations are effectively - /// immutable at runtime (a reload builds a new instance), so a static cache keyed by the instance - /// is safe; it keeps the per-tick skill selection of hundreds of bots from re-scanning the whole - /// item list for every candidate skill. - /// - private static readonly ConcurrentDictionary>> GrantingItemsCache = new(); - - private static IReadOnlyList GetGrantingItems(GameConfiguration gameConfiguration, short skillNumber) - { - var bySkill = GrantingItemsCache.GetOrAdd( - gameConfiguration, - static config => (config.Items ?? []) - .Where(item => item.Skill is not null) - .GroupBy(item => item.Skill!.Number) - .ToDictionary(group => group.Key, group => group.ToList()) as IReadOnlyDictionary>); - return bySkill.TryGetValue(skillNumber, out var items) ? items : []; - } - - private static bool IsObtainableGrantingItem( - DataModel.Configuration.Items.ItemDefinition item, - CharacterClass characterClass, - int level, - Func getAttributeValue) - { - if (!item.QualifiedCharacters.Contains(characterClass)) + var consumable = granting.Where(IsConsumableSkillGrant).ToList(); + if (consumable.Count == 0) { return false; } - if (level < item.DropLevel) + if (skill.Requirements is not { Count: > 0 }) { return false; } - // The caller's getAttributeValue resolves TOTAL attributes (at generation time from base - // stats via TotalToBaseStat, at runtime from the live attribute graph) - exactly what - // MeetsRequirements expects. Item requirements use the same totals, except scrolls which - // use the *RequirementValue variants, so those are normalized first. Level is resolved - // from the passed level, which is also what the callers map Stats.Level to. - foreach (var requirement in item.Requirements) - { - if (requirement.Attribute is not { } attribute) - { - continue; - } - - if (attribute == Stats.Level) - { - if (level < requirement.MinimumValue) - { - return false; - } - - continue; - } - - var totalAttribute = NormalizeRequirementValue(attribute); - if (getAttributeValue(totalAttribute) is not { } value || value < requirement.MinimumValue) - { - return false; - } - } - - return true; - } - - private static AttributeDefinition NormalizeRequirementValue(AttributeDefinition attribute) - { - if (attribute == Stats.TotalEnergyRequirementValue) - { - return Stats.TotalEnergy; - } - - if (attribute == Stats.TotalStrengthRequirementValue) - { - return Stats.TotalStrength; - } - - if (attribute == Stats.TotalAgilityRequirementValue) - { - return Stats.TotalAgility; - } - - if (attribute == Stats.TotalVitalityRequirementValue) - { - return Stats.TotalVitality; - } - - if (attribute == Stats.TotalLeadershipRequirementValue) - { - return Stats.TotalLeadership; - } - - return attribute; + return consumable.Any(item => IsObtainableGrantingItem(item, characterClass, level, getAttributeValue)); } /// @@ -629,6 +541,91 @@ float ClassStat(AttributeDefinition attribute) return itemGroup <= maxMeleeGroup; } + private static bool IsConsumableSkillGrant(ItemDefinition item) + => item.Group == SkillOrbItemGroup || item.Group == SkillScrollItemGroup; + + private static bool IsObtainableGrantingItem( + ItemDefinition item, + CharacterClass characterClass, + int level, + Func getAttributeValue) + { + if (!item.QualifiedCharacters.Contains(characterClass)) + { + return false; + } + + if (level < item.DropLevel) + { + return false; + } + + // The caller's getAttributeValue resolves TOTAL attributes (at generation time from base + // stats via TotalToBaseStat) - exactly what MeetsRequirements expects. Item requirements use + // the same totals, except scrolls which use the *RequirementValue variants, so those are + // normalized first. Level is resolved from the passed level, which is also what the callers + // map Stats.Level to. + // Comparing the raw MinimumValue matches the real consume gate only because the caller + // (MayBackfillSkill) restricts this to non-wearable consumables: there GetRequirement + // returns the minimum unchanged, while wearable granting items scale it by item level and + // options (see ItemExtensions.GetRequirement) - which is why those never reach this check. + foreach (var requirement in item.Requirements) + { + if (requirement.Attribute is not { } attribute) + { + continue; + } + + if (attribute == Stats.Level) + { + if (level < requirement.MinimumValue) + { + return false; + } + + continue; + } + + var totalAttribute = NormalizeRequirementValue(attribute); + if (getAttributeValue(totalAttribute) is not { } value || value < requirement.MinimumValue) + { + return false; + } + } + + return true; + } + + private static AttributeDefinition NormalizeRequirementValue(AttributeDefinition attribute) + { + if (attribute == Stats.TotalEnergyRequirementValue) + { + return Stats.TotalEnergy; + } + + if (attribute == Stats.TotalStrengthRequirementValue) + { + return Stats.TotalStrength; + } + + if (attribute == Stats.TotalAgilityRequirementValue) + { + return Stats.TotalAgility; + } + + if (attribute == Stats.TotalVitalityRequirementValue) + { + return Stats.TotalVitality; + } + + if (attribute == Stats.TotalLeadershipRequirementValue) + { + return Stats.TotalLeadership; + } + + return attribute; + } + private static AttributeDefinition GetMainDamageStat(CharacterClass characterClass) { return characterClass.StatAttributes diff --git a/src/GameLogic/Bots/BotShoppingHandler.cs b/src/GameLogic/Bots/BotShoppingHandler.cs index f5cbccda7e..a9cd24852a 100644 --- a/src/GameLogic/Bots/BotShoppingHandler.cs +++ b/src/GameLogic/Bots/BotShoppingHandler.cs @@ -238,7 +238,15 @@ private static List GetSellableJunk(OfflinePlayer player, IStorage invento } // Whatever the bot would wear stays: selling a piece it picked up as an upgrade one tick - // before it puts it on is pure loss. + // before it puts it on is pure loss. The same holds for a looted orb or scroll waiting for + // the next learn pass (see BotSkillHandler) - it is not an upgrade, so without this guard it + // would fall straight through into the junk below. Keeping it out of the junk list also + // protects it from being destroyed as unsellable under slot pressure. + if (BotSkillHandler.WantsSkillItem(player, item)) + { + continue; + } + if (!BotEquipmentHandler.IsUpgradeFor(player, item)) { junk.Add(item); diff --git a/src/GameLogic/Bots/BotSkillHandler.cs b/src/GameLogic/Bots/BotSkillHandler.cs index b42b71ac0c..522fa4fd7c 100644 --- a/src/GameLogic/Bots/BotSkillHandler.cs +++ b/src/GameLogic/Bots/BotSkillHandler.cs @@ -18,12 +18,6 @@ namespace MUnique.OpenMU.GameLogic.Bots; /// internal static class BotSkillHandler { - /// The item group of skill orbs. - private const byte OrbGroup = 12; - - /// The item group of skill scrolls and parchments. - private const byte ScrollGroup = 15; - private static readonly ItemConsumeAction ConsumeAction = new(); /// @@ -44,7 +38,7 @@ public static bool WantsSkillItem(Player player, Item item) return false; } - if (definition.Group != OrbGroup && definition.Group != ScrollGroup) + if (definition.Group != BotProgression.SkillOrbItemGroup && definition.Group != BotProgression.SkillScrollItemGroup) { return false; } diff --git a/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs b/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs index c776470a6d..f3fbaf5f0b 100644 --- a/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs +++ b/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs @@ -166,7 +166,7 @@ private async ValueTask LearnNewSkillsAsync(Player player) foreach (var skill in player.GameContext.Configuration.Skills) { if (itemGrantedSkillNumbers.Contains(skill.Number) - || !BotProgression.IsBotLearnableSkill(skill, itemGrantedSkillNumbers) + || !BotProgression.IsBotLootableSkill(skill) || !skill.QualifiedCharacters.Contains(characterClass) || skillList.ContainsSkill((ushort)skill.Number) || !BotProgression.MeetsRequirements(skill, GetValue)) diff --git a/src/GameLogic/Offline/ItemPickupHandler.cs b/src/GameLogic/Offline/ItemPickupHandler.cs index bf8c0651eb..960f59df54 100644 --- a/src/GameLogic/Offline/ItemPickupHandler.cs +++ b/src/GameLogic/Offline/ItemPickupHandler.cs @@ -143,10 +143,12 @@ private bool ShouldPickUp(Item item) return true; } - if (this._player.Account?.IsBot == true && Bots.BotSkillHandler.WantsSkillItem(this._player, item)) + if (this._config.PickUpgradeItems && this._player.Account?.IsBot == true && Bots.BotSkillHandler.WantsSkillItem(this._player, item)) { // The item is an orb or scroll teaching a skill the bot does not know yet and may currently // consume - picked up like a human would; the BotSkillHandler consumes it on its next pass. + // Gated by the same upgrade-items toggle as gear: with loot progression off, orb and scroll + // skills stay out of reach, just like better gear does. return true; } diff --git a/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs b/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs new file mode 100644 index 0000000000..df8c64907d --- /dev/null +++ b/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs @@ -0,0 +1,133 @@ +// +// Licensed under the MIT License. See LICENSE file in the project root for full license information. +// + +namespace MUnique.OpenMU.Tests; + +using MUnique.OpenMU.DataModel.Configuration; +using MUnique.OpenMU.DataModel.Configuration.Items; +using MUnique.OpenMU.DataModel.Entities; +using MUnique.OpenMU.GameLogic.Attributes; +using MUnique.OpenMU.GameLogic.Bots; + +/// +/// Tests the loot predicate of - which dropped orbs and scrolls a bot +/// wants. The actual consumption goes through the regular consume handlers and is covered by +/// . +/// +[TestFixture] +public class BotSkillHandlerTest +{ + /// + /// Tests that an orb teaching an unknown, class-qualified, lootable skill with met requirements + /// is wanted. + /// + [Test] + public async ValueTask WantsSkillItem_UnknownLootableOrb_ReturnsTrue() + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + var characterClass = player.SelectedCharacter!.CharacterClass!; + var (skill, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); + var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; + + Assert.That(BotSkillHandler.WantsSkillItem(player, item), Is.True); + } + + /// + /// Tests that an orb for an already known skill is left alone. + /// + [Test] + public async ValueTask WantsSkillItem_AlreadyKnownSkill_ReturnsFalse() + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + var characterClass = player.SelectedCharacter!.CharacterClass!; + var (skill, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); + await player.SkillList!.AddLearnedSkillAsync(skill).ConfigureAwait(false); + var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; + + Assert.That(BotSkillHandler.WantsSkillItem(player, item), Is.False); + } + + /// + /// Tests that an orb for a mount-bound skill is left alone: bots never use those, even mounted. + /// + [Test] + public async ValueTask WantsSkillItem_MountBoundSkill_ReturnsFalse() + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + var characterClass = player.SelectedCharacter!.CharacterClass!; + var (_, orb) = CreateOrb(characterClass, 47, "Impale", 12, SkillType.DirectHit, attackDamage: 15); + var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; + + Assert.That(BotSkillHandler.WantsSkillItem(player, item), Is.False); + } + + /// + /// Tests that an orb whose requirements the bot does not meet is left alone. + /// + [Test] + public async ValueTask WantsSkillItem_RequirementsUnmet_ReturnsFalse() + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + var characterClass = player.SelectedCharacter!.CharacterClass!; + var (skill, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); + orb.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergy, MinimumValue = 500 }); + var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; + + Assert.That(BotSkillHandler.WantsSkillItem(player, item), Is.False); + } + + /// + /// Tests that only orbs and scrolls are wanted: a weapon carrying a skill teaches it temporarily + /// on equip instead, never by looting. + /// + [Test] + public async ValueTask WantsSkillItem_NonOrbScrollGroup_ReturnsFalse() + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + var characterClass = player.SelectedCharacter!.CharacterClass!; + var (_, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 0); + var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; + + Assert.That(BotSkillHandler.WantsSkillItem(player, item), Is.False); + } + + private static (Skill Skill, TestOrbDefinition Orb) CreateOrb(CharacterClass characterClass, short skillNumber, string skillName, byte group, SkillType skillType = SkillType.AreaSkillAutomaticHits, int attackDamage = 45) + { + var skill = new TestSkill + { + Number = skillNumber, + Name = skillName, + SkillType = skillType, + AttackDamage = attackDamage, + NumberOfHitsPerAttack = 1, + }; + skill.QualifiedCharacters.Add(characterClass); + var orb = new TestOrbDefinition + { + Group = group, + Number = 1, + Skill = skill, + }; + orb.QualifiedCharacters.Add(characterClass); + return (skill, orb); + } + + private sealed class TestSkill : Skill + { + public TestSkill() + { + this.QualifiedCharacters = new List(); + this.Requirements = new List(); + } + } + + private sealed class TestOrbDefinition : ItemDefinition + { + public TestOrbDefinition() + { + this.Requirements = new List(); + this.QualifiedCharacters = new List(); + } + } +} diff --git a/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs b/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs index 710e5333c8..c048e17da4 100644 --- a/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs +++ b/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs @@ -14,7 +14,6 @@ namespace MUnique.OpenMU.Tests; [TestFixture] public class BotSkillRepertoireTest { - private static readonly IReadOnlySet NoItemGrantedSkills = new HashSet(); /// /// 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 @@ -32,7 +31,7 @@ public void CastleSiegeSkillIsNotLearned(short skillNumber, string name) { var skill = CreateAttackSkill(skillNumber, attackDamage: 120, name: name); - Assert.That(BotProgression.IsBotLearnableSkill(skill, NoItemGrantedSkills), Is.False); + Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); } /// @@ -45,7 +44,7 @@ public void SiegeRoleSkillIsNotLearned() { var stun = CreateAttackSkill(67, attackDamage: 0, skillType: SkillType.AreaSkillAutomaticHits, name: "Stun"); - Assert.That(BotProgression.IsBotLearnableSkill(stun, NoItemGrantedSkills), Is.False); + Assert.That(BotProgression.IsBotLootableSkill(stun), Is.False); } /// @@ -60,7 +59,7 @@ public void MultiHitSkillWithoutFlatDamageIsLearned() { var chainDrive = CreateAttackSkill(262, attackDamage: 0, hits: 4, name: "Chain Drive"); - Assert.That(BotProgression.IsBotLearnableSkill(chainDrive, NoItemGrantedSkills), Is.True); + Assert.That(BotProgression.IsBotLootableSkill(chainDrive), Is.True); } /// @@ -72,7 +71,7 @@ public void AreaSkillWithoutFlatDamageIsLearned() { var tripleShot = CreateAttackSkill(24, attackDamage: 0, skillType: SkillType.AreaSkillAutomaticHits, name: "Triple Shot"); - Assert.That(BotProgression.IsBotLearnableSkill(tripleShot, NoItemGrantedSkills), Is.True); + Assert.That(BotProgression.IsBotLootableSkill(tripleShot), Is.True); } /// @@ -84,7 +83,7 @@ public void PlainSingleHitSkillWithoutDamageIsNotLearned() { var lunge = CreateAttackSkill(20, attackDamage: 0, name: "Lunge"); - Assert.That(BotProgression.IsBotLearnableSkill(lunge, NoItemGrantedSkills), Is.False); + Assert.That(BotProgression.IsBotLootableSkill(lunge), Is.False); } /// @@ -95,7 +94,7 @@ public void OrdinaryAttackSkillIsLearned() { var evilSpirit = CreateAttackSkill(9, attackDamage: 45, name: "Evil Spirit"); - Assert.That(BotProgression.IsBotLearnableSkill(evilSpirit, NoItemGrantedSkills), Is.True); + Assert.That(BotProgression.IsBotLootableSkill(evilSpirit), Is.True); } /// diff --git a/tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs b/tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs new file mode 100644 index 0000000000..9159ccd074 --- /dev/null +++ b/tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs @@ -0,0 +1,160 @@ +// +// Licensed under the MIT License. See LICENSE file in the project root for full license information. +// + +namespace MUnique.OpenMU.Tests; + +using Moq; +using MUnique.OpenMU.DataModel; +using MUnique.OpenMU.DataModel.Configuration; +using MUnique.OpenMU.DataModel.Configuration.Items; +using MUnique.OpenMU.DataModel.Entities; +using MUnique.OpenMU.GameLogic.Attributes; +using MUnique.OpenMU.GameLogic.Bots; +using MUnique.OpenMU.Persistence; + +/// +/// Tests the starter gear outfitting: a class-appropriate weapon at the profile's item level, a full +/// armor set with per-piece qualification, and the starting potion stacks. +/// +[TestFixture] +public class BotStarterGearEquipperTest +{ + private const byte StarterItemLevel = 3; + + /// + /// Tests that a weapon of the class's fighting style is equipped at the starter item level. + /// + [Test] + public void EquipWeapon_EquipsClassWeaponAtStarterLevel() + { + var (equipper, inventory, sword) = CreateEquipper(out _); + + equipper.EquipWeapon(); + + var equipped = inventory.Items.SingleOrDefault(i => i.ItemSlot == InventoryConstants.LeftHandSlot); + Assert.Multiple(() => + { + Assert.That(equipped, Is.Not.Null); + Assert.That(equipped!.Definition, Is.SameAs(sword)); + Assert.That(equipped.Level, Is.EqualTo(StarterItemLevel)); + }); + } + + /// + /// Tests that a full armor set is equipped when the class is qualified for every piece. + /// + [Test] + public void EquipArmorSet_EquipsFullQualifiedSet() + { + var (equipper, inventory, _) = CreateEquipper(out _); + + equipper.EquipArmorSet(); + + Assert.Multiple(() => + { + Assert.That(inventory.Items.Count(i => i.ItemSlot == InventoryConstants.HelmSlot), Is.EqualTo(1)); + Assert.That(inventory.Items.Count(i => i.ItemSlot == InventoryConstants.ArmorSlot), Is.EqualTo(1)); + Assert.That(inventory.Items.Count(i => i.ItemSlot == InventoryConstants.PantsSlot), Is.EqualTo(1)); + Assert.That(inventory.Items.Count(i => i.ItemSlot == InventoryConstants.GlovesSlot), Is.EqualTo(1)); + Assert.That(inventory.Items.Count(i => i.ItemSlot == InventoryConstants.BootsSlot), Is.EqualTo(1)); + Assert.That(inventory.Items.Where(i => i.ItemSlot != InventoryConstants.LeftHandSlot).Select(i => i.Level), Is.All.EqualTo(StarterItemLevel)); + }); + } + + /// + /// Tests that a piece the class is not qualified for is skipped while the rest of the set is equipped. + /// + [Test] + public void EquipArmorSet_SkipsUnqualifiedPiece() + { + var (equipper, inventory, _) = CreateEquipper(out var definitions); + definitions.OfType().First(d => d.Group == 10).QualifiedCharacters.Clear(); + + equipper.EquipArmorSet(); + + Assert.Multiple(() => + { + Assert.That(inventory.Items.Count, Is.EqualTo(4)); + Assert.That(inventory.Items.Any(i => i.ItemSlot == InventoryConstants.GlovesSlot), Is.False); + }); + } + + /// + /// Tests that the starting potion stacks land in the first backpack slots. + /// + [Test] + public void AddPotions_AddsTwoStacks() + { + var (equipper, inventory, _) = CreateEquipper(out _); + + equipper.AddPotions(); + + Assert.Multiple(() => + { + Assert.That(inventory.Items.Count(i => i.ItemSlot == InventoryConstants.EquippableSlotsCount), Is.EqualTo(1)); + Assert.That(inventory.Items.Count(i => i.ItemSlot == InventoryConstants.EquippableSlotsCount + 1), Is.EqualTo(1)); + }); + } + + private static (BotStarterGearEquipper Equipper, ItemStorage Inventory, TestGearItemDefinition Sword) CreateEquipper(out List definitions) + { + var characterClass = new TestCharacterClass(); + + var characterMock = new Mock(); + characterMock.SetupAllProperties(); + characterMock.Setup(c => c.CharacterClass).Returns(characterClass); + var inventoryItems = new List(); + var inventoryMock = new Mock(); + inventoryMock.SetupAllProperties(); + inventoryMock.Setup(i => i.Items).Returns(inventoryItems); + characterMock.Setup(c => c.Inventory).Returns(inventoryMock.Object); + + var sword = new TestGearItemDefinition { Group = 0, Number = 0, DropLevel = 5, Durability = 10 }; + sword.QualifiedCharacters.Add(characterClass); + definitions = new List { sword }; + foreach (var (group, number) in new[] { (7, 5), (8, 5), (9, 5), (10, 5), (11, 5) }) + { + var piece = new TestGearItemDefinition { Group = (byte)group, Number = (byte)number, DropLevel = 5, Durability = 10 }; + piece.QualifiedCharacters.Add(characterClass); + definitions.Add(piece); + } + + foreach (var (group, number) in new[] { (14, 3), (14, 6) }) + { + definitions.Add(new TestGearItemDefinition { Group = (byte)group, Number = (byte)number }); + } + + var configMock = new Mock(); + configMock.Setup(c => c.Items).Returns(definitions); + + var contextMock = new Mock(); + contextMock.Setup(m => m.CreateNew(It.IsAny())).Returns(() => new Item()); + + var equipper = new BotStarterGearEquipper(contextMock.Object, configMock.Object, characterMock.Object, StarterItemLevel); + return (equipper, inventoryMock.Object, sword); + } + + private sealed class TestGearItemDefinition : ItemDefinition + { + public TestGearItemDefinition() + { + this.Requirements = new List(); + this.QualifiedCharacters = new List(); + } + } + + private sealed class TestCharacterClass : CharacterClass + { + public TestCharacterClass() + { + this.Number = 4; + this.StatAttributes = new List + { + new(Stats.BaseStrength, 30, true), + new(Stats.BaseAgility, 15, true), + new(Stats.BaseEnergy, 10, true), + }; + } + } +} diff --git a/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs b/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs index a8648f6289..813a761388 100644 --- a/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs +++ b/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs @@ -16,7 +16,6 @@ namespace MUnique.OpenMU.Tests.Offline; [TestFixture] public class BotProgressionTests { - private static readonly IReadOnlySet NoItemGrantedSkills = new HashSet(); /// /// Tests that the split assigns all points proportionally when nothing is capped. /// @@ -81,13 +80,10 @@ public void GetVitalityTarget_IsStableAndWithinRange() } /// - /// Tests that skills only ever obtained by consuming an orb/scroll or equipping a weapon or pet - /// carrying the skill are never learned for free: the gate lives on that item, not on the skill, so - /// would find nothing to fail. The set of item-granted skills comes - /// from the game configuration (), which - /// covers orbs, scrolls, weapons and pets alike. + /// Tests that orb/scroll skills carrying no requirements of their own are never backfilled at + /// generation: the gate lives on the orb or scroll alone, so the loot path teaches them instead. /// - /// The number of an item-granted skill. + /// The number of an orb-gated skill. [TestCase((short)41, "Twisting Slash")] [TestCase((short)51, "Ice Arrow")] [TestCase((short)55, "Fire Slash")] @@ -97,8 +93,9 @@ public void GetVitalityTarget_IsStableAndWithinRange() [TestCase((short)260, "Killing Blow")] [TestCase((short)261, "Beast Uppercut")] [TestCase((short)270, "Phoenix Shot")] - public void IsBotLearnableSkill_ItemGrantedSkillWithoutOwnRequirements_ReturnsFalse(short skillNumber, string name) + public void MayBackfillSkill_ConsumableGrantWithoutSkillRequirements_ReturnsFalse(short skillNumber, string name) { + var characterClass = new CharacterClass(); var skill = new Skill { Number = skillNumber, @@ -107,8 +104,17 @@ public void IsBotLearnableSkill_ItemGrantedSkillWithoutOwnRequirements_ReturnsFa AttackDamage = 0, NumberOfHitsPerAttack = 4, }; + var orb = new TestItemDefinition + { + Group = 12, + Number = 1, + DropLevel = 1, + Skill = skill, + }; + orb.QualifiedCharacters.Add(characterClass); + var grants = new Dictionary> { [skillNumber] = new List { orb } }; - Assert.That(BotProgression.IsBotLearnableSkill(skill, new HashSet { skillNumber }), Is.False); + Assert.That(BotProgression.MayBackfillSkill(skill, grants, characterClass, 1, _ => 0f), Is.False); } /// @@ -117,7 +123,7 @@ public void IsBotLearnableSkill_ItemGrantedSkillWithoutOwnRequirements_ReturnsFa /// missing item would otherwise give it away for free. /// [Test] - public void IsBotLearnableSkill_ExplicitlyExcludedWithoutGrantingItem_ReturnsFalse() + public void IsBotLootableSkill_ExplicitlyExcludedWithoutGrantingItem_ReturnsFalse() { var skill = new Skill { @@ -128,17 +134,19 @@ public void IsBotLearnableSkill_ExplicitlyExcludedWithoutGrantingItem_ReturnsFal NumberOfHitsPerAttack = 4, }; - Assert.That(BotProgression.IsBotLearnableSkill(skill, NoItemGrantedSkills), Is.False); + Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); } /// - /// Tests that an item-granted skill which also carries requirements of its own stays learnable: the - /// item is not the gate, the requirements are. Rageful Blow is granted by an orb yet demands level - /// 170 (see the initialization), and a bot meeting that requirement may use it like any player. + /// Tests that an orb/scroll skill which also carries requirements of its own is backfilled once its + /// granting orb is obtainable: the item is not the only gate, the requirements are. Rageful Blow is + /// granted by an orb yet demands level 170 (see the initialization), and a bot meeting that + /// requirement may use it like any player. /// [Test] - public void IsBotLearnableSkill_ItemGrantedSkillWithOwnRequirements_ReturnsTrue() + public void MayBackfillSkill_ObtainableOrbWithSkillRequirements_ReturnsTrue() { + var characterClass = new CharacterClass(); var skill = new SkillWithRequirements(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = 170 }) { Number = 42, @@ -147,8 +155,18 @@ public void IsBotLearnableSkill_ItemGrantedSkillWithOwnRequirements_ReturnsTrue( AttackDamage = 60, NumberOfHitsPerAttack = 1, }; + var orb = new TestItemDefinition + { + Group = 12, + Number = 12, + DropLevel = 78, + Skill = skill, + }; + orb.QualifiedCharacters.Add(characterClass); + orb.Requirements.Add(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = 170 }); + var grants = new Dictionary> { [42] = new List { orb } }; - Assert.That(BotProgression.IsBotLearnableSkill(skill, new HashSet { 42 }), Is.True); + Assert.That(BotProgression.MayBackfillSkill(skill, grants, characterClass, 170, _ => 0f), Is.True); } /// @@ -164,7 +182,7 @@ public void IsBotLearnableSkill_ItemGrantedSkillWithOwnRequirements_ReturnsTrue( [TestCase((short)73, "Mana Rays")] [TestCase((short)74, "Fire Blast")] [TestCase((short)269, "Charge")] - public void IsBotLearnableSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, string name) + public void IsBotLootableSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, string name) { var skill = new Skill { @@ -175,7 +193,7 @@ public void IsBotLearnableSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, NumberOfHitsPerAttack = 4, }; - Assert.That(BotProgression.IsBotLearnableSkill(skill, NoItemGrantedSkills), Is.False); + Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); } /// @@ -189,7 +207,7 @@ public void IsBotLearnableSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, [TestCase((short)70, "Invisibility")] [TestCase((short)71, "Cancel Invisibility")] [TestCase((short)72, "Abolish Magic")] - public void IsBotLearnableSkill_CastleSiegeRoleSkill_ReturnsFalse(short skillNumber, string name) + public void IsBotLootableSkill_CastleSiegeRoleSkill_ReturnsFalse(short skillNumber, string name) { var skill = new Skill { @@ -200,29 +218,7 @@ public void IsBotLearnableSkill_CastleSiegeRoleSkill_ReturnsFalse(short skillNum NumberOfHitsPerAttack = 1, }; - Assert.That(BotProgression.IsBotLearnableSkill(skill, NoItemGrantedSkills), Is.False); - } - - /// - /// Tests that mount-bound skills are never learned by a bot - a bot fighting with one on foot - /// does something no player can do. - /// - [TestCase((short)47, "Impale", 28)] - [TestCase((short)49, "Fire Breath", 110)] - [TestCase((short)76, "Plasma Storm", 110)] - public void IsBotLearnableSkill_MountRequiredSkill_ReturnsFalse(short skillNumber, string name, int levelRequirement) - { - var skill = new SkillWithRequirements(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = levelRequirement }) - { - Number = skillNumber, - Name = name, - SkillType = SkillType.DirectHit, - AttackDamage = 15, - NumberOfHitsPerAttack = 1, - }; - - Assert.That(BotProgression.IsBotLearnableSkill(skill, NoItemGrantedSkills), Is.False); - Assert.That(BotProgression.RequiresMount(skill), Is.True); + Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); } /// @@ -244,118 +240,152 @@ public void RequiresMount_OrdinarySkill_ReturnsFalse() } /// - /// Tests that a skill with no granting item is not item-gated. + /// Tests that a skill with no granting item at all is backfilled freely. /// [Test] - public void IsGrantingItemObtainable_NoGrantingItem_ReturnsTrue() + public void MayBackfillSkill_NoGrantingItem_ReturnsTrue() { - var config = new GameConfiguration(); - var characterClass = new CharacterClass(); + var grants = new Dictionary>(); var skill = new Skill { Number = 9, Name = "Evil Spirit" }; - Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 1, _ => 0f), Is.True); + Assert.That(BotProgression.MayBackfillSkill(skill, grants, new CharacterClass(), 1, _ => 0f), Is.True); } /// - /// Tests that a bot below the granting item's drop level cannot have the skill yet: with the item - /// dropping from level 50 on, a low-level bot with enough energy still must wait. + /// Tests that a bot below the granting orb/scroll's drop level is not backfilled: with the item + /// dropping from level 50 on, a low-level bot with enough energy still must wait for the loot path. /// [Test] - public void IsGrantingItemObtainable_BelowDropLevel_ReturnsFalse() + public void MayBackfillSkill_BelowDropLevel_ReturnsFalse() { - var (config, characterClass, skill) = CreateEvilSpiritSetup(dropLevel: 50); + var (grants, characterClass, skill) = CreateScrollGrant(dropLevel: 50); float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 300f : null; - Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 30, GetValue), Is.False); + Assert.That(BotProgression.MayBackfillSkill(skill, grants, characterClass, 30, GetValue), Is.False); } /// - /// Tests that the same bot may learn the skill once it reaches the drop level with the required energy. + /// Tests that the same bot is backfilled once it reaches the drop level with the required energy. /// [Test] - public void IsGrantingItemObtainable_AtDropLevelWithRequirements_ReturnsTrue() + public void MayBackfillSkill_AtDropLevelWithRequirements_ReturnsTrue() { - var (config, characterClass, skill) = CreateEvilSpiritSetup(dropLevel: 50); + var (grants, characterClass, skill) = CreateScrollGrant(dropLevel: 50); float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 300f : null; - Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 50, GetValue), Is.True); + Assert.That(BotProgression.MayBackfillSkill(skill, grants, characterClass, 50, GetValue), Is.True); } /// - /// Tests that the granting item must accept the bot's class. + /// Tests that the granting orb/scroll must accept the bot's class. /// [Test] - public void IsGrantingItemObtainable_WrongClass_ReturnsFalse() + public void MayBackfillSkill_WrongClass_ReturnsFalse() { - var (config, _, skill) = CreateEvilSpiritSetup(dropLevel: 50); + var (grants, _, skill) = CreateScrollGrant(dropLevel: 50); var otherClass = new CharacterClass { Number = 4 }; float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 300f : null; - Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, otherClass, 50, GetValue), Is.False); + Assert.That(BotProgression.MayBackfillSkill(skill, grants, otherClass, 50, GetValue), Is.False); } /// - /// Tests that the item's own requirements gate the skill: without the required energy the scroll - /// could not have been consumed, even at the drop level. + /// Tests that the orb/scroll's own requirements gate the backfill: without the required energy the + /// scroll could not have been consumed, even at the drop level. /// [Test] - public void IsGrantingItemObtainable_RequirementsNotMet_ReturnsFalse() + public void MayBackfillSkill_RequirementsNotMet_ReturnsFalse() { - var (config, characterClass, skill) = CreateEvilSpiritSetup(dropLevel: 50); + var (grants, characterClass, skill) = CreateScrollGrant(dropLevel: 50); float? GetValue(AttributeDefinition attribute) => attribute == Stats.TotalEnergy ? 100f : null; - Assert.That(BotProgression.IsGrantingItemObtainable(skill, config, characterClass, 50, GetValue), Is.False); + Assert.That(BotProgression.MayBackfillSkill(skill, grants, characterClass, 50, GetValue), Is.False); } /// - /// Tests that the scrolls' *RequirementValue attributes resolve to the same base stats as the - /// skills' totals, so the generation-time lookup finds the bot's stats. + /// Tests that a skill granted only by worn equipment or a pet is never backfilled, even with stat + /// requirements of its own: those are learned temporarily by equipping the item, so writing them + /// into the learned skills would make them permanent. /// [Test] - public void TotalToBaseStat_RequirementValues_MapToBaseStats() + public void MayBackfillSkill_EquipmentGrantedOnly_ReturnsFalse() { - Assert.That(BotProgression.TotalToBaseStat(Stats.TotalEnergyRequirementValue), Is.EqualTo(Stats.BaseEnergy)); - Assert.That(BotProgression.TotalToBaseStat(Stats.TotalStrengthRequirementValue), Is.EqualTo(Stats.BaseStrength)); - Assert.That(BotProgression.TotalToBaseStat(Stats.TotalAgilityRequirementValue), Is.EqualTo(Stats.BaseAgility)); - Assert.That(BotProgression.TotalToBaseStat(Stats.TotalVitalityRequirementValue), Is.EqualTo(Stats.BaseVitality)); - Assert.That(BotProgression.TotalToBaseStat(Stats.TotalLeadershipRequirementValue), Is.EqualTo(Stats.BaseLeadership)); - } - - private static (GameConfiguration Config, CharacterClass CharacterClass, Skill Skill) CreateEvilSpiritSetup(byte dropLevel) - { - var config = new TestGameConfiguration(); - var characterClass = new CharacterClass { Number = 0 }; - var skill = new Skill { Number = 9, Name = "Evil Spirit" }; - var scroll = new TestItemDefinition + var characterClass = new CharacterClass { Number = 4 }; + var skill = new SkillWithRequirements(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = 110 }) { - Group = 15, - Number = 8, - Name = "Scroll of Evil Spirit", - DropLevel = dropLevel, + Number = 49, + Name = "Fire Breath", + SkillType = SkillType.DirectHit, + AttackDamage = 30, + NumberOfHitsPerAttack = 1, + }; + var pet = new TestItemDefinition + { + Group = 13, + Number = 3, + Name = "Horn of Dinorant", + DropLevel = 110, Skill = skill, }; - scroll.QualifiedCharacters.Add(characterClass); - scroll.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergyRequirementValue, MinimumValue = 220 }); - config.Items.Add(scroll); - return (config, characterClass, skill); + pet.QualifiedCharacters.Add(characterClass); + var grants = new Dictionary> { [skill.Number] = new List { pet } }; + + Assert.That(BotProgression.MayBackfillSkill(skill, grants, characterClass, 110, _ => 0f), Is.False); } - private sealed class TestGameConfiguration : GameConfiguration + /// + /// Tests that a worn-equipment grant does not block the backfill when an obtainable orb exists + /// alongside it: the permanent skill comes from the consumable, the equipment grant is temporary. + /// + [Test] + public void MayBackfillSkill_MixedGrantsWithObtainableOrb_ReturnsTrue() { - public TestGameConfiguration() + var characterClass = new CharacterClass { Number = 4 }; + var skill = new SkillWithRequirements(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = 170 }) { - this.Items = new List(); - } + Number = 42, + Name = "Rageful Blow", + SkillType = SkillType.AreaSkillAutomaticHits, + AttackDamage = 60, + NumberOfHitsPerAttack = 1, + }; + var weapon = new TestItemDefinition + { + Group = 0, + Number = 1, + Name = "Blade of Rageful Blow", + DropLevel = 78, + Skill = skill, + }; + weapon.QualifiedCharacters.Add(characterClass); + var orb = new TestItemDefinition + { + Group = 12, + Number = 12, + Name = "Orb of Rageful Blow", + DropLevel = 78, + Skill = skill, + }; + orb.QualifiedCharacters.Add(characterClass); + orb.Requirements.Add(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = 170 }); + var grants = new Dictionary> { [skill.Number] = new List { weapon, orb } }; + float? GetValue(AttributeDefinition attribute) => attribute == Stats.Level ? 170f : null; + + Assert.That(BotProgression.MayBackfillSkill(skill, grants, characterClass, 170, GetValue), Is.True); } - private sealed class TestItemDefinition : ItemDefinition + /// + /// Tests that the scrolls' *RequirementValue attributes resolve to the same base stats as the + /// skills' totals, so the generation-time lookup finds the bot's stats. + /// + [Test] + public void TotalToBaseStat_RequirementValues_MapToBaseStats() { - public TestItemDefinition() - { - this.Requirements = new List(); - this.QualifiedCharacters = new List(); - this.BasePowerUpAttributes = new List(); - } + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalEnergyRequirementValue), Is.EqualTo(Stats.BaseEnergy)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalStrengthRequirementValue), Is.EqualTo(Stats.BaseStrength)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalAgilityRequirementValue), Is.EqualTo(Stats.BaseAgility)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalVitalityRequirementValue), Is.EqualTo(Stats.BaseVitality)); + Assert.That(BotProgression.TotalToBaseStat(Stats.TotalLeadershipRequirementValue), Is.EqualTo(Stats.BaseLeadership)); } /// @@ -414,6 +444,37 @@ public void IsBotLootableSkill_CastableBuff_ReturnsTrue() Assert.That(BotProgression.IsBotLootableSkill(greaterDefense), Is.True); } + private static (Dictionary> Grants, CharacterClass CharacterClass, Skill Skill) CreateScrollGrant(byte dropLevel) + { + var characterClass = new CharacterClass { Number = 0 }; + var skill = new SkillWithRequirements(new AttributeRequirement { Attribute = Stats.TotalEnergy, MinimumValue = 220 }) + { + Number = 9, + Name = "Evil Spirit", + }; + var scroll = new TestItemDefinition + { + Group = 15, + Number = 8, + Name = "Scroll of Evil Spirit", + DropLevel = dropLevel, + Skill = skill, + }; + scroll.QualifiedCharacters.Add(characterClass); + scroll.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergyRequirementValue, MinimumValue = 220 }); + var grants = new Dictionary> { [skill.Number] = new List { scroll } }; + return (grants, characterClass, skill); + } + + private sealed class TestItemDefinition : ItemDefinition + { + public TestItemDefinition() + { + this.Requirements = new List(); + this.QualifiedCharacters = new List(); + } + } + private sealed class SkillWithRequirements : Skill { public SkillWithRequirements(AttributeRequirement requirement) From c547606192f0e88237013fbb57588aafa6eda547 Mon Sep 17 00:00:00 2001 From: Eduardo <6845999+eduardosmaniotto@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:20:53 -0300 Subject: [PATCH 3/4] address codacy warning --- tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs b/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs index df8c64907d..465815eeda 100644 --- a/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs +++ b/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs @@ -27,7 +27,7 @@ public async ValueTask WantsSkillItem_UnknownLootableOrb_ReturnsTrue() { var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); var characterClass = player.SelectedCharacter!.CharacterClass!; - var (skill, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); + var (_, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; Assert.That(BotSkillHandler.WantsSkillItem(player, item), Is.True); @@ -70,7 +70,7 @@ public async ValueTask WantsSkillItem_RequirementsUnmet_ReturnsFalse() { var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); var characterClass = player.SelectedCharacter!.CharacterClass!; - var (skill, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); + var (_, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); orb.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergy, MinimumValue = 500 }); var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; From a1acb3f5510e035dddf126b886f5c71316145b7a Mon Sep 17 00:00:00 2001 From: Eduardo <6845999+eduardosmaniotto@users.noreply.github.com> Date: Sun, 6 Sep 2026 17:10:09 -0300 Subject: [PATCH 4/4] Address second review: skill-list race, gate naming, test precision - Fix the skill-list race the sell guard introduced: GetSellableJunk no longer reads the skill list, so the navigator-timer shopping decision stays race-free. Pending skill orbs are pulled back out of the junk list in SellJunkAsync instead, which runs serialized on the MU Helper tick with the learn pass that mutates it. - Rename IsBotLootableSkill to MayBotOwnSkill to match its role as the gate for looting, generation backfill and level-up alike. - Tests: realistic TotalEnergyRequirementValue on the orb fixture, non-mount equipment-grant case, and a staff-vs-sword proof that build preference wins over drop level. --- src/GameLogic/Bots/BotGenerator.cs | 2 +- src/GameLogic/Bots/BotProgression.cs | 2 +- src/GameLogic/Bots/BotShoppingHandler.cs | 24 +++++----- src/GameLogic/Bots/BotSkillHandler.cs | 2 +- .../Bots/BotSkillProgressionPlugIn.cs | 2 +- .../BotSkillHandlerTest.cs | 2 +- .../BotSkillRepertoireTest.cs | 12 ++--- .../BotStarterGearEquipperTest.cs | 8 +++- .../Offline/BotProgressionTests.cs | 44 +++++++++---------- 9 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/GameLogic/Bots/BotGenerator.cs b/src/GameLogic/Bots/BotGenerator.cs index 5d6bec48f3..b78a47ab79 100644 --- a/src/GameLogic/Bots/BotGenerator.cs +++ b/src/GameLogic/Bots/BotGenerator.cs @@ -548,7 +548,7 @@ private void LearnClassSkills(IPlayerContext context, Character character, Chara var grantingItems = BotProgression.GetGrantingItems(this._gameContext.Configuration); foreach (var skill in this._gameContext.Configuration.Skills) { - if (!BotProgression.IsBotLootableSkill(skill) + if (!BotProgression.MayBotOwnSkill(skill) || !skill.QualifiedCharacters.Contains(characterClass) || !BotProgression.MeetsRequirements(skill, GetValue) || !BotProgression.MayBackfillSkill(skill, grantingItems, characterClass, level, GetValue) diff --git a/src/GameLogic/Bots/BotProgression.cs b/src/GameLogic/Bots/BotProgression.cs index 7ac9bc8894..d06de7f99e 100644 --- a/src/GameLogic/Bots/BotProgression.cs +++ b/src/GameLogic/Bots/BotProgression.cs @@ -351,7 +351,7 @@ or SkillType.AreaSkillExplicitHits /// debuffs and utility skills are left out. /// /// The skill to check. - public static bool IsBotLootableSkill(Skill skill) + public static bool MayBotOwnSkill(Skill skill) { if (skill.MasterDefinition is not null || CastleSiegeOnlySkillNumbers.Contains(skill.Number) diff --git a/src/GameLogic/Bots/BotShoppingHandler.cs b/src/GameLogic/Bots/BotShoppingHandler.cs index a9cd24852a..93fa777d0f 100644 --- a/src/GameLogic/Bots/BotShoppingHandler.cs +++ b/src/GameLogic/Bots/BotShoppingHandler.cs @@ -238,15 +238,7 @@ private static List GetSellableJunk(OfflinePlayer player, IStorage invento } // Whatever the bot would wear stays: selling a piece it picked up as an upgrade one tick - // before it puts it on is pure loss. The same holds for a looted orb or scroll waiting for - // the next learn pass (see BotSkillHandler) - it is not an upgrade, so without this guard it - // would fall straight through into the junk below. Keeping it out of the junk list also - // protects it from being destroyed as unsellable under slot pressure. - if (BotSkillHandler.WantsSkillItem(player, item)) - { - continue; - } - + // before it puts it on is pure loss. if (!BotEquipmentHandler.IsUpgradeFor(player, item)) { junk.Add(item); @@ -263,14 +255,24 @@ private static List GetSellableJunk(OfflinePlayer player, IStorage invento /// private static async ValueTask<(int Sold, List Unsold)> SellJunkAsync(OfflinePlayer player, IStorage inventory) { - var junk = GetSellableJunk(player, inventory) + var junk = GetSellableJunk(player, inventory); + + // A looted orb or scroll waiting for the next learn pass is not an upgrade, so the classifier + // above files it as junk - pull it back out here, where reading the skill list is safe: this + // runs on the MU Helper tick, serialized with the learn pass that mutates it (never on the + // navigator's timer, where NeedsShopping only decides whether a trip is worthwhile). Keeping it + // out of this list also protects it from being destroyed as unsellable under slot pressure + // downstream, which only ever sees what is returned here. + junk.RemoveAll(item => BotSkillHandler.WantsSkillItem(player, item)); + + var pricedJunk = junk .Select(i => (Item: i, Price: PriceCalculator.CalculateSellingPrice(i, i.Durability()))) .OrderByDescending(x => x.Price) .ToList(); var sold = 0; var unsold = new List(); - foreach (var (item, _) in junk) + foreach (var (item, _) in pricedJunk) { if (await SellAction.SellItemAsync(player, item.ItemSlot).ConfigureAwait(false)) { diff --git a/src/GameLogic/Bots/BotSkillHandler.cs b/src/GameLogic/Bots/BotSkillHandler.cs index 522fa4fd7c..505c8c3add 100644 --- a/src/GameLogic/Bots/BotSkillHandler.cs +++ b/src/GameLogic/Bots/BotSkillHandler.cs @@ -50,7 +50,7 @@ public static bool WantsSkillItem(Player player, Item item) if (!skill.QualifiedCharacters.Contains(characterClass) || !definition.QualifiedCharacters.Contains(characterClass) - || !BotProgression.IsBotLootableSkill(skill)) + || !BotProgression.MayBotOwnSkill(skill)) { return false; } diff --git a/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs b/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs index f3fbaf5f0b..55769b9816 100644 --- a/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs +++ b/src/GameLogic/Bots/BotSkillProgressionPlugIn.cs @@ -166,7 +166,7 @@ private async ValueTask LearnNewSkillsAsync(Player player) foreach (var skill in player.GameContext.Configuration.Skills) { if (itemGrantedSkillNumbers.Contains(skill.Number) - || !BotProgression.IsBotLootableSkill(skill) + || !BotProgression.MayBotOwnSkill(skill) || !skill.QualifiedCharacters.Contains(characterClass) || skillList.ContainsSkill((ushort)skill.Number) || !BotProgression.MeetsRequirements(skill, GetValue)) diff --git a/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs b/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs index 465815eeda..a87b77443e 100644 --- a/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs +++ b/tests/MUnique.OpenMU.Tests/BotSkillHandlerTest.cs @@ -71,7 +71,7 @@ public async ValueTask WantsSkillItem_RequirementsUnmet_ReturnsFalse() var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); var characterClass = player.SelectedCharacter!.CharacterClass!; var (_, orb) = CreateOrb(characterClass, 9, "Evil Spirit", 12); - orb.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergy, MinimumValue = 500 }); + orb.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergyRequirementValue, MinimumValue = 500 }); var item = new Item { Definition = orb, Durability = 1, ItemSlot = 12 }; Assert.That(BotSkillHandler.WantsSkillItem(player, item), Is.False); diff --git a/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs b/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs index c048e17da4..2b18c66fe3 100644 --- a/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs +++ b/tests/MUnique.OpenMU.Tests/BotSkillRepertoireTest.cs @@ -31,7 +31,7 @@ public void CastleSiegeSkillIsNotLearned(short skillNumber, string name) { var skill = CreateAttackSkill(skillNumber, attackDamage: 120, name: name); - Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(skill), Is.False); } /// @@ -44,7 +44,7 @@ public void SiegeRoleSkillIsNotLearned() { var stun = CreateAttackSkill(67, attackDamage: 0, skillType: SkillType.AreaSkillAutomaticHits, name: "Stun"); - Assert.That(BotProgression.IsBotLootableSkill(stun), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(stun), Is.False); } /// @@ -59,7 +59,7 @@ public void MultiHitSkillWithoutFlatDamageIsLearned() { var chainDrive = CreateAttackSkill(262, attackDamage: 0, hits: 4, name: "Chain Drive"); - Assert.That(BotProgression.IsBotLootableSkill(chainDrive), Is.True); + Assert.That(BotProgression.MayBotOwnSkill(chainDrive), Is.True); } /// @@ -71,7 +71,7 @@ public void AreaSkillWithoutFlatDamageIsLearned() { var tripleShot = CreateAttackSkill(24, attackDamage: 0, skillType: SkillType.AreaSkillAutomaticHits, name: "Triple Shot"); - Assert.That(BotProgression.IsBotLootableSkill(tripleShot), Is.True); + Assert.That(BotProgression.MayBotOwnSkill(tripleShot), Is.True); } /// @@ -83,7 +83,7 @@ public void PlainSingleHitSkillWithoutDamageIsNotLearned() { var lunge = CreateAttackSkill(20, attackDamage: 0, name: "Lunge"); - Assert.That(BotProgression.IsBotLootableSkill(lunge), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(lunge), Is.False); } /// @@ -94,7 +94,7 @@ public void OrdinaryAttackSkillIsLearned() { var evilSpirit = CreateAttackSkill(9, attackDamage: 45, name: "Evil Spirit"); - Assert.That(BotProgression.IsBotLootableSkill(evilSpirit), Is.True); + Assert.That(BotProgression.MayBotOwnSkill(evilSpirit), Is.True); } /// diff --git a/tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs b/tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs index 9159ccd074..c1ea232713 100644 --- a/tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs +++ b/tests/MUnique.OpenMU.Tests/BotStarterGearEquipperTest.cs @@ -112,7 +112,13 @@ private static (BotStarterGearEquipper Equipper, ItemStorage Inventory, TestGear var sword = new TestGearItemDefinition { Group = 0, Number = 0, DropLevel = 5, Durability = 10 }; sword.QualifiedCharacters.Add(characterClass); - definitions = new List { sword }; + + // A staff the class could wield but its build rejects, at a lower drop level than the sword: + // preference must win over availability. + var staff = new TestGearItemDefinition { Group = 5, Number = 0, DropLevel = 1, Durability = 10 }; + staff.QualifiedCharacters.Add(characterClass); + + definitions = new List { sword, staff }; foreach (var (group, number) in new[] { (7, 5), (8, 5), (9, 5), (10, 5), (11, 5) }) { var piece = new TestGearItemDefinition { Group = (byte)group, Number = (byte)number, DropLevel = 5, Durability = 10 }; diff --git a/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs b/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs index 813a761388..b2017a9a9d 100644 --- a/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs +++ b/tests/MUnique.OpenMU.Tests/Offline/BotProgressionTests.cs @@ -123,7 +123,7 @@ public void MayBackfillSkill_ConsumableGrantWithoutSkillRequirements_ReturnsFals /// missing item would otherwise give it away for free. /// [Test] - public void IsBotLootableSkill_ExplicitlyExcludedWithoutGrantingItem_ReturnsFalse() + public void MayBotOwnSkill_ExplicitlyExcludedWithoutGrantingItem_ReturnsFalse() { var skill = new Skill { @@ -134,7 +134,7 @@ public void IsBotLootableSkill_ExplicitlyExcludedWithoutGrantingItem_ReturnsFals NumberOfHitsPerAttack = 4, }; - Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(skill), Is.False); } /// @@ -182,7 +182,7 @@ public void MayBackfillSkill_ObtainableOrbWithSkillRequirements_ReturnsTrue() [TestCase((short)73, "Mana Rays")] [TestCase((short)74, "Fire Blast")] [TestCase((short)269, "Charge")] - public void IsBotLootableSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, string name) + public void MayBotOwnSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, string name) { var skill = new Skill { @@ -193,7 +193,7 @@ public void IsBotLootableSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, NumberOfHitsPerAttack = 4, }; - Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(skill), Is.False); } /// @@ -207,7 +207,7 @@ public void IsBotLootableSkill_SiegeMarkedSkill_ReturnsFalse(short skillNumber, [TestCase((short)70, "Invisibility")] [TestCase((short)71, "Cancel Invisibility")] [TestCase((short)72, "Abolish Magic")] - public void IsBotLootableSkill_CastleSiegeRoleSkill_ReturnsFalse(short skillNumber, string name) + public void MayBotOwnSkill_CastleSiegeRoleSkill_ReturnsFalse(short skillNumber, string name) { var skill = new Skill { @@ -218,7 +218,7 @@ public void IsBotLootableSkill_CastleSiegeRoleSkill_ReturnsFalse(short skillNumb NumberOfHitsPerAttack = 1, }; - Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(skill), Is.False); } /// @@ -313,17 +313,17 @@ public void MayBackfillSkill_EquipmentGrantedOnly_ReturnsFalse() var characterClass = new CharacterClass { Number = 4 }; var skill = new SkillWithRequirements(new AttributeRequirement { Attribute = Stats.Level, MinimumValue = 110 }) { - Number = 49, - Name = "Fire Breath", - SkillType = SkillType.DirectHit, - AttackDamage = 30, + Number = 62, + Name = "Earthshake", + SkillType = SkillType.AreaSkillAutomaticHits, + AttackDamage = 150, NumberOfHitsPerAttack = 1, }; var pet = new TestItemDefinition { Group = 13, - Number = 3, - Name = "Horn of Dinorant", + Number = 4, + Name = "Dark Horse", DropLevel = 110, Skill = skill, }; @@ -395,11 +395,11 @@ public void TotalToBaseStat_RequirementValues_MapToBaseStats() [TestCase((short)47, "Impale")] [TestCase((short)49, "Fire Breath")] [TestCase((short)76, "Plasma Storm")] - public void IsBotLootableSkill_MountRequiredSkill_ReturnsFalse(short skillNumber, string name) + public void MayBotOwnSkill_MountRequiredSkill_ReturnsFalse(short skillNumber, string name) { var skill = new Skill { Number = skillNumber, Name = name, SkillType = SkillType.DirectHit, AttackDamage = 15, NumberOfHitsPerAttack = 1 }; - Assert.That(BotProgression.IsBotLootableSkill(skill), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(skill), Is.False); Assert.That(BotProgression.RequiresMount(skill), Is.True); } @@ -410,11 +410,11 @@ public void IsBotLootableSkill_MountRequiredSkill_ReturnsFalse(short skillNumber /// [TestCase((short)9, "Evil Spirit", SkillType.AreaSkillAutomaticHits, 45)] [TestCase((short)41, "Twisting Slash", SkillType.AreaSkillAutomaticHits, 0)] - public void IsBotLootableSkill_OrbGatedAttackSkill_ReturnsTrue(short skillNumber, string name, SkillType skillType, int attackDamage) + public void MayBotOwnSkill_OrbGatedAttackSkill_ReturnsTrue(short skillNumber, string name, SkillType skillType, int attackDamage) { var skill = new Skill { Number = skillNumber, Name = name, SkillType = skillType, AttackDamage = attackDamage, NumberOfHitsPerAttack = 1 }; - Assert.That(BotProgression.IsBotLootableSkill(skill), Is.True); + Assert.That(BotProgression.MayBotOwnSkill(skill), Is.True); } /// @@ -422,26 +422,26 @@ public void IsBotLootableSkill_OrbGatedAttackSkill_ReturnsTrue(short skillNumber /// siege-only attacks alike. /// [Test] - public void IsBotLootableSkill_NonCombatSkill_ReturnsFalse() + public void MayBotOwnSkill_NonCombatSkill_ReturnsFalse() { var summonGoblin = new Skill { Number = 30, Name = "Summon Goblin", SkillType = SkillType.SummonMonster, AttackDamage = 0 }; var defense = new Skill { Number = 18, Name = "Defense", SkillType = SkillType.Buff, AttackDamage = 0, MagicEffectDef = new MagicEffectDefinition() }; var crescentMoon = new Skill { Number = 44, Name = "Crescent Moon Slash", SkillType = SkillType.DirectHit, AttackDamage = 90 }; - Assert.That(BotProgression.IsBotLootableSkill(summonGoblin), Is.False); - Assert.That(BotProgression.IsBotLootableSkill(defense), Is.False); - Assert.That(BotProgression.IsBotLootableSkill(crescentMoon), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(summonGoblin), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(defense), Is.False); + Assert.That(BotProgression.MayBotOwnSkill(crescentMoon), Is.False); } /// /// Tests that a castable class buff with a magic effect is lootable from its orb. /// [Test] - public void IsBotLootableSkill_CastableBuff_ReturnsTrue() + public void MayBotOwnSkill_CastableBuff_ReturnsTrue() { var greaterDefense = new Skill { Number = 27, Name = "Greater Defense", SkillType = SkillType.Buff, AttackDamage = 0, MagicEffectDef = new MagicEffectDefinition() }; - Assert.That(BotProgression.IsBotLootableSkill(greaterDefense), Is.True); + Assert.That(BotProgression.MayBotOwnSkill(greaterDefense), Is.True); } private static (Dictionary> Grants, CharacterClass CharacterClass, Skill Skill) CreateScrollGrant(byte dropLevel)