Re-introduce the session locking mechanism #633
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As is evident from the various looting-related bug reports, "fast loot" addons still break Rarity's NPC detection method - even with the latest changes. I suspect this is due to how AceEvent handles the aggregated LOOT_READY triggers: By the time we're notified the fast loot addon has already taken all the loot, so Rarity can't see anything (and thus won't add any attempts). This should be largely identical to the previous implementation, the only change being that the delay is now configurable (through code only, for now). If spam-opening containers becomes a significant concern, the delay could be exposed to the config UI to allow users to adjust it so that attempts are no longer failing to be detected. Obviously, this is sub-optimal as ideally we'd be able to handle both scenarios gracefully. But one being seemingly more prevalent than the other, I'd rather not break "fast loot" addons and deal with the relatively fewer bug reports concerning container items not being counted. Perhaps a better solution exists that solves both problems at once, but I really don't have the time to do more than what's absolutely necessary here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As is evident from the various looting-related bug reports, "fast loot" addons still break Rarity's NPC detection method - even with the latest changes. I suspect this is due to how AceEvent handles the aggregated LOOT_READY triggers: By the time we're notified the fast loot addon has already taken all the loot, so Rarity can't see anything (and thus won't add any attempts).
This should be largely identical to the previous implementation, the only change being that the delay is now configurable (through code only, for now). If spam-opening containers becomes a significant concern, the delay could be exposed to the config UI to allow users to adjust it so that attempts are no longer failing to be detected.
Obviously, this is sub-optimal as ideally we'd be able to handle both scenarios gracefully. But one being seemingly more prevalent than the other, I'd rather not break "fast loot" addons and deal with the relatively fewer bug reports concerning container items not being counted. Perhaps a better solution exists that solves both problems at once, but I really don't have the time to do more than what's absolutely necessary here.
Follow-up to #629 and #620