Calculation Rewriting Part 2.3: Skill and Item parsing #515
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace IParser<TResult> by IStringParser<TResult> and move these to subnamespace. The non-generic IParser interface will later be generalized to also support GivenStatsParser and parsers for items and skills, making this separation necessary. Plus this also allows grouping the now IStringParsers into a namespace.
Change previous IParser interface to ICoreParser. Add new IParser<TResult> interface, implemented by ICoreParser using CoreParserParameter. Usage of ICoreParser/CoreParser stayed the same, however the underlying interface can now be used for new parsers.
logging it and creating an appropriate ParseResult. Add tests for CoreParser.
Refactor ParseResult for easier and safer usage
- With(Keyword) is now in IDamageRelatedStatBuilder instead of IStatBuilder and is used directly everywhere it needs to be. - IConditionBuilders.With(Keyword) is no longer part specific, just MainSkillHasKeyword, and directly builds to a value condition. This the correct way everywhere it is still used because those mods refer to skills and are thus not affected by the skill part. - Add IConditionBuilders.WithPart(Keyword) for those case where skill part specific keywords are required but the stat is not damage related,
that deserializes gem json into SkillDefinitions (far from complete atm). Flesh out SkillDefinition a bit as required for the implemented deserialization parts. Implement selection of Keywords from active skill types and gem tags.
to support everything relevant for Frenzy
Add gems.json and gem_tooltips.json to GameModel.Tests/Data containing 20 gems for easily testing fields that can't be tested with Frenzy as the one example skill alone.
- Ignore skills with unreleased base items - Move ItemClass to GameModel project - Deserialize active_skill.weapon_restrictions - Deserialize per_level/static.crit_chance, .cooldown, .mana_multiplier and .mana_reservation_override - Deserialize support_gem - Set ActiveSkillDefinition.ProvidesBuff from active skill types
The integration test currently fails because there is no implementation. Add SkillJsonDeserializer.DeserializeAsync() without parameters that uses the embedded resource files.
Just casting from/to double does not work with flags above 2^52. Casting to int in between stops working even earlier. (other enums don't have this problem because they are ints, which can be cast from/to double without loss)
(most is hardcoded for frenzy) Add missing gem related stats (requirements, surfacing the keyword stats from IStatFactory as IStatBuilders, damage effectiveness)
- MainHand only skills - dual wield requiring skills - shield requiring skills - weapon restrictions
(Vaal) Dual Strike and (Vaal) Cyclone have base_skill_number_of_additional_hits. Double Strike, Cleave and Riposte have skill_double_hits_when_dual_wielding.
Requirements come from the gem, not the skill. Item-innate skills do not apply requirements. Change all other modifiers in ActiveSkillParser to be global. Skill local isn't required for any of them.
Add modifiers for all properties to BaseSet their real stat, even if the base item doesn't have that property. They might be added by property modifiers.
because they can be affected by local modifiers
(only 10 uniques for now)
(somehow thought it is a keystone like Iron Grip)
Modifiers from nodes have a condition so they are only enabled when set to skilled. This way keystones from items just work (no doubled modifiers) and the whole tree can be parsed and added to the calculator initially (which should be good for performance).
It matches against the keystone definitions from GameModel, no hardcoding required (except for now in GameModel because there is no real skill tree data yet)
- Remove IItemSlotBuilder(s) - Remove IMatchContext.First and .Last - Remove IResolvable as ancestor of ILeechStatBuilder
This was referenced Dec 26, 2018
- Initialize from CharacterGivenStats - Add 2 maximum passive points when Bandit.None - Add 1 to one of the stats in PassiveNodeParser (depending on PassiveNodeDefinition.IsAscendancyNode) - Add PassiveNodeDefinition.PassivePointsGranted to maximum in PassiveNodeParser
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part 3 of #501, this contains parser classes that turn skills and items (and passive nodes) into modifiers so they can be added to a calculator. These parser classes will be the API the program uses for parsing.
As usual, it turned out to be much more work than I expected, mainly because skill parsing consists of a lot more than just the natural language stat parsing I implemented previously and also required a bunch of extensions in the Builder project.
Parsing fully supports most skills. The main things not currently supported are minion skills, item-inherent skills and socketed gem modifiers (from items and supports like Empower). Other unsupported skills require less work and are already planned out, but I skipped implementing them for now.