valkyrnstudios / RankSentinel

MIT License
4 stars 2 forks source link

Coordinate between players to avoid spamming #3

Closed SabreValkyrn closed 2 years ago

SabreValkyrn commented 2 years ago

If multiple people have the addon, offenders get hit with N whispers.

Coordinate between running addons so only one person whispers

Road-block commented 2 years ago

The way I would go about this, rather than start a ping-pong of messages between addons "you go", "no you go", "you first", "I insist" 😄

  1. Instantiate a groupMembers table for holding other group members with this addon enabled.
  2. Monitor GROUP_ROSTER_UPDATE
  3. In the GROUP_ROSTER_UPDATE event handler check for groupstatus with IsInGroup This is to differentiate between group changed/joined or group disbanded cases.
  4. If group disbanded just wipe your groupMembers table and do your announces as you would normally in solo mode.
  5. If group formed or members changed use the SendAddonMessage API to send a simple version ping and your whisper option enable status to RAID channel (it will default to PARTY automatically if you are not in a raid, so no need for complex logic to decide where to send)
  6. If you receive a version ping reply with version and status of the whisper option.
  7. For each instance of the addon receiving replies add the player to our groupMembers table (just tinsert the name in an array style table to facilitate sorting) if they replied with whisper status enabled.
  8. In the announcing code of each instance of the addon, do a simple alphabetic sort on the groupMembers table (which should contain a current list of all group members with the addon that have whisper sends enabled)
  9. Check if the first name (groupMembers[1]) is "you" and send or another group member and do not send.

This should minimize both addon comms and cpu usage and still have the desired effect. Only 1 player in a group with whispers enabled will send whispers. If no players have whispers enabled no one will send (that is the reason for sending an addon message with sentinel prefix and a message of v=x.y:w=1 (example format that can be parsed to get version and whisper option status)

SabreValkyrn commented 2 years ago

Thanks for the detailed recommendation! My work so far is on the avoid_multiple_spams branch.

I was registering GROUP_ROSTER_UPDATE in some of my earlier attempts, but was having troubles differentiating between the operations and it spamming.

Looked like at least 3 GROUP_ROSTER_UPDATE on every roster change (join or move), which seemed like a lot of cluster election/broadcasting, but that was from the perspective of existing members.

In case you missed it, I removed SpellSentinel from CurseForge. Feel free to use this repo as long as you like though, I just can't distribute a built addon with CurseForge because of licensing issues.

SpellSnob was an unofficial and unauthorized fork of GogoWatch. Work is ongoing to port SpellSentinel changes into GogoWatch.

Once I get a proof of concept done with avoid_multiple_spams I'll refocus my efforts on GogoWatch.

Road-block commented 2 years ago

I took a look.
I can see where you're going with it but it's over-engineered imho 😋

"Tom", "Dick" and "Harry" have SpellSentinel installed and whisper option : true.

When they join a group they send their version + whisper enable status. Now each addon has the same list, they sort it alphabetically {"Dick", "Harry", "Tom"). "Dick"'s SpellSentinel compares the first name to the player's name, it is the same > it announces.

"Harry"'s and "Tom"'s SpellSentinels compare player name to "Dick" it's not the same, they don't send. Each SpellSentinel instance can decide on its own if it should send or not with no further communication needed other than "x, y and z in my group have SpellSentinel installed with whispers : on")

SabreValkyrn commented 2 years ago

Using

function SpellSentinel:GROUP_ROSTER_UPDATE()
    if IsInGroup() then
        self:PrintMessage("GROUP_ROSTER_UPDATE:IsInGroup")
    else
        self:PrintMessage("GROUP_ROSTER_UPDATE:NotIsInGroup")
    end
end

This event triggers:

That seems really heavy, since there's no JOINED_GROUP event to register with which is really the event cluster quorum really needs.

I'll continue with making the most recent person in raid to trigger a quorum broadcast and election on each reload, zone transition, or login PLAYER_ENTERING_WORLD

SabreValkyrn commented 2 years ago

MVP done, bloated branch with too many other little things so merged before I look into rebuilding ability data.

Road-block commented 2 years ago

GROUP_JOINED and GROUP_LEFT exist but those afaik fire for the player themselves not other players in their group.

Since you started making use of the Ace framework you could utilize the RegisterBucketEvent player made API if you do not want to make your own batch event processing using C_Timer.

This would allow you to ignore all the "spam" around group roster changes and only react to the last event firing (depending on your interval)

If you want I can prototype just the group send logic and submit a patch for review.

Road-block commented 2 years ago

Leaving this here for reference.
These are the relevant events for group formation / disband (a couple cycles where the player leaves group or the last member of the group that is not the player leaves)

Clipboard 1

The GROUP_JOINED / GROUP_FORMED / GROUP_LEFT events would not need batching, GROUP_ROSTER_UPDATE would.

SabreValkyrn commented 2 years ago

Oh, fascinating, I didn't know about those details.

A pull request proposal would be fantastic!

SabreValkyrn commented 2 years ago

If you want I can prototype just the group send logic and submit a patch for review.

@Road-block I just butchered branches and main history to give RankSentinel a clean version history after the full rebuild, apologies if that broke your proof-of-concept PR.

https://github.com/valkyrnstudios/RankSentinel/issues/9#issuecomment-1000986079