feat: add 3.17 skill tree (with support for new skillsPerOrbit values) #3972
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
dbjorge
commented
Jan 28, 2022
| @@ -381,14 +361,6 @@ local PassiveTreeClass = newClass("PassiveTree", function(self, treeVersion) | |||
| if node.isAscendancyStart then | |||
| group.isAscendancyStart = true | |||
| end | |||
| if node.o ~= 4 then | |||
There was a problem hiding this comment.
I think this is safe to remove since self:ProcessNode gets called a few lines down anyway and will do the same calculation
LocalIdentity
approved these changes
Jan 28, 2022
|
Seems to ignore the radius changes on large cluster jewels so do we need a different calculation on these? |
2 tasks
|
@Guitaraholic see #3982 |
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.
Builds on @ppoelzl 's #3971 with an update to support the new
skillsPerOrbitvalue that was causing visual issues. Submitting as a combined PR instead of separating the orbit angle updates into a standalone PR so they can be tested against the new tree more easily, per @Wires77 requestUses tree data to source info about orbit angles/radii rather than hard-coding them (but includes a fallback to account for the 2_6 tree data not including the necessary constants). Thanks @Regisle for pointing out that the tree data included this!
I implemented a version of this that calculated the special cases at 16 and 40 by assuming cases >= 8 would want the 45 degree marks, but it ended up feeling significantly less readable than just hardcoding those cases. Our best "spec" for the behavior from GGG is a hardcoded table rather than a formula, anyway.
Steps taken to verify a working solution:
Link to a build that showcases this PR:
Convert any of the in-tree test builds
Before screenshot:
n/a
After screenshot:
3.16 tree still rendering successfully:
Converted to 3.17 tree: