garbagemule / MobArena

MobArena plugin for Minecraft
GNU General Public License v3.0
193 stars 151 forks source link

MobArena prefix for isolated chat to distinguish chat from global chat - i.e. it being isolated. #395

Open mibby opened 6 years ago

mibby commented 6 years ago

Was wondering since MobArena utilizes a global messenger for prefix, if a prefix could be added to the isolated chat while part of the mobarena, so players know their chat is arena only.

jwflory commented 6 years ago

@garbagemule I was thinking about this one – what if we echoed a player's message back in the chat window to them with a [MobArena] prefix for whatever they typed? To me, that makes it clear to a user that MobArena is interpreting everything you type, and if that's not what you want, it should be obvious to exit the isolated chat.

Thoughts?

garbagemule commented 6 years ago

This is a very simple problem at first glance, but consider chat plugins that change the message format. I see only three potential ways to solve this:

A. Ignore chat plugins. MobArena isolated chat becomes its own thing with only very basic formatting customization (in the form of the messenger prefix). B. Make MobArena's isolated chat more customizable. This would require a survey of existing chat plugins to see what kind of formatting they support, and then mimicking this behavior, hoping to give server owners a way to make the chat look "flush" with the rest of the experience they're trying to create. C. Integrate with the chat plugins somehow, e.g. via Vault. Make the isolated chat a "channel" of the chat plugin, such that it can take care of everything. This, if possible, is probably the most versatile solution, and MobArena already has a dependency on Vault, so it requires only "more of the same".

Perhaps we could start off with option A and see if anyone complains about the formatting. If not, job's done, and it'll probably take no more than an hour or two to implement and wrap up. Then if someone does complain about it, perhaps if option C is possible, we could go that route at that point in time. I like to not break people's setups, but I am also a fan of the YAGNI(y) principle.

Option B is risky, because it requires mimicking behavior of other plugins in an area that they are specialized in. To be "complete" may require reinventing the wheel.

jwflory commented 6 years ago

@garbagemule Options B and C seem like costlier methods. I would opt for Option A because I think it's the least expensive option for developer time and resources, and I see the benefits for all three options as roughly the same.

mibby commented 6 years ago

Any update on a prefix for MobArena's isolated chat, similar to PVPArena? :)

garbagemule commented 6 years ago

No updates. Still not sure what the best idea here is. We will have bStats in the next release, and I'll probably add a few extra graphs to it to help figure out how much the isolated chat feature is actually being used. If it's a lot of people, it would make sense to gather some more feedback before making a decision. Otherwise, if almost no one uses it, the impact is low enough that we could try.

@mibby have you tried to solve this with a chat plugin with proximity-based channels? It honestly seems like the absolute best way to go about this, because you get a plugin that does exactly what it's supposed to do, and you have 100 % control over what happens. With MobArena using weird hacks, you'll lose a lot of that potential control.

Edit: Looks like I mixed this issue and the Bungee issue up a bit. I'm still interested in seeing what a proximity based chat plugin solution could do for something like this (as well as for the Bungee issue). Is this something you can look into, @mibby?

garbagemule commented 6 years ago

@mibby come on Discord :)

mibby commented 5 years ago

@garbagemule Any possible update on isolating MobArena chat with a prefix? I recently installed DiscordSRV and MobArena chat is bleeding into the global chat bridge (discord channel), while it is not showing up in-game. Testing PVPArena, chat is properly isolated both in-game global chat and discord bridge - and has the prefix to signify players are chatting in the arena only.

SaitDev commented 5 years ago

To deal with DiscordSRV but doesnt prevent other plugins from processing the chat event im afraid this can be done only on DiscordSRV side. That will give the best win edit: I'll send them a pull request this weekend or next week when i have some spare time :D

mibby commented 5 years ago

Wouldn't it be best for MobArena to properly isolate and prefix the chat as that specific arena's chat instead? That way you wouldn't have to rely on BungeeCord, other plugins that listen for global chat events, or DiscordSRV to fix separately.

i.e. How PVPArena handles player chat works perfectly.

SaitDev commented 5 years ago

Not really. See https://github.com/garbagemule/MobArena/issues/320#issuecomment-286911968 For add. prefix, no i dont mean about this, but about the issue with discordsrv. Hope this this make it clear for u

garbagemule commented 5 years ago

@mibby can we explore chat plugins and proximity-based channels? With MobArena's new command things and permission things, I'm pretty sure we could do something without changes in MobArena.

mibby commented 5 years ago

@garbagemule I'll take another deep look at my environment and see if I can find a solution, but my situation is a bit complicated. I don't really rely on using any channel type chat plugins. I have everything inter-connected using the normal "global" chat that is parsed. This is to maintain a community and have everything connected across every environment - BungeeCord, all servers, discord, etc.

With the exception of a separate staff channel (built into ChatControlPro without having to actually use channels), I tend to keep all arena-like plugins isolated so only participating players inside the arena can see that chatter. Seeing global chat is fine inside the arena - it's preventing arena spam from entering global that's the problem. Most of the arena plugins I've used or come across throughout the years have supported isolating chat and a prefix when chatting while inside the arena. MobArena is just unfortunately one of the ones that doesn't fully support a proper type of isolation or player awareness (prefix).

garbagemule commented 5 years ago

MobArena is just unfortunately one of the ones that doesn't fully support a proper type of isolation

I am a bit of a pedant, which is probably why this sentence annoys me. The problem here is not MobArena's solution to chat isolation, it's BungeeCord's approach to chat propagation. MobArena receives a chat event and filters out the players who shouldn't receive it, and lets the chat event fly. This is the proper way to handle it. The chat event should not be cancelled, because that implies that the chat message doesn't propagate out to anyone. The proper way for BungeeCord to handle this would be to somehow inject a recipient into the chat event that we can then filter out, or to only propagate chat messages where the number of recipients is the same as the number of players on the server. BungeeCord is a hack and a workaround to some limitation in Minecraft server setups, and the responsibility of dealing with issues introduced by that hack lies on the hack, not on plugins following protocol. Other plugins that solve the problems that BungeeCord introduces are doing the developer community a disservice by making "BungeeCord support" a thing in plugins that don't have anything to do with BungeeCord.

I am completely aware of the problem, but I disagree with the proposed solution, because it is a hacky workaround to an issue introduced by a hacky workaround, and it affects everyone, including those for which the proper solution actually works. A specific use case is only interesting in itself for defining the problem, but it is insufficient for defining a solution. If we cancel the chat event and use MobArena's messenger to propagate messages, not only are we using the API completely incorrectly, we also force the responsibility of a chat plugin on to MobArena, having to mimic formatting one way or another. This breaks the overall server experience for people who use formatting different to the one we guess is "the right one". If a single-node server has a chat plugin that changes the style from <garbagemule> hi to garbagemule: hi or /dev/garbagemule$ hi, that style is now broken with MobArena's isolated chat because people running multi-node servers are pestered by BungeeCord being broken.

A chat plugin that supports proximity-based joining/leaving and "activation" could solve the problem: Define a region where the "arena channel" is effective - around the arena region would make sense - and then as players join the arena, they join that channel and communicate within it, all the while being able to still "hear" what's going on "outside". Because BungeeCord is broken w.r.t chat, I'm assuming if such a chat plugin exists, it also deals wtih BungeeCord accordingly.

I could implement the suggested, improper way of doing this, and I could do it right now. It's not that I am incapable of doing it. It's not that I don't understand the problem. It's not that I'm lazy. It's that the proposed solution is fundamentally wrong, and I want to explore proper ways of handling it instead of implementing a hack in MobArena that has potentially negative consequences for other people who are not part of the debate.

I hope that makes sense, and I'm sorry if I come across as grumpy.

mibby commented 5 years ago

If the goal is to not cancel the chat event but instead only change the recipients of who the message is sent to (participating players only), would it be possible to append a ConversationPrefix to the message and act solely as a message relay? That way MobArena would not have to change formatting styles defined across any variety of chat plugin and could just append a prefix configured as the ConversationPrefix before each message is relayed out? Utilizing the global prefix setting in the main config or one defined per-arena.

MobArena in essence would be relaying the messages to everyone as a form of plugin channel message so BungeeCord / DiscordSRV would not pick it up and receive it as global chat?

Outside Arena; Player Chats > Chat Plugin Formats Chat > Message is sent out to everyone globally.

Inside Arena; Player Chats > Chat Plugin Formats Chat > MobArena receives the message > MobArena appends ConversationPrefix before message > MobArena calculates participating players > Message is sent out to participating players only.

My understanding may be completely wrong for how it is intended to function in the API, but hopefully can spin some ideas around to resolve the issue properly without having to find and rely on a 3rd party location / region based channel plugin.

garbagemule commented 5 years ago

We could easily tack a prefix on the message. That's not an issue in and of itself, but I don't think that's going to help much. Just to clear up any confusion about how chat events and the faux chat messages work:

When a player types in the chat, the message they type is wrapped in a chat event which is fired off. Let's say I type hello. We now have a chat event which consists of a message (hello), a sender (a reference to the Player instance that represents my presence on the server), and a set of recipients, which by default is every player on the server.

What MobArena's isolated chat does is that it removes non-arena players from the list of recipients. This means that the message is untouched, and MobArena makes zero assumptions about anything any other plugin might be doing with the event.

The proposed solution requires that we cancel the event so that the server doesn't see it as a successful event, and thus BungeeCord won't either, so it won't propagate it to other nodes in the network. But now we have to manually send the chat message to the arena players. The only feasible way to do this is to use the sendMessage() method. This method takes a raw string to send to the player. If we just blindly take the message from the chat event and send that to the arena players, they will see the raw string in their chat window, i.e. it is no longer a chat message, so it isn't processed by the server as one, so it's just the text. You would see hello on your screen, not <garbagemule> hello. This means that we would have to try to mimic how the server shows chat messages, but we can't feasibly know how a chat plugin might format the messages. Because this stuff is fully customizable in chat plugins, we can never be certain.

I don't know the internals of BungeeCord, so I don't know if there is a way to prevent a successful chat message from propagating to other nodes. But I can't see how tucking a prefix on the message will help anything. It'd just propagate a message all the same, but this time the message contains a prefix. How is that different from a chat event that starts with the same prefix?

SaitDev commented 5 years ago

mobarena extension now support discordsrv, isolate chat wont be sent by discordsrv. Also placeholderapi as well, here what I test with my chat plugin venturechat chat config: format: '{mobarena_arena_prefix}&r&o{nickname}&r:' result: https://imgur.com/a/CMHk4Jk ChatControl Pro is supporting placeholder api https://github.com/kangarko/ChatControl-Pro/wiki/Variables If you see this solution give enough what you expect, feel free to test this build https://ci.appveyor.com/project/SaitDev/mobarenaextension/build/artifacts

mibby commented 5 years ago

Thanks @SaitDev! I'll give your extension a test and see if it is exactly what I'm looking for.

SaitDev commented 5 years ago

that was old build, i just update link to latest

mibby commented 5 years ago

I noticed and already grabbed the latest. :P Appreciate it though! I'll give it a test in a little bit.

mibby commented 5 years ago

@garbagemule @SaitDev After further testing with the MobArenaExtension plugin, I believe it perfectly does everything I requested in this ticket. It isolates the arena chat from DiscordSRV so it isn't processed as global chat when isolation is on and having the {mobarena_arena_prefix} set before the player name in the chat formatting displays the [MobArena] prefix only when players are actually inside the arena - effectively serving as an arena isolated chat prefix like PVPArena.

Unless you want to actually implement a distinguished chat prefix when isolation is turned on, this workaround works perfectly for me. :)

mibby commented 5 years ago

@garbagemule I believe you may have broke the isolated chat feature in MobArena or @SaitDev, the placeholder is not detecting it and is overriding as public chat? :( Players inside the arena seem to be able to chat publicly despite isolation turned on.

link

MobArena version 0.103.1 MobArenaExtension version 0.1-SNAPSHOT (dev 19) ChatControlPro version 7.11.11 Paper dev 509 (Spigot 1.13.2)

MobArena Config

arenas:
  default:
    settings:
      isolated-chat: true

MobArena Extension Config

general:
  log-level: 2
extension:
  mythicmob:
    enable: false
  placeholderapi:
    enable: true
  discordsrv:
    enable: true

ChatControl Formatting (cut down for sake of simplicity in example)

Formats:
  Chat:
    MobArena:
      Message: '{mobarena_arena_prefix}'
      Sender_Permission: chatcontrol.groups.default
    Player:
      Message: '{display_name}&f: '
    The_Rest:
      Message: '{message}'

It should display the entire message formatting as {mobarena_arena_prefix} playername: message with the mobarena prefix only showing up when they are actually inside the arena. Intended behavior with isolation enabled is the message should only be sent to everyone also part of the arena, not to everyone globally.

With the PVPArena plugin, chat is still properly isolated inside those arenas only.

SaitDev commented 5 years ago

so the placeholder only show up when they are inside arena though their chat is sending as public?

mibby commented 5 years ago

That seems to be the case, yes. Which leads me to believe isolated-chat is actually not functioning as intended.

SaitDev commented 5 years ago

I test on paper 1.13.2 - 509 with venturechat 2.13.1 instead of chatcontrolpro but no issue. Unfortunately i dont have chatcontrolpro to try it out

mibby commented 5 years ago

@garbagemule Any ideas? :( ChatControlPro shouldn't be sending chat any differently than normal global public chat. I believe MA is either not isolating or getting the list of active participants to send chat to wrong.

garbagemule commented 5 years ago

There have been no changes to how chat is handled since the feature was introduced.

As always, test on plain Spigot with nothing but MobArena installed. I can't currently test this myself, but give it a whirl. If it works in that setting, your princess is in another castle.

mibby commented 5 years ago

@garbagemule I've finally had the time to test this issue again but for the life of me, I simply cannot figure out why isolation doesn't work.

The setting is enabled.

      isolated-chat: true

Disabling MobArenaExtension plugin, isolation still does not work using the discord test build as of 02/16/19.

Paper dev 571 (Spigot 1.13.2) ChatControlPro v7.14.1

This use to work fine with an earlier MobArena version with ChatControlPro, so I have my doubts it is ChatControlPro causing the breakage. If it is ChatControl, perhaps @kangarko may know what changed.

kangarko commented 5 years ago

@mibby if ChatControl should cause that, it's probably a priority issue

Try changing our event priorities of Listener according to this guide: https://github.com/kangarko/ChatControl-Pro/wiki/Listener-Priorities

mibby commented 5 years ago

@kangarko I tried the following but nothing seems to work.

Original settings.

Listener_Priority:
  Checker: HIGH
  Formatter: HIGH
  Signs: HIGHEST

Lowered

Listener_Priority:
  Checker: NORMAL
  Formatter: NORMAL
  Signs: HIGHEST

Lowest

Listener_Priority:
  Checker: LOWEST
  Formatter: LOWEST
  Signs: HIGHEST

Highest

Listener_Priority:
  Checker: HIGHEST
  Formatter: HIGHEST
  Signs: HIGHEST

Removing ChatControlPro, MobArena isolation works as intended.

kangarko commented 5 years ago

Continuing at https://github.com/kangarko/ChatControl-Pro/issues