New broadcast subscriber API

[dktapps]

Introduction

This PR introduces the following:

• MessageBroadcastSubscriber: allows subscribing to broadcastMessage() without being a CommandSender
• MinecraftMessageBroadcastSubscriber: allows subscribing to broadcastTip(), broadcastPopup() and broadcastTitle() in a similar manner to broadcastMessage() (previously entirely impossible)
• broadcastMessage(), broadcastTitle(), broadcastTip() and broadcastPopup() now require specifying a CommandSender $source - this is to allow subscribers to process messages differently based on who sent them (useful for implementing blocking features)
• broadcastMessage(), broadcastTitle(), broadcastTip() and broadcastPopup() now support providing a custom channel - this simplifies various use-cases, but was mainly needed to allow subscribers to know what the intended destination of the message was
• All broadcast functions in Server that used to accept CommandSender now accept MessageBroadcastSubscriber
• All broadcast functions in Server that used to accept Player now accept MinecraftMessageBroadcastSubscriber
• Player now implements MinecraftMessageBroadcastSubscriber

Relevant issues

#5352

Changes

API changes

• The following API methods now accept MessageBroadcastSubscriber[] $recipients instead of CommandSender[]:
  • Server::subscribeToBroadcastChannel()
  • Server::unsubscribeFromBroadcastChannel()
  • Server::unsubscribeFromAllBroadcastChannels()
  • Server::broadcastMessage()
  • PlayerChatEvent::__construct()
  • PlayerChatEvent::setRecipients()
• The following API methods now accept MinecraftMessageBroadcastSubscriber[] $recipients instead of Player[]
  • Server::broadcastTip()
  • Server::broadcastPopup()
  • Server::broadcastTitle()
• The following API methods now require a CommandSender $source as their first argument, and have a new string $channelId = Server::BROADCAST_CHANNEL_USERS argument before $recipients:
  • Server::broadcastMessage()
  • Server::broadcastTip()
  • Server::broadcastPopup()
  • Server::broadcastTitle()
• The following API methods now return MessageBroadcastSubscriber in place of CommandSender:
  • Server::getBroadcastChannelSubscribers()
  • PlayerChatEvent::getRecipients()
• Player now implements MinecraftMessageBroadcastSubscriber

Note: The introduction of source CommandSender and channel ID were not strictly necessary to achieve the base functionality of this PR. However, the broadcast subscriber interfaces are far more useful with this information included for reasons mentioned above.

I'm open to allowing these functions to accept null for a source. It would obviously be best for BC if the source params were optional, but in the case of broadcastTitle() specifically, that will be very messy and inconsistent.

Behavioural changes

None. This PR is not supposed to change observable behaviour.

Backwards compatibility

BC breaks described above.

Follow-up

• Split BROADCAST_CHANNEL_USERS into more channels, e.g. CHAT, JOIN_QUIT, GAME_EVENTS (death, skin change, etc)
• Have permissions tested by the broadcast system instead of the janky hack in Player that unsubscribes itself from channels when it doesn't have permissions
• Support Translatable in broadcastTip(), broadcastPopup() and broadcastTitle()
• Consolidate title parameters into a Title structure or similar
• Implement support for Xbox Live user blocking by passing along XUID with chat messages

Tests

This has been tested under the following situations:

• Player invokes command & observe player & console messages
• Console invokes command & observe player and console messages
• Chat works correctly

[SOF3]

rename to ChatBroadcastReceiver instead? It is ambiguous what exactly you are broadcasting, for example, sound packets.

[dktapps]

You could make the same argument about broadcast channels. I don't want the terminology to be inconsistent.

[dktapps]

We could even just throw out the interface entirely and require a closure. The only downside to closures is that they are a bit nasty to describe the type of.

Never mind, it's assumed that Player is an instance of ChatBroadcastSubscriber in some places...

[SOF3]

Is channel ID supposed to be something that plugins can define their own? Also, would it be a better idea to use singleton objects instead of IDs?

[dktapps]

Is channel ID supposed to be something that plugins can define their own? Also, would it be a better idea to use singleton objects instead of IDs?

Yes, they are entirely arbitrary. I don't know if there's a particular reason to use objects though... seems like a probably unnecessary complication, since I'm not concerned about channel IDs being stored on disk.

[ShockedPlot7560]

Seems ok to me. The fact that you can set an arbitrary channel ID allows good flexibility if plugins want to use this system to define channels.

[dktapps]

Changing this to draft as there's still a few design issues here. This turned out to be a lot more complicated to complete than I imagined.

[dktapps]

Closing this as the basic concept seems to be flawed. We could achieve the same thing by adding a simple MessageBroadcastEvent and adding some extra parameters to broadcastMessage(), and I'm sure plugin devs would rather have an event than a weird esoteric interface.

However, most of the follow-ups still apply and I'll submit them as issues separately.