Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly generate dependencies for nodes affected by intuitive leap-like jewels #7719

Merged
merged 2 commits into from
Jul 20, 2024

Conversation

trimbe
Copy link
Contributor

@trimbe trimbe commented Jul 19, 2024

Fixes #151, fixes #7642
Supersedes #7654

Description of the problem being solved:

Dependencies for intuitive leap-like nodes were handled by orphan pruning, and the nodes never had their dependencies properly resolved. Adds new logic to properly resolve their dependencies and thus adds support for features such as hover dependency highlighting and deallocated tooltip stat summaries. Also, corrects pathing behavior for nodes going through allocated nodes that are also affected by intuitive leap-like jewels (#7642).

Steps taken to verify a working solution:

  • Created a build with a variety of intuitive leap-like jewels present, some overlapping
  • Verified that dependencies were generated correctly in all situations (that I could think of)
  • Verified that deallocated node sums and tooltip stat summaries correctly reflected dependencies
  • Verified that pathing through nodes affected by intuitive leap-like jewels was possible and that the path lengths were correct

Link to a build that showcases this PR:

https://pobb.in/42VzhUbnYZFc

Before screenshot:

image

After screenshot:

image

@Wires77
Copy link
Member

Wires77 commented Jul 20, 2024

One deallocated node this doesn't highlight on hover (Vampirism):
image.

Also these nodes get unallocated when unallocating Impossible Escape even though they're in range of Intuitive Leap:
image

This looks good on first glance though, plus kudos for tackling one of the older bugs I've seen fixed by a PR in a while!

@trimbe
Copy link
Contributor Author

trimbe commented Jul 20, 2024

Thanks for taking a look (and finding issues)! I've fixed the socket dependency issue, but I'm unable to reproduce the Vampirism scenario. Is it possible you weren't on my branch for that one? I notice the socket still has the uncolored base art in your screenshot, and the behavior is identical to the release version.

@Wires77
Copy link
Member

Wires77 commented Jul 20, 2024

It is possible, thanks for fixing that. At some point I'd love to see some tests around this behaviour, since different jewels can be easily missed. The tests aren't well set up for tree tests, however, so I'm not going to let that hold up this PR

@Wires77 Wires77 merged commit d8dd69e into PathOfBuildingCommunity:dev Jul 20, 2024
2 checks passed
@trimbe trimbe deleted the intuitiveleap-dependency branch July 22, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants