Add Virtual Packet ID Method to Protocols
#1
I was working on the scoreboard, and noticed that in order to add a network packet I have to duplicate the code several times (protocol 1.8, 1.9, 1.12, and 1.12.1). In each case the only change is the packet ID.

So what if there were a virtual method, cProtocol::GetPacketID(ePacketId a_PacketId), which stored the network packet ID for a packet? Then, instead of:
Quote:cPacketizer Pkt(*this, 0x40);  // Update Health packet
you would do:
Quote:cPacketizer Pkt(*this, GetPacketID(packetHealthUpdate));  // Update Health packet

Then, there wouldn't have to be a whole bunch of copying/pasting code every time a new protocol comes out and Mojang changes every packet ID. We just copy/paste the GetPacketID method. It would ensure that nothing gets forgotten when a protocol change happens (look at the currently open PR #3962: https://github.com/cuberite/cuberite/pull/3962, which adds leash packets to the 1.12.1 protocol)

It'd also minimize the amount of code duplication. PR #3908 (https://github.com/cuberite/cuberite/pull/3908/) implemented 1.12.1 support, and added over 800 lines of code. By my count, out of 34 Send methods that were added, these were all just copied/pasted with the packet ID changed:
- SendRespawn
- SendPlayerListAddPlayer
- SendPlayerListRemovePlayer
- SendPlayerListUpdateGameMode
- SendPlayerListUpdatePing
- SendPlayerListUpdateDisplayName
- SendPlayerAbilities
- SendPlayerMoveLook
- SendUseBed
- SendDestroyEntity
- SendRemoveEntityEffect
- SendEntityHeadLook
- SendCameraSetTo
- SendDisplayObjective
- SendEntityMetadata
- SendEntityVelocity
- SendEntityEquipment
- SendExperience
- SendHealth
- SendScoreboardObjective
- SendAttachEntity
- SendDetachEntity
- SendScoreUpdate
- SendTimeUpdate
- SendSetRawTitle
- SendSetRawSubTitle
- SendTeleportEntity
- SendEntityProperties
- SendPlayerMaxSpeed
Or, 29 out of 34. That's just the upgrade from 1.12 to 1.12.1, overall we support 10 distinct protocols (the 1.8 to 1.9 transition is doesn't really apply here, though, since 1.9 doesn't inherit from 1.8).

The only downside I see is possibly performance - it's adding a virtual method call to every sent packet. But, that's basically free, compared to the labor cost of  maintaining so much duplicate code, and the cost of sending a network packet anyway.

Anyway, I thought I'd post this on the forum in case anyone has an opinion before I went off and made a PR, since it seems like the sort of change that would warrant some discussion (and presumably has been discussed before, but I couldn't find anything on it).
Reply
Thanks given by:


Messages In This Thread
Add Virtual Packet ID Method to Protocols - by lkolbly - 08-27-2017, 02:21 AM



Users browsing this thread: 1 Guest(s)