-
Notifications
You must be signed in to change notification settings - Fork 221
Add support for misc mods in Uniques and Tree #554
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 misc mods in Uniques and Tree #554
Conversation
for 2 add. projs. for spell skills
|
Ended up finding a bunch of other surface level stuff I could tackle.
Think that's all, or very close. Let me know if there's anything I could do better. |
| ["projectiles deal (%d+)%% increased damage with hits and ailments for each enemy pierced"] = function(num) return { mod("Damage", "INC", num, nil, 0, bor(KeywordFlag.Hit, KeywordFlag.Ailment), { type = "PerStat", stat = "PiercedCount" }, { type = "SkillType", skillType = SkillType.Projectile }) } end, | ||
| ["(%d+)%% increased bonuses gained from equipped quiver"] = function(num) return {mod("EffectOfBonusesFromQuiver", "INC", num)} end, | ||
| ["(%d+)%% increased bonuses gained from equipped rings"] = function(num) return {mod("EffectOfBonusesFromRings", "INC", num)} end, | ||
| ["(%d+)%% chance for spell skills to fire 2 additional projectiles"] = function(num) return { mod("TwoAdditionalProjectilesChance", "BASE", num, nil , ModFlag.Spell) } end, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to integrate this into the main projectile count box instead of being separate as we use the projectile count of a skill in a number of areas for DPS calculations
|
I made a couple fixes to your PR before merging |
src/Modules/ModParser.lua
Outdated
| ["cannot be stunned if you have at least (%d+) crab barriers"] = function(num) return { flag("StunImmune", { type = "StatThreshold", stat = "CrabBarriers", threshold = num }), } end, | ||
| ["cannot be blinded"] = { flag("Condition:CannotBeBlinded"), flag("BlindImmune") }, | ||
| ["cannot be blinded while affected by precision"] = { flag("Condition:CannotBeBlinded", { type = "Condition", var = "AffectedByPrecision"}), flag("BlindImmune", { type = "Condition", var = "AffectedByPrecision"}) }, | ||
| ["cannot be blinded while on full life"] = { flag("Condition:CannotBeBlinded", { type = "Condition", var = "FullLife"}), flag("BlindImmune", { type = "Condition", var = "FullLife"}) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LocalIdentity coming back to this line. BlindImmune is used for the CalcSections page for "Blind Avoidance Chance".
PathOfBuilding-PoE2/src/Modules/CalcSections.lua
Line 1774 in 278ccf8
| { label = "Blind Avoid Ch.", haveOutput = "BlindAvoidChance", { format = "{0:output:BlindAvoidChance}%", { modName = { "AvoidBlind", "BlindImmune" } }, }, }, |
And in CalcDefence.lua,
PathOfBuilding-PoE2/src/Modules/CalcDefence.lua
Line 1615 in 278ccf8
| output.BlindAvoidChance = modDB:Flag(nil, "BlindImmune") and 100 or m_min(modDB:Sum("BASE", nil, "AvoidBlind"), 100) |
although I'm not sure on how it's used there exactly. Any specific reason why there is 2 flags like that? Is it possible/worth it to combine into one flag?
Also, CalcPerform.lua uses CannotBeBlinded, but doesn't seem to prevent blind on line 505.
| if modDB:Flag(nil, "Blind") and not modDB:Flag(nil, "CannotBeBlinded") then |
Toggling "are you blinded" is config lowers evasions by the 20%, or equipping and item with "you are blinded" also bypasses the check. I assume CannotBeBlinded should be priority right?
Adding
Condition:CannotBeBlinded seems to make it function properly.








EDIT: Check both comments plz
Description of the problem being solved:
The passive tree had multiple nodes for "%Increased/Reduced Projectile Speed for Spell Skills", at the top right of the tree, notables being "Turn the Clock Forward" and "Turn the Clock Back". While adding the Chance of 2 Add Projs, I realized the quality on Fireball also adds chance of 2 projs, so that seems to calculate too now.
The same cluster as well as another south west of it, had "% chance for Spell Skills to fire 2 additional Projectiles"
I figured a separate text box for the 2 additional projectile chance was a better option than tacking on fractions of a projectile to the projectile count. I decided on "2 Add. Proj. Chance" for the short description, although suggestions appreciated. I avoided using +2 because I feel that would cause confusion with +2 skills. The other names in the code like "TwoAdditionalProjectilesChance" feel long but necessary, as it's not just a chance of ONE, but two projs. So if GGG adds any mods that give a chance of one additional proj, it would have to be implemented separately.
Steps taken to verify a working solution:
-Tried to copy previous implementation of "Terrain Chain" to keep everything uniform.
-Added a spell flag to the % chance 2 proj, so that it doesn't apply to bow skills like snipe or LA.
-Tested ball lighting and ember fusilade in game to confirm it does apply to the skill. They work, and I'm assuming GGG has been consistent with this.
Link to a build that showcases this PR:
https://maxroll.gg/poe2/pob/3jkov05o
Before screenshot:
After screenshot: