Skip to content

Guess main socket group based on dps + Full import button#1401

Open
Musholic wants to merge 2 commits intoPathOfBuildingCommunity:devfrom
Musholic:guess_socket_dps
Open

Guess main socket group based on dps + Full import button#1401
Musholic wants to merge 2 commits intoPathOfBuildingCommunity:devfrom
Musholic:guess_socket_dps

Conversation

@Musholic
Copy link
Contributor

@Musholic Musholic commented Sep 16, 2025

Description of the problem being solved:

This PR allows to have a one-button full import while allowing to better guess which socket group is the main one based on DPS calculations of all groups. It also includes the main skill in Full DPS.

Note that if we cannot import with only one button, it causes an issue where the variable mainSkillEmpty = #self.build.skillsTab.socketGroupList == 0 is false because there is a skill coming from the tree ("Called shots" in my case).

This is also useful for the automatic test I'm preparing to have more relevant tests (having a skill correctly included for the full dps stats) (Musholic#11).

  • I tested the performance on a simple character import (it was very fast), however it needs to be tested on more complex characters before we merge this PR
    • There was some tests done by LocalIdentiy at the time the PR was made with no significant performance impact (around 130ms max delay). There is a possible performance improvements to skip check on non damaging skills but it needs a reliable way to tell if a skill is non damaging.

Steps taken to verify a working solution:

  • Tested with my lvl 35 character

Before screenshot:

image

After screenshot:

image

@LocalIdentity LocalIdentity added the enhancement New feature, calculation, or mod label Sep 17, 2025
@Musholic
Copy link
Contributor Author

The PR is rebased and ready to be merged IMO, I updated the description regarding the performance tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature, calculation, or mod

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants