pyCraft
pyCraft copied to clipboard
Passing data to read method on ChunkDataPacket
Hey! So I am interested in implementing the ChunkDataPacket for this client and have noticed that when reading this packet, the server expects the client to know which dimension it is in. (from information received from previous join game, respawn.. etc packets)
This way, the client knows whether to read a SkyLight Field in the Chunk Section Structure. See https://wiki.vg/Chunk_Format#Chunk_Section_structure .
However at the moment the connection object does not store information like this in memory and nor does the packet reactor pass this information to the read() method of a packet.
Would extending the packet reactor ( https://github.com/ammaraskar/pyCraft/blob/master/minecraft/networking/connection.py#L563 ) to do something along the lines of:
if packet_id in self.clientbound_packets:
packet = self.clientbound_packets[packet_id]()
packet.context = self.connection.context
if isinstance(packet, clientbound.play.ChunkDataPacket):
packet.read(packet_data, self.connection.dimension)
else:
packet.read(packet_data)
if isinstance(packet, (clientbound.play.JoinGamePacket, clientbound.play.RespawnPacket)):
self.connection.dimension = packet.dimension
return packet
else:
return packets.Packet(context=self.connection.context)
be a reasonable solution? Packets which do this include Join game and respawn packet. Implementing a couple of special cases here would work but WDYT? https://wiki.vg/Protocol#Join_Game https://wiki.vg/Protocol#Respawn
From my understanding, several packets require information similar to this and it is essential the read() method is passed the information, to parse the packet properly.
Thanks for any help :-D
I agree that it does look like the connection will need to keep track of the current dimension in order to properly read this packet; however:
-
A better location for this information is in the
ConnectionContext
object, which will always be present as thecontext
field of aPacket
instance when it's being read or written.The description says it is for "static" parameters, but that can be changed: it would also work for those which change during the course of a connection (as long as static- and class-methods of
Packet
s do not access the fields of thecontext
which do change). -
A better place for the code which updates this information is in the
react()
method of the appropriatePacketReactor
, rather than inread_packet()
.
Thanks for the informative response / help. I agree with your points. I will look into it :)
While on this topic, would information such as players_by_uuid
be worthwhile also storing in the Connection
object?
Adding an elif
to the react()
method in the PacketReactor
which goes:
elif packet.packet_name == 'player list item':
packet.apply(self)
This way the Connection
object always has a list of every player's name available by uuid as key.
@Zachy24 Unless a UUID-player mapping is needed by the internals, I think it's best not to do that, as it would be unnecessary processing if the user doesn't need that data. It's generally up to the user of the library to decide what packets they want to process and how, and the API of PlayerListItemPacket.PlayerList
makes it relatively easy for them to do it in the standard way, if they choose to.