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 Elusive and its modifiers, unify Wither and Withered #1708

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Quotae
Copy link

@Quotae Quotae commented Sep 27, 2019

Added support for Elusive. The option only shows up if the character has something that checks for it, including any gems, the Assassin node, and Badge of the Brotherhood, everything but the Ascendant Assassin node. Effect of Elusive on passives, items, and gems scales the buff accordingly. It's a simple toggle, no support for the scaling effect of the buff because I couldn't figure out how to do it, but the tooltip mentions that it decays over time.

image

Additionally, the Wither spell and other sources of Withered like Withering Touch now apply the same debuff, instead of being two separate debuffs that stack. The input box for WIthered Stacks is now in the Effective DPS column. The box now also shows up when Withering Step is equipped.

image

supports elusive and unifies wither with the withered debuff available elsewhere
left a comment in on accident
removed comment I didn't intend to leave in
fixed withering step not enabling elusive. I accidentally left in the remnants of a failed attempt to get it working for the things my implementation doesn't work for
Quotae referenced this pull request in Quotae/PathOfBuilding Oct 1, 2019
This reverts commit 689b2fc, reversing
changes made to 39853e0.
@Legrems
Copy link

Legrems commented Oct 2, 2019

You can already applied Withered to the enemy. Just add the skill wither, and make it 15 stacks :)

@Legrems
Copy link

Legrems commented Oct 2, 2019

And by the way, you realy need to have usefull commit name ^^'

- Added a display for current Elusive effect in the Calcs tab, under Movement Speed in Other Defences, which only shows up while Elusive.
- Much better support for Nightblade Support. Now gives Crit Multi and Base Crit Chance to attacks while using Claws or Daggers.
- Tweaked how I implemented Elusive Effect on gems. Now is implemented specifically for Claws/Daggers on Nightblade, and is added as a global Skill Mod for Withering Step and any future gems which give the stat.
- Are you Elusive? button works and shows up more consistently.
@Quotae
Copy link
Author

Quotae commented Oct 3, 2019

It's not just to apply Withered to the enemy, it's so that when you're using Withering Touch and the Wither spell, you can't apply both the Wither spell's 15 stacks AND the 15 Withering Touch stacks. Both the Wither spell and Withering Touch apply Withered, so they shouldn't stack separately, because they're the same debuff with a limit of 15 stacks. Also, Withering Step can supply Withered Stacks, so it doesn't make sense to have Withered Stacks under a skill-specific option, it should be in a general location.

And I left comments on all those commits, I just didn't realize I could change the name of them, lol. Got it this time though. Copy/paste changelog from the comment of that most recent commit:

  • Added a display for current Elusive effect in the Calcs tab, under Movement Speed in Other Defences, which only shows up while Elusive.
    image

  • Much better support for Nightblade Support. Now gives Crit Multi and Base Crit Chance to attacks while using Claws or Daggers, which it didn't do at all until now.

  • Tweaked how I implemented Elusive Effect on gems. Now it's implemented specifically for Claws/Daggers on Nightblade, and is added as a global Skill Mod for Withering Step and any future gems which give the stat.

  • Are you Elusive? button works and shows up more consistently.

Hopefully that's thorough enough for you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants