-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor channels #1423
base: dev
Are you sure you want to change the base?
Refactor channels #1423
Conversation
Signed-off-by: William Jeffcock <[email protected]>
816ef4e
to
51a6564
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if ChannelManager should just be renamed to ChatManager too
} | ||
|
||
public void setChannel(MatchPlayer player, Channel<?> channel) { | ||
Channel<?> previous = selectedChannel.get(player.getId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to get & put, put already gives you previous value
return; | ||
} | ||
|
||
context.store(MESSAGE_KEY, message.substring(spaceIndex + 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a trim() after the substring
} | ||
|
||
public static void playSound(MatchPlayer player, Sound sound) { | ||
SettingValue value = player.getSettings().getValue(SettingKey.SOUNDS); | ||
if (value.equals(SettingValue.SOUNDS_ALL) | ||
|| value.equals(SettingValue.SOUNDS_CHAT) | ||
|| (sound.equals(Sounds.DIRECT_MESSAGE) && value.equals(SettingValue.SOUNDS_DM))) { | ||
|| (sound.equals(DM_SOUND) && value.equals(SettingValue.SOUNDS_DM))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this in chat dispatcher even make sense anymore?
private static final String PREFIX_FORMAT = "%s: %s"; | ||
private static final String AC_FORMAT = | ||
TextTranslations.translateLegacy(ADMIN_CHAT_PREFIX) + PREFIX_FORMAT; | ||
private static final Sound DM_SOUND = sound(key("random.orb"), Sound.Source.MASTER, 1f, 1.2f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import Sounds.DIRECT_MESSAGE
from the Sounds
interface if it's needed; slipped through rebase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chat dispatcher is only kept for backwards-compat, it should simply not have any of this left
|
||
private final OnlinePlayerUUIDMapAdapter<MessageSenderIdentity> selectedPlayer, lastMessagedBy; | ||
|
||
private static final Sound SOUND = sound(key("random.orb"), Sound.Source.MASTER, 1f, 1.2f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This however, yeah, it should reference Sounds
for multi-version support
.append(text("] ", NamedTextColor.WHITE)) | ||
.build(); | ||
|
||
private static final Sound SOUND = sound(key("random.orb"), Sound.Source.MASTER, 1f, 0.7f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for this
Continuation of #1306 updated to new cloudy framework