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

Update active skill checks to compare by base gem where appropriate #7126

Merged

Conversation

sida-wang
Copy link
Contributor

@sida-wang sida-wang commented Dec 20, 2023

Fixes #7121, fixes #7105, fixes #7146, fixes #7187.

Description of the problem being solved:

Currently skill name matching is performed on exact matches. @Paliak recently introduced logic to match against the base gem for Replica Dragonfang's Flight in #7101.

This PR updates the logic to use the gem's gameId which is the same across variants and applies the check to the "SkillName" type in modStore.lua as well as the ifSkill check in ConfigTab.lua which can be activated using the includeTransfigured option.

Implements a helper function in CalcTools.lua calcLib.getGameIdFromGemName(gemName, dropVaal) which performs lookups in data.gemForBaseName and data.gems to get the gameId. This includes a type check on the input to ensure string due to the listOrSingleIfOption function passing the entire table to the function if no table entries satisfy the condition. The dropVaal option provides a boolean option to find the base non-vaal gem of a skill which has been used throughout this PR. The option is kept in case specific scenarios are identified in the future which require Vaal gems to be treated separately to their non-Vaal counterparts.

Performed a full pass of all type = "SkillName" tags and ifSkill tags in the source code to update them to the new logic where appropriate.

Steps taken to verify a working solution:

  • Tested examples from linked issues.

Non-reaper damage penalty no longer applying across variants
image

Tavukai and Mark of the Red Covenant now applying to SRS variants.
image

  • Confirmed Dragonfang's behaviour still working correctly

@sida-wang sida-wang marked this pull request as draft December 20, 2023 08:29
@sida-wang sida-wang changed the title Update tooling to enable base gem matching Update tooling to allow matching skills based on their base skill Dec 20, 2023
@QuickStick123 QuickStick123 added the bug: behaviour Behavioral differences label Dec 20, 2023
@sida-wang sida-wang changed the title Update tooling to allow matching skills based on their base skill Update active skill checks to compare by base gem where appropriate Dec 20, 2023
@sida-wang sida-wang marked this pull request as ready for review December 21, 2023 00:07
@LocalIdentity LocalIdentity merged commit 3a07117 into PathOfBuildingCommunity:dev Jan 2, 2024
@sida-wang sida-wang deleted the update-skillname-tag branch January 20, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: behaviour Behavioral differences
Projects
None yet
3 participants