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

[UI] Show config option as valid if any active skill has valid flags and tags, not just the main skill. #7362

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

spawnie-no-oni
Copy link
Contributor

@spawnie-no-oni spawnie-no-oni commented Feb 25, 2024

Fixes #7360 .

Description of the problem being solved:

Currently a config option in the Config tab will be set to show only if the selected main skill has both relevant tags and flags.

For instance, when using a projectile skill with Point Blank, the user can change the Projectile distance travelled config and the DPS calculation will be updated accordingly. If the user then changes the main skill dropdown to a non-projectile skill, the Projectile distance travelled config will then appear as invalid, even when still affecting the DPS calculation.

image

This also happens with similarly tag-limited config like any melee skill supported by Close Combat support.

Note that if a skill with the correct tag is selected as main skill, the config will show as valid, even though the the DPS of the selected skill is not affected by the config. For instance, if a Boneshatter gem is linked to Close Combat support, the config will appear valid as long as a melee skill is selectd as main skill, even if that melee skill is not supported by Close Combat (for instance an unlinked Leap Slam).

This is confusing.

This commit makes it so as long as any active skill has a corresponding tag, the config will show as valid. This will not affect DPS calculations as the tagTypesUsed and modsUsed arrays are only checked by the Config Tab UI code.

A possible downside is that someone could accidentally link say, Close Combat to Leap Slam, and then get confused why the config tab is appearing but not affecting their actual main skill dps. I think this is less potentially confusing for anyone who is familiar with PoE than the current state.

Steps taken to verify a working solution:

  • Create a new build.
  • Add one projectile attack to the build (for instance, Splitting Steel) and any other non-projectile skill.
  • In the Item tab, equip a Beltimber Blade.
  • Select the projectile attack as your main skill and go to the Configuration tab.
  • Set the Projectile travel distance config to 70.
  • Select smite as your main skill
  • Notice the Projectile Travel Distance checkbox displays no error.

(Alternatively you can import the linked build, open the Config tab, and select different skills as your main skill)

Link to a build that showcases this PR:

https://pobb.in/BEdB7P5GZ-5p

Before screenshot:

image

After screenshot:

image

…and tags, not just the main skill.

Currently a config option is set to show only if the selected main skill has both relevant tags and flags.

For instance, when using a projectile skill with Point Blank, the user can change the Projectile distance travelled config and the DPS calculation will be update accordingly. If the user then changes the main skill dropdown to a non-projectile skill, the projectile distance travelled config will then appear as invalid, even when still affecting the DPS calculation.
This also happens with similarly tag-limited config like any melee skill supported by Close Combat support.

Note that if a skill with the correct tag is selected as main skill, the config will show as valid, even though the the DPS of the selected skill is not affected by the config.
For instance, if a Boneshatter gem is linked to Close Combat support, the config will appear valid as long as a melee skill is selectd as main skill, even if that melee skill is not supported by Close Combat (for instance an unlinked leap slam).

This is confusing for some users.

This commit makes it so as long as any active skill has a corresponding tag, the confg will show as valid. This will not affect DPS calculations as the tagTypesUsed and modsUsed arrays are only checked by the Config Tab UI code.

A possible downside is that someone could accidentally link say, Close Combat to Leap Slam, and then get confused why the config tab is appearing but not affecting their actual skill dps. I think this is less potentially confusing for anyone who is familiar with PoE than the current state.
@Paliak Paliak added the user-interface Changes that only affect the UI label Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user-interface Changes that only affect the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Far Shot proj travel UI bug
2 participants