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

Add support for jewel limits #5666

Merged
merged 2 commits into from
Apr 1, 2023

Conversation

Regisle
Copy link
Member

@Regisle Regisle commented Feb 10, 2023

Built ontop of #5664 (So that one should be merged first)

Implements limits for jewels as well as an option to ignore the limits

This correctly limits most types of limits, including basic ones (like anima stone), radius jewels (like unnatural instinct), grand spectrums (limit of 3 across all grand spectrums), impossible escape (though this is a little buggy as it doesn't unallocate nodes that are allocated)

This is broken for historic jewels atm though

@QuickStick123 QuickStick123 added the enhancement New feature, calculation, or mod label Feb 10, 2023
Copy link
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do historic jewels really not have a limit in their jewelData? They look the same on the UI.

item.jewelData.limitDisabled = nil
if item.limit and not env.configInput.ignoreJewelLimits then
if jewelLimits[item.title] and jewelLimits[item.title] >= item.limit then
item.jewelData.limitDisabled = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this line for? I don't see anywhere the limitDisabled variable is used

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in src/Classes/PassiveSpec.lua to try and disable historic jewels, also answering the earlier question, no they dont have limits ingame, we give them limits in the unique data, but they dont actually have them

Copy link
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a commit to fix Timeless jewels. This PR still has the issue with any jewels that let you allocate things in their radius (Thread of Hope in addition to Impossible Escape). That considered, it's a minor problem, so this looks alright to me

@Wires77 Wires77 changed the title Jewel limits Add support for jewel limits Apr 1, 2023
@Wires77 Wires77 merged commit aa060c5 into PathOfBuildingCommunity:dev Apr 1, 2023
@Regisle Regisle deleted the JewelLimits branch April 2, 2023 02:00
Dullson pushed a commit to Dullson/PathOfBuilding that referenced this pull request Dec 6, 2023
* Initial Implementation of Jewel Limits

* Fix Timeless jewels

---------

Co-authored-by: Wires77 <[email protected]>
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