From 09a8c1b06381752a585f7923893526f0f8f6fda7 Mon Sep 17 00:00:00 2001 From: Diego Amatte Date: Sat, 17 Oct 2026 23:06:16 +0000 Subject: [PATCH] fix(items): require the skill requirements to learn a skill with an item A player could learn a skill with a scroll, orb or crystal without complying with the requirements of the skill itself, and then couldn't use it. E.g. the Orb of Summoning has no energy requirement, but Summon Goblin needs 90 energy and Summon Soldier (orb +6) 280. Learning a skill with an item now also requires the requirements of the skill. The requirement check of Player.TryConsumeForSkillAsync is extracted to Player.CompliesRequirements(Skill) and reused for it. Split out of #995, without the client data packets. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Uz47mrsSwm3hxZc7gCMxd9 --- src/GameLogic/Player.cs | 12 +- .../LearnablesConsumeHandlerPlugIn.cs | 8 +- .../LearnablesConsumeHandlerPlugInTest.cs | 121 ++++++++++++++++++ 3 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 tests/MUnique.OpenMU.Tests/LearnablesConsumeHandlerPlugInTest.cs diff --git a/src/GameLogic/Player.cs b/src/GameLogic/Player.cs index a5372cea80..f29898ed76 100644 --- a/src/GameLogic/Player.cs +++ b/src/GameLogic/Player.cs @@ -1179,6 +1179,16 @@ public ValueTask NewLocateablesInScopeAsync(IEnumerable newObjects) return this._observerToWorldViewAdapter.NewLocateablesInScopeAsync(newObjects); } + /// + /// Determines whether the player complies with the of a skill, so that it can use it. + /// + /// The skill. + /// true, if the player complies with the requirements; Otherwise, false. + public bool CompliesRequirements(Skill skill) + { + return !skill.Requirements.Any(r => r.MinimumValue > this.Attributes![r.Attribute]); + } + /// /// Tries to consume the of a skill. /// @@ -1194,7 +1204,7 @@ public async ValueTask TryConsumeForSkillAsync(SkillEntry skillEntry) return false; } - if (skill.Requirements.Any(r => r.MinimumValue > this.Attributes![r.Attribute])) + if (!this.CompliesRequirements(skill)) { return false; } diff --git a/src/GameLogic/PlayerActions/ItemConsumeActions/LearnablesConsumeHandlerPlugIn.cs b/src/GameLogic/PlayerActions/ItemConsumeActions/LearnablesConsumeHandlerPlugIn.cs index a5592bc3df..40d38fd945 100644 --- a/src/GameLogic/PlayerActions/ItemConsumeActions/LearnablesConsumeHandlerPlugIn.cs +++ b/src/GameLogic/PlayerActions/ItemConsumeActions/LearnablesConsumeHandlerPlugIn.cs @@ -38,10 +38,16 @@ public override async ValueTask ConsumeItemAsync(Player player, Item item, } /// + /// + /// Besides the requirements of the item, the player has to comply with the requirements of the skill. + /// Otherwise, the player could learn a skill which it can't use, and the item would be lost. + /// E.g. the Orb of Summoning has no energy requirement itself, but the skills of its levels have. + /// protected override bool CheckPreconditions(Player player, Item item) { return base.CheckPreconditions(player, item) - && player.CompliesRequirements(item); + && player.CompliesRequirements(item) + && (this.GetLearnableSkill(item, player.GameContext.Configuration) is not { } skill || player.CompliesRequirements(skill)); } /// diff --git a/tests/MUnique.OpenMU.Tests/LearnablesConsumeHandlerPlugInTest.cs b/tests/MUnique.OpenMU.Tests/LearnablesConsumeHandlerPlugInTest.cs new file mode 100644 index 0000000000..f22d60eaa9 --- /dev/null +++ b/tests/MUnique.OpenMU.Tests/LearnablesConsumeHandlerPlugInTest.cs @@ -0,0 +1,121 @@ +// +// 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; +using MUnique.OpenMU.GameLogic.Attributes; +using MUnique.OpenMU.GameLogic.PlayerActions.ItemConsumeActions; + +/// +/// Tests that a skill can only be learned with an item when the player could also use it. +/// +[TestFixture] +public class LearnablesConsumeHandlerPlugInTest +{ + private const byte ItemSlot = 12; + + /// + /// Tests that a skill is only learned with an item if the player complies with the requirements of the skill, + /// even when the item itself has no requirements. + /// + /// The energy requirement of the skill. The test player has 10 energy. + /// If set to true, the skill is expected to be learned. + [TestCase(10, true)] + [TestCase(20, false)] + public async ValueTask SkillIsOnlyLearnedWhenItCanBeUsedAsync(int skillEnergyRequirement, bool expectedLearned) + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + var skill = CreateSkill(1, skillEnergyRequirement); + var scroll = await AddItemAsync(player, CreateItemDefinition(15, 0, skill, player), 0).ConfigureAwait(false); + + var consumed = await new LearnablesConsumeHandlerPlugIn().ConsumeItemAsync(player, scroll, null, FruitUsage.Undefined).ConfigureAwait(false); + + Assert.Multiple(() => + { + Assert.That(consumed, Is.EqualTo(expectedLearned)); + Assert.That(player.SkillList!.ContainsSkill((ushort)skill.Number), Is.EqualTo(expectedLearned)); + Assert.That(scroll.Durability, Is.EqualTo(expectedLearned ? 0 : 1), "the item is only consumed when the skill is learned"); + }); + } + + /// + /// Tests that the Orb of Summoning requires the requirements of the skill of its item level. + /// + /// The level of the orb. + /// If set to true, the skill is expected to be learned. + [TestCase(0, true)] + [TestCase(1, false)] + public async ValueTask SummoningOrbRequiresTheRequirementsOfTheSkillOfItsLevelAsync(byte itemLevel, bool expectedLearned) + { + var player = await PlayerTestHelper.CreatePlayerAsync().ConfigureAwait(false); + var skills = new[] { CreateSkill(30, 10), CreateSkill(31, 20) }; + foreach (var skill in skills) + { + player.GameContext.Configuration.Skills.Add(skill); + } + + var definition = CreateItemDefinition(ItemConstants.SummonOrb.Group, ItemConstants.SummonOrb.Number!.Value, skills[0], player); + var orb = await AddItemAsync(player, definition, itemLevel).ConfigureAwait(false); + + var consumed = await new SummoningOrbConsumeHandlerPlugIn().ConsumeItemAsync(player, orb, null, FruitUsage.Undefined).ConfigureAwait(false); + + Assert.Multiple(() => + { + Assert.That(consumed, Is.EqualTo(expectedLearned)); + Assert.That(player.SkillList!.ContainsSkill((ushort)skills[itemLevel].Number), Is.EqualTo(expectedLearned)); + }); + } + + private static Skill CreateSkill(short number, int energyRequirement) + { + var skill = new TestSkill(number); + skill.Requirements.Add(new AttributeRequirement { Attribute = Stats.TotalEnergy, MinimumValue = energyRequirement }); + return skill; + } + + private static ItemDefinition CreateItemDefinition(byte group, short number, Skill skill, Player player) + { + var definition = new TestItemDefinition(group, number, skill); + definition.QualifiedCharacters.Add(player.SelectedCharacter!.CharacterClass!); + return definition; + } + + private static async ValueTask AddItemAsync(Player player, ItemDefinition definition, byte level) + { + var item = new Item { Definition = definition, Durability = 1, Level = level }; + await player.Inventory!.AddItemAsync(ItemSlot, item).ConfigureAwait(false); + return item; + } + + private sealed class TestSkill : Skill + { + public TestSkill(short number) + { + this.Number = number; + this.Name = $"Skill {number}"; + this.Requirements = new List(); + this.ConsumeRequirements = new List(); + this.QualifiedCharacters = new List(); + } + } + + private sealed class TestItemDefinition : ItemDefinition + { + public TestItemDefinition(byte group, short number, Skill skill) + { + this.Group = group; + this.Number = number; + this.Skill = skill; + this.Width = 1; + this.Height = 1; + this.MaximumItemLevel = 6; + this.Requirements = new List(); + this.QualifiedCharacters = new List(); + } + } +}