Skip to content
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

Chat recording changes #26

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Chat recording changes #26

wants to merge 13 commits into from

Conversation

Iaiao
Copy link
Contributor

@Iaiao Iaiao commented Jan 10, 2021

Resolves: #12

  • Added an option to log everything players receive in the chat (not only chat but server/plugin messages and literally everything ProtocolLib can intercept).
  • Now the chat format, death messages, join/quit messages are taken from the events and not the config. Useful when you have local and global chats, or staff / supporter chats / channels / whatever

@Jumper251
Copy link
Owner

Thanks for the changes.
Some problems I have noticed:

  1. Join/Leave messages are recorded twice (as event and as everything)
  2. The same message is recorded for every player in the replay
  3. Replays from older versions probably would not be displayed correct

It will be difficult to read the chat with all the private messages when there are a lot of players in a replay.

@Iaiao
Copy link
Contributor Author

Iaiao commented Feb 21, 2021

Sorry for late reply, I'll fix 1 and test 3. 2 is probably a design problem because when you are recording 2 or more players, some users expect it to replay everything in players' chats, others expect another behavior. Maybe I will add something like "record only everything in my chat" to configuration, but the chat recording configuration is already a bit complicated, so I think the better design option would be "chat recording mode: DISABLED / CHAT / EVERYTHING IN CHAT / EVERYTHING IN EVERYONES CHAT", or maybe you have better configuration ideas?

@Jumper251
Copy link
Owner

I can't think of any use case where you would want so see the same message twice.

It might make more sense to record specific private or global messages from the plugin that sends them (with the API). Otherwise how would you know if the message is a global announcement (should only be displayed once) or a message that can be displayed multiple times.

@Iaiao
Copy link
Contributor Author

Iaiao commented Feb 22, 2021

Do you mean all plugins should work with AdvancedReplay's API or vice versa or what?
I think it's enough to "buffer" recorded packets for one game tick and if the message is a global announcement (there are same packets within one tick) replay them as [Player1, Player2, Player3 received]. Although it won't work with "Hi, " announcements, it should work for chat and most global messages

@Jumper251
Copy link
Owner

That could work but I would probably not show the player names with global announcements.
I think the options EVERYTHING IN CHAT / EVERYTHING IN EVERYONES CHAT can be combined into one.

@Iaiao
Copy link
Contributor Author

Iaiao commented Apr 19, 2021

I came up with the use of HoverEvent for displaying message recipients, so it doesn't take additional space and makes it easy to check which players were online at the moment. "everything" was renamed to "plugin_messages" so it makes sense for end users. Also it works with replays saved in older versions (and even displays messages!). There are some minor cosmetic bugs (e.g. messages saved in old replays are gray, join/leave/death messages don't show recipients when recording them from events ("plugin_messages" is turned off)), I'll fix them tomorrow.

@Jumper251
Copy link
Owner

Are join, quit and death events plugin or chat messages? They are recorded even if both options are set to false but will only be replayed when both are enabled.
If only one player is online no quit message will be recorded.

I think message duplicates should be filtered while recording to avoid storing unnecessary data.

It seems like some of your changes broke the reset_changes config option from a recent update.

@Iaiao
Copy link
Contributor Author

Iaiao commented May 3, 2021

Are join, quit and death events plugin or chat messages?

They are chat messages because they're more useful than players' command feedback.

They are recorded even if both options are set to false but will only be replayed when both are enabled.

Oh, I did something with config conditionals that now I don't even know what I did, now they should be clear.

If only one player is online no quit message will be recorded.

Fixed.

I think message duplicates should be filtered while recording to avoid storing unnecessary data.

Done.

It seems like some of your changes broke the reset_changes config option from a recent update.

It doesn't work on master for me (or maybe I don't understand what exactly it should do, but the blocks don't disappear after the end of the replay).

@Jumper251
Copy link
Owner

Why are messages sometimes recorded in messages and sometimes in actions?

@Iaiao
Copy link
Contributor Author

Iaiao commented May 8, 2021

They're recorded in messages when they're recorded as packets and we need to compress messages, there's no need to do this in event listener recorders. But yes, it would be better to record them in messages to simplify the code.


WrapperPlayServerChat packet = new WrapperPlayServerChat(event.getPacket());

String message = new TextComponent(ComponentSerializer.parse(packet.getMessage().getJson())).toLegacyText();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some tests on Spigot 1.8.8 and there is an error message with plugin_messages enabled while trying to record death messages. I think the ComponentSerializer cannot parse the message.

[17:53:59 ERROR]: [AdvancedReplay] Unhandled exception occured in onPacketSending(PacketEvent) for AdvancedReplay java.lang.IllegalArgumentException: No enum constant net.md_5.bungee.api.chat.HoverEvent.Action.SHOW_ENTITY at java.base/java.lang.Enum.valueOf(Unknown Source) ~[?:?] at net.md_5.bungee.api.chat.HoverEvent$Action.valueOf(HoverEvent.java:16) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.BaseComponentSerializer.deserialize(BaseComponentSerializer.java:71) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.TextComponentSerializer.deserialize(TextComponentSerializer.java:25) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.TextComponentSerializer.deserialize(TextComponentSerializer.java:17) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.TreeTypeAdapter.read(TreeTypeAdapter.java:58) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:803) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:868) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson$1.deserialize(Gson.java:126) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.ComponentSerializer.deserialize(ComponentSerializer.java:62) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.ComponentSerializer.deserialize(ComponentSerializer.java:17) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.TreeTypeAdapter.read(TreeTypeAdapter.java:58) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.read(TypeAdapterRuntimeTypeWrapper.java:40) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.internal.bind.ArrayTypeAdapter.read(ArrayTypeAdapter.java:72) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:803) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:868) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson$1.deserialize(Gson.java:126) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.TranslatableComponentSerializer.deserialize(TranslatableComponentSerializer.java:28) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.TranslatableComponentSerializer.deserialize(TranslatableComponentSerializer.java:16) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.TreeTypeAdapter.read(TreeTypeAdapter.java:58) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:803) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:868) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson$1.deserialize(Gson.java:126) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.ComponentSerializer.deserialize(ComponentSerializer.java:60) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.ComponentSerializer.deserialize(ComponentSerializer.java:17) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.TreeTypeAdapter.read(TreeTypeAdapter.java:58) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:803) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:768) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:717) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at com.google.gson.Gson.fromJson(Gson.java:689) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.md_5.bungee.chat.ComponentSerializer.parse(ComponentSerializer.java:34) ~[spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at me.jumper251.replay.replaysystem.recording.PacketRecorder$1.onPacketSending(PacketRecorder.java:317) ~[Replay.jar:?] at com.comphenix.protocol.injector.SortedPacketListenerList.invokeSendingListener(SortedPacketListenerList.java:195) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.SortedPacketListenerList.invokePacketSending(SortedPacketListenerList.java:149) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.PacketFilterManager.handlePacket(PacketFilterManager.java:588) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.PacketFilterManager.invokePacketSending(PacketFilterManager.java:564) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.ProtocolInjector.packetQueued(ProtocolInjector.java:338) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.ProtocolInjector.onPacketSending(ProtocolInjector.java:298) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.ChannelInjector.processSending(ChannelInjector.java:378) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.ChannelInjector.access$800(ChannelInjector.java:64) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.ChannelInjector$3.handleScheduled(ChannelInjector.java:343) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.ChannelInjector$3.onMessageScheduled(ChannelInjector.java:313) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.ChannelProxy$2.schedulingRunnable(ChannelProxy.java:127) [ProtocolLib-S4.jar:4.5.0] at com.comphenix.protocol.injector.netty.EventLoopProxy.execute(EventLoopProxy.java:95) [ProtocolLib-S4.jar:4.5.0] at net.minecraft.server.v1_8_R3.NetworkManager.a(NetworkManager.java:192) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.NetworkManager.handle(NetworkManager.java:141) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PlayerConnection.sendPacket(PlayerConnection.java:907) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PlayerList.sendAll(PlayerList.java:880) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PlayerList.sendMessage(PlayerList.java:1192) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PlayerList.sendMessage(PlayerList.java:1197) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityPlayer.die(EntityPlayer.java:423) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityLiving.damageEntity(EntityLiving.java:812) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityHuman.damageEntity(EntityHuman.java:800) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityPlayer.damageEntity(EntityPlayer.java:496) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityLiving.e(EntityLiving.java:939) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityHuman.e(EntityHuman.java:1440) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.Block.fallOn(Block.java:640) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.Entity.a(Entity.java:811) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityLiving.a(EntityLiving.java:160) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.EntityPlayer.a(EntityPlayer.java:621) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PlayerConnection.a(PlayerConnection.java:456) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PacketPlayInFlying.a(SourceFile:126) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PacketPlayInFlying$PacketPlayInPosition.a(SourceFile:57) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.PlayerConnectionUtils$1.run(SourceFile:13) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) [?:?] at java.base/java.util.concurrent.FutureTask.run(Unknown Source) [?:?] at net.minecraft.server.v1_8_R3.SystemUtils.a(SourceFile:44) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.MinecraftServer.B(MinecraftServer.java:715) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.DedicatedServer.B(DedicatedServer.java:374) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.MinecraftServer.A(MinecraftServer.java:654) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at net.minecraft.server.v1_8_R3.MinecraftServer.run(MinecraftServer.java:557) [spigot_server-1.8.8-R0.1-SNAPSHOT-latest.jar:git-Spigot-db6de12-18fbb24] at java.base/java.lang.Thread.run(Unknown Source) [?:?] [17:53:59 ERROR]: Parameters: net.minecraft.server.v1_8_R3.PacketPlayOutChat@7909c818[ a=TranslatableComponent{key='death.fell.accident.generic', args=[TextComponent{text='Jumper251', siblings=[], style=Style{hasParent=true, color=null, bold=null, italic=null, underlined=null, obfuscated=null, clickEvent=ClickEvent{action=SUGGEST_COMMAND, value='/msg Jumper251 '}, hoverEvent=HoverEvent{action=SHOW_ENTITY, value='TextComponent{text='{name:"Jumper251",id:"28bc683e-40e9-47bc-a1fb-e067b77d8262"}', siblings=[], style=Style{hasParent=false, color=null, bold=null, italic=null, underlined=null, obfuscated=null, clickEvent=null, hoverEvent=null, insertion=null}}'}, insertion=Jumper251}}], siblings=[], style=Style{hasParent=false, color=null, bold=null, italic=null, underlined=null, obfuscated=null, clickEvent=null, hoverEvent=null, insertion=null}} components=<null> b=1 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Chat recording
2 participants