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

Fix numerous fullDPS tooltip issues #8247

Merged

Conversation

Paliak
Copy link
Contributor

@Paliak Paliak commented Aug 24, 2024

Fixes #8245 #8163

Description of the problem being solved:

There exist multiple issues with fullDPS breakdowns not showing up or showing up when they shouldn't be. This pr attempts to address those issues.

Marking as draft and there are probably many more edge cases i have not yet considered. Especially around trader functionality.

Steps taken to verify a working solution:

@Paliak Paliak added the bug: accuracy Wording differences label Aug 24, 2024
@nofate121
Copy link
Contributor

Using the same PoB i used here to test things:
#8245 (comment)
https://pobb.in/8eINEmkZKPwM

First doig the same test as in my comment linked above.

  1. Skills
  • with convocation ticked on: every gem shows -4 to active minion limit
  • convocation ticked off: gemlist is not not even sorted by dps anymore, and every gem gives the same wrong stats
    grafik
  • when i tick my second raise spectre off: skills dps seems to be fine again
  1. seems fixed

  2. Minion dps is now shown, but the numbers are totally wrong.
    This is just the same wand i have already equipped.
    grafik

  • with convocation and my second raise spectre ticked off: everything seems to be fine
    grafik
  1. Every single item i hover gives -4 to active minion limit, which is obviously wrong. That is with convocation ticked on.
  2. When i tick concocation off, and change Main Skill to Shield Charge, every single item i hover shows +4 to active minion limit, also wrong.

@Paliak
Copy link
Contributor Author

Paliak commented Aug 25, 2024

#8245 Should work.
obraz

#8163 Should work.
obraz

  1. Skills.

Should now work.
obraz
obraz

  1. Minion dps is now shown, but the numbers are totally wrong.

Seems fixed.
obraz

Thanks for testing.

@Paliak
Copy link
Contributor Author

Paliak commented Aug 30, 2024

Marking as ready for review as i can't think of anything else to test.

Tested:

@Paliak Paliak marked this pull request as ready for review August 30, 2024 17:43
@Nightblade
Copy link
Contributor

This seems to fix the bug I noticed recently where the FullDPS line is absent from item tooltips when PoB auto-loads the last saved build on startup. Not extensively tested though.

@LocalIdentity LocalIdentity merged commit 118e0b4 into PathOfBuildingCommunity:dev Sep 20, 2024
1 of 2 checks passed
@Paliak Paliak deleted the fix-misc-fullDPS-issues branch September 20, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: accuracy Wording differences
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not showing Full Dps in item and skill comparison .
4 participants