Refactor Channels #1306
Refactor Channels #1306
Conversation
Signed-off-by: William Jeffcock <[email protected]>
| import tc.oc.pgm.util.Players; | ||
| import tc.oc.pgm.util.named.NameStyle; | ||
|
|
||
| public interface Channel<T> { |
There was a problem hiding this comment.
This could extend Adventure's Audience.
There was a problem hiding this comment.
given the extra overhead that a channel implies, it probably shouldn't.
You don't want someone treating this like it was just another casual audience, when it may not act like one at all (eg: requirement for a target to send within the channel). Effectively speaking the only channel that could properly implement audience would be global chat
| import tc.oc.pgm.util.Players; | ||
| import tc.oc.pgm.util.named.NameStyle; | ||
|
|
||
| public interface Channel<T> { |
There was a problem hiding this comment.
This API needs to be fully documented, explaining each method's purpose and params
| return getViewers(target); | ||
| } | ||
|
|
||
| default void sendMessage(ChannelMessageEvent<T> event) {} |
There was a problem hiding this comment.
| default void sendMessage(ChannelMessageEvent<T> event) {} | |
| void sendMessage(ChannelMessageEvent<T> event); |
either force the downstream to implement this, or have an actual default impl, but don't just leave an empty no-op method for something this critical.
I'd personally advocate for having a default impl here, just like broadcastMessage has
|
|
||
| default void sendMessage(ChannelMessageEvent<T> event) {} | ||
|
|
||
| default Component formatMessage(T target, @Nullable MatchPlayer sender, Component message) { |
There was a problem hiding this comment.
i believe this method should not default to global-chat behavior, force downstream to implement it too
| } | ||
|
|
||
| default void registerCommand(PaperCommandManager<CommandSender> manager) { | ||
| String name = getAliases()[0]; |
There was a problem hiding this comment.
if aliases is an empty array, this throws an out of bounds exception. Throw your own illegal argument exception instead
| String[] aliases = new String[getAliases().length - 1]; | ||
| System.arraycopy(getAliases(), 1, aliases, 0, aliases.length); |
There was a problem hiding this comment.
This needs to go into some util, you could also use Arrays.copyOfRange:
| String[] aliases = new String[getAliases().length - 1]; | |
| System.arraycopy(getAliases(), 1, aliases, 0, aliases.length); | |
| String[] aliases = Arrays.copyOfRange(getAliases(), 1, getAliases().length); |
| manager | ||
| .commandBuilder("msg", "tell") | ||
| .argument( | ||
| StringArgument.<CommandSender, MatchPlayer>ofType( | ||
| TypeToken.get(MatchPlayer.class), "target") | ||
| .withParser( | ||
| manager | ||
| .getParserRegistry() | ||
| .createParser( | ||
| TypeToken.get(MatchPlayer.class), ParserParameters.empty()) | ||
| .orElseThrow(IllegalStateException::new)) | ||
| .build()) | ||
| .handler( | ||
| context -> { | ||
| MatchPlayer sender = | ||
| context.inject(MatchPlayer.class).orElseThrow(IllegalStateException::new); | ||
| final MatchPlayer target = context.<MatchPlayer>get("target"); | ||
| setSelectedTarget(sender, target); | ||
| PGM.get().getChannelManager().setChannel(sender, this); | ||
| })); | ||
|
|
||
| manager.command( | ||
| manager | ||
| .commandBuilder("msg", "tell") | ||
| .argument( | ||
| StringArgument.<CommandSender, MatchPlayer>ofType( | ||
| TypeToken.get(MatchPlayer.class), "target") | ||
| .withParser( | ||
| manager | ||
| .getParserRegistry() | ||
| .createParser( | ||
| TypeToken.get(MatchPlayer.class), ParserParameters.empty()) | ||
| .orElseThrow(IllegalStateException::new)) | ||
| .build()) | ||
| .argument( | ||
| StringArgument.<CommandSender>builder("message") |
There was a problem hiding this comment.
you can chain commands like these.
after the .handler(context -> ...) you can append the next .argument and another .handler. The first handler is used if the command is up to the first argument, the second is used when the command also has the 2nd param
|
|
||
| @Override | ||
| public Map<String, Object> processChatShortcut(MatchPlayer sender, String message) { | ||
| if (message.length() == 1) throw usage("/msg <player> [message]"); |
There was a problem hiding this comment.
if it's a shortcut, it probably isn't a /command syntax
| if (message.length() == 1) throw usage("/msg <player> [message]"); | ||
| Map<String, Object> arguments = new HashMap<String, Object>(); | ||
|
|
||
| int spaceIndex = message.indexOf(" "); |
There was a problem hiding this comment.
| int spaceIndex = message.indexOf(" "); | |
| int spaceIndex = message.indexOf(' '); |
|
|
||
| SettingValue option = sender.getSettings().getValue(SettingKey.MESSAGE); | ||
| if (option.equals(SettingValue.MESSAGE_OFF)) | ||
| throw exception("command.message.disabled", text("/toggle dm", NamedTextColor.RED)); |
There was a problem hiding this comment.
could be made into a clickable text to toggle
| } | ||
|
|
||
| for (MatchPlayer viewer : getMatch().getPlayers()) { | ||
| if (shouldShowTouched(toucher.getParty(), viewer.getParty()) |
There was a problem hiding this comment.
Pretty sure this is a behavior change with matches with +2 teams. Before it would show this to all players in a team that had touched, while now it will only show to the team that touched.
Eg, a 4-team DTC map:
Blue touches red core <- Blue is notified
Green touches red core <- Before, both green and blue are notified, now only green is.
This PR refactors channels and adds some new features:
Channelclass that allows other plugins (such as PGMSquads) to implement custom channels/msg <target>is now a channel, and therefore can be set to your default - messages will be sent to your selected target without running/msgor/rMatch, and others in the newMatch)