Merge pull request #7 from PoESkillTree/secondary-skill-fix

Do additional skill stat parsing in gem parsing instead of skill parsing
PoESkillTree · Mar 10, 2020 · e6c2f10 · e6c2f10
2 parents 9c35ce4 + 41a3118
commit e6c2f10
Show file tree

Hide file tree

Showing 37 changed files with 657 additions and 381 deletions.
diff --git a/PoESkillTree.Engine.Computation.Builders/Stats/GemStatBuilders.cs b/PoESkillTree.Engine.Computation.Builders/Stats/GemStatBuilders.cs
@@ -43,8 +43,7 @@ private IStatBuilder AdditionalLevels(string identityInfix, IGemTagBuilder gemTa
             Additional("Levels" + identityInfix, buildIdentitySuffix);
 
         public IStatBuilder AdditionalLevels(Skill skill) =>
-            FromIdentity($"Skill.AdditionalLevels.{skill.ItemSlot}.{skill.SocketIndex}.{skill.SkillIndex}", typeof(int),
-                ExplicitRegistrationTypes.ChangeInvalidatesSkillParse(skill));
+            FromIdentity($"Skill.AdditionalLevels.{skill.ItemSlot}.{skill.SocketIndex}.{skill.SkillIndex}", typeof(int));
 
 
         public IStatBuilder AdditionalQualityForModifierSourceItemSlot =>
@@ -54,8 +53,7 @@ private IStatBuilder AdditionalLevels(string identityInfix, IGemTagBuilder gemTa
             Additional("Quality.SupportSkill", GetItemSlot);
 
         public IStatBuilder AdditionalQuality(Skill skill) =>
-            FromIdentity($"Skill.AdditionalQuality.{skill.ItemSlot}.{skill.SocketIndex}.{skill.SkillIndex}", typeof(int),
-                ExplicitRegistrationTypes.ChangeInvalidatesSkillParse(skill));
+            FromIdentity($"Skill.AdditionalQuality.{skill.ItemSlot}.{skill.SocketIndex}.{skill.SkillIndex}", typeof(int));
 
 
         public IStatBuilder IncreasedReservationForModifierSourceItemSlot =>

diff --git a/PoESkillTree.Engine.Computation.Builders/Stats/MetaStatBuilders.cs b/PoESkillTree.Engine.Computation.Builders/Stats/MetaStatBuilders.cs
@@ -9,7 +9,6 @@
 using PoESkillTree.Engine.Computation.Common.Builders.Stats;
 using PoESkillTree.Engine.Computation.Common.Builders.Values;
 using PoESkillTree.Engine.GameModel;
-using PoESkillTree.Engine.GameModel.Items;
 using PoESkillTree.Engine.GameModel.Skills;
 using PoESkillTree.Engine.Utils.Extensions;
 
@@ -200,11 +199,14 @@ public IStatBuilder ActiveSkillSocketIndex(string skillId)
         public IStatBuilder MainSkillSocketIndex => FromIdentity(typeof(int));
         public IStatBuilder MainSkillSkillIndex => FromIdentity(typeof(uint));
 
-        public IStatBuilder SkillBaseCost(ItemSlot itemSlot, int socketIndex, int skillIndex)
-            => FromIdentity($"{itemSlot.GetName()}.{socketIndex}.{skillIndex}.Cost", typeof(uint));
+        public IStatBuilder SkillBaseCost(Skill skill)
+            => FromIdentity($"{skill.ItemSlot.GetName()}.{skill.SocketIndex}.{skill.SkillIndex}.Cost", typeof(uint));
 
-        public IStatBuilder SkillHasType(ItemSlot itemSlot, int socketIndex, int skillIndex, string activeSkillType)
-            => FromIdentity($"{itemSlot.GetName()}.{socketIndex}.{skillIndex}.Type.{activeSkillType}", typeof(bool));
+        public IStatBuilder SkillHasType(Skill skill, string activeSkillType)
+            => FromIdentity($"{skill.ItemSlot.GetName()}.{skill.SocketIndex}.{skill.SkillIndex}.Type.{activeSkillType}", typeof(bool));
+
+        public IStatBuilder SkillIsEnabled(Skill skill)
+            => FromIdentity($"{skill.ItemSlot.GetName()}.{skill.SocketIndex}.{skill.SkillIndex}.IsEnabled", typeof(bool));
 
         public IStatBuilder ActiveCurses => FromIdentity(typeof(int));
 

diff --git a/PoESkillTree.Engine.Computation.Common/Builders/Stats/IMetaStatBuilders.cs b/PoESkillTree.Engine.Computation.Common/Builders/Stats/IMetaStatBuilders.cs
@@ -104,8 +104,9 @@ public interface IMetaStatBuilders
         IStatBuilder MainSkillSocketIndex { get; }
         IStatBuilder MainSkillSkillIndex { get; }
 
-        IStatBuilder SkillBaseCost(ItemSlot itemSlot, int socketIndex, int skillIndex);
-        IStatBuilder SkillHasType(ItemSlot itemSlot, int socketIndex, int skillIndex, string activeSkillType);
+        IStatBuilder SkillBaseCost(Skill skill);
+        IStatBuilder SkillHasType(Skill skill, string activeSkillType);
+        IStatBuilder SkillIsEnabled(Skill skill);
 
         IStatBuilder ActiveCurses { get; }
         ValueBuilder ActiveCurseIndex(int numericSkillId);

diff --git a/PoESkillTree.Engine.Computation.Common/ExplicitRegistrationType.cs b/PoESkillTree.Engine.Computation.Common/ExplicitRegistrationType.cs
@@ -1,5 +1,4 @@
 using PoESkillTree.Engine.GameModel;
-using PoESkillTree.Engine.GameModel.Skills;
 using PoESkillTree.Engine.Utils;
 
 namespace PoESkillTree.Engine.Computation.Common
@@ -78,15 +77,6 @@ public PassiveTreeConnection(ushort sourceNode, ushort targetNode)
 
             protected override object ToTuple() => (GetType().Name, SourceNode, TargetNode);
         }
-
-        public sealed class ChangeInvalidatesSkillParse : ExplicitRegistrationType
-        {
-            public ChangeInvalidatesSkillParse(Skill skill) => Skill = skill;
-
-            public Skill Skill { get; }
-
-            protected override object ToTuple() => (GetType().Name, Skill);
-        }
     }
 
     public static class ExplicitRegistrationTypes
@@ -110,8 +100,5 @@ public static ExplicitRegistrationType.IndependentResult IndependentResult(NodeT
         public static ExplicitRegistrationType.PassiveTreeConnection PassiveTreeConnection(
             ushort sourceNode, ushort targetNode)
             => new ExplicitRegistrationType.PassiveTreeConnection(sourceNode, targetNode);
-
-        public static ExplicitRegistrationType.ChangeInvalidatesSkillParse ChangeInvalidatesSkillParse(Skill skill) =>
-            new ExplicitRegistrationType.ChangeInvalidatesSkillParse(skill);
     }
 }
diff --git a/PoESkillTree.Engine.Computation.Console/CompositionRoot.cs b/PoESkillTree.Engine.Computation.Console/CompositionRoot.cs
@@ -2,7 +2,6 @@
 using System.Threading.Tasks;
 using PoESkillTree.Engine.Computation.Common.Builders;
 using PoESkillTree.Engine.Computation.Core;
-using PoESkillTree.Engine.Computation.Core.Nodes;
 using PoESkillTree.Engine.Computation.Data;
 using PoESkillTree.Engine.Computation.Data.Steps;
 using PoESkillTree.Engine.Computation.Parsing;
@@ -27,8 +26,7 @@ public CompositionRoot()
                 () => Builders.BuilderFactories.CreateAsync(_gameData.Value));
             _parser = new Lazy<Task<IParser>>(
                 async () => await Parser<ParsingStep>.CreateAsync(_gameData.Value, _builderFactories.Value,
-                    ParsingData.CreateAsync(_gameData.Value, _builderFactories.Value),
-                    new SimpleValueCalculationContext(_calculator.Value.NodeRepository)).ConfigureAwait(false));
+                    ParsingData.CreateAsync(_gameData.Value, _builderFactories.Value)).ConfigureAwait(false));
         }
 
         public GameData GameData => _gameData.Value;

diff --git a/PoESkillTree.Engine.Computation.IntegrationTests/CompositionRootTestBase.cs b/PoESkillTree.Engine.Computation.IntegrationTests/CompositionRootTestBase.cs
@@ -2,8 +2,6 @@
 using System.Threading.Tasks;
 using PoESkillTree.Engine.Computation.Common.Builders;
 using PoESkillTree.Engine.Computation.Common.Data;
-using PoESkillTree.Engine.Computation.Core;
-using PoESkillTree.Engine.Computation.Core.Nodes;
 using PoESkillTree.Engine.Computation.Data.Steps;
 using PoESkillTree.Engine.Computation.Parsing;
 using PoESkillTree.Engine.GameModel;
@@ -24,8 +22,7 @@ public abstract class CompositionRootTestBase
                 () => Data.ParsingData.CreateAsync(LazyGameData.Value, LazyBuilderFactories.Value));
 
         private static readonly Lazy<Task<Parser<ParsingStep>>> LazyParser = new Lazy<Task<Parser<ParsingStep>>>(
-            () => Parser<ParsingStep>.CreateAsync(LazyGameData.Value, LazyBuilderFactories.Value,
-                LazyParsingData.Value, new SimpleValueCalculationContext(Calculator.Create().NodeRepository)));
+            () => Parser<ParsingStep>.CreateAsync(LazyGameData.Value, LazyBuilderFactories.Value, LazyParsingData.Value));
 
         protected static GameData GameData => LazyGameData.Value;
         protected static Task<IBuilderFactories> BuilderFactoriesTask => LazyBuilderFactories.Value;

diff --git a/PoESkillTree.Engine.Computation.IntegrationTests/SkillParserTest.cs b/PoESkillTree.Engine.Computation.IntegrationTests/SkillParserTest.cs
@@ -72,6 +72,7 @@ public void ParseFrenzyReturnsCorrectResult()
                         true),
                     ("Frenzy.ActiveSkillItemSlot", Form.BaseSet, (double) frenzy.ItemSlot, global, false),
                     ("Frenzy.ActiveSkillSocketIndex", Form.BaseSet, frenzy.SocketIndex, global, false),
+                    ("Boots.0.0.IsEnabled", Form.TotalOverride, 1, global, false),
                     ("Frenzy.Reservation", Form.Increase, null, global, false),
                     ("Frenzy.Instances", Form.BaseAdd, 1, global, false),
                     ("Skills[].Instances", Form.BaseAdd, 1, global, false),
@@ -140,7 +141,7 @@ public void ParseFrenzyReturnsCorrectResult()
                     ("Range.Attack.OffHand.Skill", Form.BaseAdd, null, global, true),
                 }.Select(t => (t.stat, t.form, (NodeValue?) t.value, t.source, t.mainSkillOnly)).ToArray();
 
-            var actual = _parser.ParseActiveSkill(new ActiveSkillParserParameter(frenzy, Entity.Character, SkillModification));
+            var actual = _parser.ParseActiveSkill(new ActiveSkillParserParameter(frenzy, Entity.Character));
 
             AssertCorrectModifiers(valueCalculationContextMock, isMainSkillStat, expectedModifiers, actual);
         }
@@ -166,6 +167,7 @@ public void ParseAddedColdDamageSupportReturnsCorrectResult()
                         Form.BaseSet, (double) support.ItemSlot, global, false),
                     ("SupportAddedColdDamage.ActiveSkillSocketIndex",
                         Form.BaseSet, support.SocketIndex, global, false),
+                    ("Boots.1.0.IsEnabled", Form.TotalOverride, 1, global, false),
                     ("Frenzy.Cost", Form.More, levelDefinition.ManaMultiplier * 100 - 100, global, false),
                     ("Cold.Damage.Attack.MainHand.Skill", Form.Increase,
                         levelDefinition.QualityStats[0].Value * 20 / 1000, global, true),
@@ -209,7 +211,7 @@ public void ParseAddedColdDamageSupportReturnsCorrectResult()
                     ("Cold.Damage.Secondary.Skill", Form.BaseAdd, addedDamageValue, global, true))
                 .ToArray();
 
-            var actual = _parser.ParseSupportSkill(new SupportSkillParserParameter(frenzy, support, Entity.Character, SkillModification));
+            var actual = _parser.ParseSupportSkill(new SupportSkillParserParameter(frenzy, support, Entity.Character));
 
             AssertCorrectModifiers(valueCalculationContextMock, isMainSkillStat, expectedModifiers, actual);
         }
@@ -307,13 +309,13 @@ private ParseResult Parse(string skillId)
                 var activeSkill = Skill.FromGem(activeGem, true);
                 var supportGem = new Gem(skillId, level, 20, default, 1, 0, true);
                 var supportSkill = Skill.FromGem(supportGem, true);
-                return _parser.ParseSupportSkill(new SupportSkillParserParameter(activeSkill, supportSkill, Entity.Character, SkillModification));
+                return _parser.ParseSupportSkill(new SupportSkillParserParameter(activeSkill, supportSkill, Entity.Character));
             }
             else
             {
                 var gem = new Gem(skillId, level, 20, default, 0, 0, true);
                 var skill = Skill.FromGem(gem, true);
-                return _parser.ParseActiveSkill(new ActiveSkillParserParameter(skill, Entity.Character, SkillModification));
+                return _parser.ParseActiveSkill(new ActiveSkillParserParameter(skill, Entity.Character));
             }
         }
 
@@ -322,7 +324,5 @@ private static IEnumerable<string> ReadParseableSkills()
 
         private static IEnumerable<string> ReadNotParseableSkills()
             => ReadDataLines("NotParseableSkills");
-
-        private static readonly SkillModification SkillModification = new SkillModification(0, 0);
     }
 }
diff --git a/PoESkillTree.Engine.Computation.Parsing.Tests/SkillParsers/ActiveSkillParserTest.cs b/PoESkillTree.Engine.Computation.Parsing.Tests/SkillParsers/ActiveSkillParserTest.cs
@@ -88,24 +88,6 @@ public void FrenzySetsManaCost()
             Assert.AreEqual(new NodeValue(20), actual);
         }
 
-        [TestCase(0, ExpectedResult = 10)]
-        [TestCase(2, ExpectedResult = 20)]
-        [TestCase(3, ExpectedResult = 40)]
-        [TestCase(4, ExpectedResult = 40)]
-        public int? FrenzyIsParsedUsingItsActualSkillLevel(int additionalLevels)
-        {
-            var (definition, skill) = CreateFrenzyDefinition(true,
-                (1, 10), (4, 40), (2, 20));
-            var valueCalculationContext = MockValueCalculationContextForMainSkill(skill);
-            var sut = CreateSut(definition);
-
-            var result = sut.Parse(skill, Entity.Character, new SkillModification(additionalLevels, 0));
-
-            var actual = GetValueForIdentity(result.Modifiers, "Belt.0.0.Cost")
-                .Calculate(valueCalculationContext);
-            return (int?) actual.SingleOrNull();
-        }
-
         [TestCase(true)]
         [TestCase(false)]
         public void FrenzyAddsToSkillInstances(bool isActiveSkill)
@@ -129,28 +111,55 @@ public void FrenzyAddsToSkillInstances(bool isActiveSkill)
         }
 
         [Test]
-        public void ParseReturnsEmptyResultForDisabledSkill()
+        public void ParseReturnsEmptyResultForDisabledGem()
         {
-            var (definition, skill) = CreateFrenzyDefinition(false);
+            var (definition, skill) = CreateFrenzyDefinition(gemIsEnabled: false);
             var sut = CreateSut(definition);
 
             var result = Parse(sut, skill);
 
             Assert.IsEmpty(result.Modifiers);
         }
 
-        private static (SkillDefinition, Skill) CreateFrenzyDefinition(bool isEnabled = true) =>
-            CreateFrenzyDefinition(isEnabled, (1, 10));
+        [Test]
+        public void ParseReturnsOnlyIsEnabledStatForDisabledSkill()
+        {
+            var (definition, skill) = CreateFrenzyDefinition(skillIsEnabled: false);
+            var sut = CreateSut(definition);
 
-        private static (SkillDefinition, Skill) CreateFrenzyDefinition(bool isEnabled, params (int level, int manaCost)[] levels)
+            var result = Parse(sut, skill);
+
+            Assert.IsEmpty(result.Modifiers);
+        }
+
+        [Test]
+        public void ParseReturnsIsEnabledStatForEnabledSkill()
+        {
+            var (definition, skill) = CreateFrenzyDefinition();
+            var sut = CreateSut(definition);
+            var context = MockValueCalculationContextForInactiveSkill(skill);
+
+            var (_, _, modifiers) = Parse(sut, skill);
+
+            Assert.IsTrue(AnyModifierHasIdentity(modifiers, "Belt.0.0.IsEnabled"));
+            Assert.AreEqual((NodeValue?) true, GetValueForIdentity(modifiers, "Belt.0.0.IsEnabled").Calculate(context));
+        }
+
+        private static (SkillDefinition, Skill) CreateFrenzyDefinition(bool gemIsEnabled = true, bool skillIsEnabled = true) =>
+            CreateFrenzyDefinition(gemIsEnabled, skillIsEnabled, (1, 10));
+
+        private static (SkillDefinition, Skill) CreateFrenzyDefinition(params (int level, int manaCost)[] levels) =>
+            CreateFrenzyDefinition(true, true, levels);
+
+        private static (SkillDefinition, Skill) CreateFrenzyDefinition(bool gemIsEnabled, bool skillIsEnabled, params (int level, int manaCost)[] levels)
         {
             var activeSkill = CreateActiveSkillDefinition("Frenzy", new[] { "attack" },
                 new[] { Keyword.Melee, Keyword.Projectile });
             var levelDefinitions = levels
                 .Select(t => (t.level, CreateLevelDefinition(manaCost: t.manaCost)))
                 .ToDictionary();
             return (CreateActive("Frenzy", activeSkill, levelDefinitions),
-                CreateSkillFromGem("Frenzy", 1, 0, isEnabled));
+                CreateSkillFromGem("Frenzy", 1, 0, gemIsEnabled, skillIsEnabled));
         }
 
         #endregion
@@ -249,9 +258,8 @@ public void FlameTotemSetsSpellDamage()
             Assert.IsFalse(AnyModifierHasIdentity(modifiers, "Fire.Damage.Spell.Ignite"));
         }
 
-        [TestCase(0)]
-        [TestCase(10)]
-        public void FlameTotemStatsAreParsedCorrectly(int additionalQuality)
+        [Test]
+        public void FlameTotemStatsAreParsedCorrectly()
         {
             var (definition, skill) = CreateFlameTotemDefinition();
             var source = new ModifierSource.Local.Skill("FlameTotem", "Flame Totem");
@@ -264,7 +272,7 @@ public void FlameTotemStatsAreParsedCorrectly(int additionalQuality)
             {
                 new UntranslatedStatParserParameter(source, new[]
                 {
-                    new UntranslatedStat("totem_life_+%", 10 + additionalQuality),
+                    new UntranslatedStat("totem_life_+%", 10),
                 }),
                 new UntranslatedStatParserParameter(source, new[]
                 {
@@ -277,7 +285,7 @@ public void FlameTotemStatsAreParsedCorrectly(int additionalQuality)
                 p.Parse(EmptyParserParameter(source)) == EmptyParseResult);
             var sut = CreateSut(definition, statParser);
 
-            var result = sut.Parse(skill, Entity.Character, new SkillModification(0, additionalQuality));
+            var result = sut.Parse(skill, Entity.Character);
 
             var modifiers = result.Modifiers;
             Assert.IsTrue(AnyModifierHasIdentity(modifiers, "s1"));
@@ -1499,8 +1507,8 @@ private static (SkillDefinition, Skill) CreateRighteousFireDefinition()
 
         #endregion
 
-        private static Skill CreateSkillFromGem(string skillId, int level, int quality = 0, bool isEnabled = true) =>
-            Skill.FromGem(new Gem(skillId, level, quality, ItemSlot.Belt, 0, 0, isEnabled), isEnabled);
+        private static Skill CreateSkillFromGem(string skillId, int level, int quality = 0, bool gemIsEnabled = true, bool skillIsEnabled = true) =>
+            Skill.FromGem(new Gem(skillId, level, quality, ItemSlot.Belt, 0, 0, gemIsEnabled), skillIsEnabled);
 
         private static Skill CreateSkillFromItem(string skillId, int level, int quality = 0) =>
             Skill.FromItem(skillId, level, quality, ItemSlot.Belt, 0, true);
@@ -1535,6 +1543,6 @@ private static ActiveSkillParser CreateSut(SkillDefinition skillDefinition)
         }
 
         private static ParseResult Parse(IParser<ActiveSkillParserParameter> sut, Skill activeSkill) =>
-            sut.Parse(activeSkill, Entity.Character, new SkillModification(0, 0));
+            sut.Parse(activeSkill, Entity.Character);
     }
 }