ammaraskar / pyCraft

Minecraft-client networking library in Python
Other
813 stars 184 forks source link

Passing data to read method on ChunkDataPacket #101

Open zacholade opened 6 years ago

zacholade commented 6 years ago

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

joodicator commented 6 years ago

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:

  1. A better location for this information is in the ConnectionContext object, which will always be present as the context field of a Packet 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 Packets do not access the fields of the context which do change).

  2. A better place for the code which updates this information is in the react() method of the appropriate PacketReactor, rather than in read_packet().

zacholade commented 6 years ago

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.

joodicator commented 6 years ago

@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.