Stop chat stream for picking up faction/party "private" chat messages #170

Merged
merged 2 commits into from Aug 3, 2012

Conversation

Projects
None yet
2 participants
@lucasec
Contributor

lucasec commented Jul 27, 2012

Not sure if this is an issue or intentional.

JSONAPI's chat stream picks up messages sent over Faction chat (Factions) and Party chat (mcMMO).

I adjusted the event priority so JSONAPI will not receive events for those messages.

Depending on use cases, picking up those messages may not be a bad thing. In Adminium's case, seeing party/faction chat would be valuable, except that the messages would not appear differently than normal chat messages. In my case, I would not want such messages appearing as I am using the chat stream on a public website.

Raise chat event handler priority
JSONAPI was picking up faction chat and party chat messages, which may
be intentional.  For my use, I only wanted to pick up global chat msgs.
@lucasec

This comment has been minimized.

Show comment Hide comment
@lucasec

lucasec Jul 27, 2012

Contributor

It seems my commit also included a new build. My apologies.

Contributor

lucasec commented Jul 27, 2012

It seems my commit also included a new build. My apologies.

@alecgorge

This comment has been minimized.

Show comment Hide comment
@alecgorge

alecgorge Jul 27, 2012

Owner

Thanks for the PR, but before I merge it, why does heightening the priority make it not receive messages by Factions?

BTW, Including the new build is good here because my CI server doesn't actually do the compiling (not enough memory on the VPS ;( ), but it does extract those artifacts and puts them up for download.

Owner

alecgorge commented Jul 27, 2012

Thanks for the PR, but before I merge it, why does heightening the priority make it not receive messages by Factions?

BTW, Including the new build is good here because my CI server doesn't actually do the compiling (not enough memory on the VPS ;( ), but it does extract those artifacts and puts them up for download.

@lucasec

This comment has been minimized.

Show comment Hide comment
@lucasec

lucasec Jul 27, 2012

Contributor

I dug up the event priority stuff here: http://jd.bukkit.org/apidocs/org/bukkit/event/Event.Priority.html
Admittedly, it says the methods are depreciated, but this still works.

Basically, a high priority means JSONAPI will get the event after plugins with a lower priority. Seems kinda backwards to me-- but that's what the docs say. I've tested it and it works.

When Faction chat or Party chat is on, Factions or mcMMO, respectively, catches the chat event, cancels it, and then sends messages out to the members of the faction/party. So if JSONAPI gets the event after those two plugins, the event will already have been canceled if it was not a public chat, so JSONAPI's handler will not receive the event because ignoreCanceled=true is set. Without the priority set, JSONAPI was getting the chat events before Factions/mcMMO had a chance to cancel it.

Contributor

lucasec commented Jul 27, 2012

I dug up the event priority stuff here: http://jd.bukkit.org/apidocs/org/bukkit/event/Event.Priority.html
Admittedly, it says the methods are depreciated, but this still works.

Basically, a high priority means JSONAPI will get the event after plugins with a lower priority. Seems kinda backwards to me-- but that's what the docs say. I've tested it and it works.

When Faction chat or Party chat is on, Factions or mcMMO, respectively, catches the chat event, cancels it, and then sends messages out to the members of the faction/party. So if JSONAPI gets the event after those two plugins, the event will already have been canceled if it was not a public chat, so JSONAPI's handler will not receive the event because ignoreCanceled=true is set. Without the priority set, JSONAPI was getting the chat events before Factions/mcMMO had a chance to cancel it.

@lucasec

This comment has been minimized.

Show comment Hide comment
@lucasec

lucasec Jul 27, 2012

Contributor

Hey I'm not sure if that build is actually up to date. If you could, hold off a sec on merging it.

Contributor

lucasec commented Jul 27, 2012

Hey I'm not sure if that build is actually up to date. If you could, hold off a sec on merging it.

Build #2
Apparently the first build didn't go through.
@lucasec

This comment has been minimized.

Show comment Hide comment
@lucasec

lucasec Jul 27, 2012

Contributor

Apparently my local source just reset for some reason. I re-cloned my repo and built again just to be safe. Noob mistake I guess.

Contributor

lucasec commented Jul 27, 2012

Apparently my local source just reset for some reason. I re-cloned my repo and built again just to be safe. Noob mistake I guess.

alecgorge added a commit that referenced this pull request Aug 3, 2012

Merge pull request #170 from lucasec/master
Stop chat stream for picking up faction/party "private" chat messages

@alecgorge alecgorge merged commit 5bf9d4d into alecgorge:master Aug 3, 2012

@alecgorge

This comment has been minimized.

Show comment Hide comment
@alecgorge

alecgorge Aug 3, 2012

Owner

Okay, it the priority thing seems backwards to me too.

Merged :)

Owner

alecgorge commented Aug 3, 2012

Okay, it the priority thing seems backwards to me too.

Merged :)

alecgorge added a commit that referenced this pull request Feb 25, 2013

Merge pull request #170 from lucasec/master
Stop chat stream for picking up faction/party "private" chat messages

alecgorge added a commit that referenced this pull request Mar 16, 2013

Merge pull request #170 from lucasec/master
Stop chat stream for picking up faction/party "private" chat messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment