Refactor out the action of writing out the packet header.

This allows subclasses of Packet to just call the new
method instead of having to duplicate the header writing
and compression code.

minecraft/networking/packets.py

@@ -107,18 +107,9 @@ class Packet(object):
                 value = data_type.read(file_object)
                 setattr(self, var_name, value)
 
-    def write(self, socket, compression_threshold=None):
-        # buffer the data since we need to know the length of each packet's
-        # payload
-        packet_buffer = PacketBuffer()
-        # write packet's id right off the bat in the header
-        VarInt.send(self.id, packet_buffer)
-
-        for field in self.definition:
-            for var_name, data_type in field.items():
-                data = getattr(self, var_name)
-                data_type.send(data, packet_buffer)
-
+    # Writes a packet buffer to the socket with the appropriate headers
+    # and compressing the data if necessary
+    def _write_buffer(self, socket, packet_buffer, compression_threshold):
         # compression_threshold of None means compression is disabled
         if compression_threshold is not None:
             if len(packet_buffer.get_writable()) > compression_threshold != -1:
@@ -140,6 +131,20 @@ class Packet(object):
         VarInt.send(len(packet_buffer.get_writable()), socket)  # Packet Size
         socket.send(packet_buffer.get_writable())  # Packet Payload
 
+    def write(self, socket, compression_threshold=None):
+        # buffer the data since we need to know the length of each packet's
+        # payload
+        packet_buffer = PacketBuffer()
+        # write packet's id right off the bat in the header
+        VarInt.send(self.id, packet_buffer)
+        # write every individual field
+        for field in self.definition:
+            for var_name, data_type in field.items():
+                data = getattr(self, var_name)
+                data_type.send(data, packet_buffer)
+
+        self._write_buffer(socket, packet_buffer, compression_threshold)
+
     def __str__(self):
         str = type(self).__name__
         if self.id is not None:

tests/test_packets_with_logic.py

@@ -126,11 +126,11 @@ class PlayerListItemTest(unittest.TestCase):
         player_list = PlayerListItemPacket.PlayerList()
         by_uuid = player_list.players_by_uuid
 
-        packet_buffer = self.make_add_player_packet()
+        packet_buffer = self.make_add_player_packet(display_name=False)
         self.read_and_apply(packet_buffer, player_list)
         self.assertEqual(by_uuid[fake_uuid].gamemode, 42)
         self.assertEqual(by_uuid[fake_uuid].ping, 69)
-        self.assertEqual(by_uuid[fake_uuid].display_name, "display")
+        self.assertIsNone(by_uuid[fake_uuid].display_name)
 
         # Change the game mode
         packet_buffer = self.make_action_base(1)
@@ -144,12 +144,6 @@ class PlayerListItemTest(unittest.TestCase):
         self.read_and_apply(packet_buffer, player_list)
         self.assertEqual(by_uuid[fake_uuid].ping, 70)
 
-        # Remove the display name
-        packet_buffer = self.make_action_base(3)
-        Boolean.send(False, packet_buffer)
-        self.read_and_apply(packet_buffer, player_list)
-        self.assertIsNone(by_uuid[fake_uuid].display_name)
-
         # Change the display name
         packet_buffer = self.make_action_base(3)
         Boolean.send(True, packet_buffer)
@@ -157,6 +151,12 @@ class PlayerListItemTest(unittest.TestCase):
         self.read_and_apply(packet_buffer, player_list)
         self.assertEqual(by_uuid[fake_uuid].display_name, "display2")
 
+        # Remove the display name
+        packet_buffer = self.make_action_base(3)
+        Boolean.send(False, packet_buffer)
+        self.read_and_apply(packet_buffer, player_list)
+        self.assertIsNone(by_uuid[fake_uuid].display_name)
+
         # Remove the player
         packet_buffer = self.make_action_base(4)
         self.read_and_apply(packet_buffer, player_list)