Option to fire on death animation#879
Conversation
…d instead of ActorDeath
…d instead of ActorDeath
…d instead of ActorDeath
iProdigy
left a comment
iProdigy
left a comment
There was a problem hiding this comment.
some quick comments to get the ball rolling on this review
| handleNotify(null); | ||
|
|
||
| if (self || actor.getActor() == lastTarget.get()) | ||
| lastTarget = new WeakReference<>(null); |
There was a problem hiding this comment.
if self and config.notifyOnAnim(), we don't want to clear lastTarget since the animation event comes later. then we can clear lastTarget from within onAnimationChanged
one remaining concern is interactions might change between time of ActorDeath & AnimationChanged, so killer detection may also be worse
| boolean self = client.getLocalPlayer() == actor.getActor(); | ||
|
|
||
| if (self && isEnabled()) | ||
| if (self && isEnabled() && !config.notifyOnAnim()) |
There was a problem hiding this comment.
not critical for this PR but: if we wanted to avoid delayed screenshot, we could capture now & use later (or destroy if unused for x ticks)
|
Made all the minor corrections. For killer detection: I'll need some more time to figure out how to hold on to and use/destroy the screenshot. |
The trade notifier has similar logic you can look at: https://github.com/pajlads/DinkPlugin/blob/master/src/main/java/dinkplugin/notifiers/TradeNotifier.java#L127 again: not a requirement for this PR (i'd care more about resolving the killer at ActorDeath & holding onto that info) |
2 participants
Summary
Added an option in the deathSection to fire the death notification on corresponding animations instead of ActorDeath.
When enabled, the handlenotify() method will simply be bypassed inside the onActorDeath() method and will be performed on death animation instead. This approach will prevent some rare false positives where the player reaches 0 hp, but doesn't actually die. All other events that trigger this notifier (onGameMessage and onScript) will remain the same.