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

Add traits that enable object like parsing/serializing of game query packets #148

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Douile
Copy link
Collaborator

@Douile Douile commented Oct 30, 2023

This is a draft implementation of what #104 could look like.

It adds 3 traits:

  • ToPacket - to encode a struct as a Vec<u8> for sending over the network
  • FromPacket - to create a struct from &[u8]
  • ExtendFromPacket - to extend the data in a struct from &[u8], this is useful when responses come in multiple packets e.g. player responses

To help with implementing the ToPacket trait WriteBuffer was also added, this functions very similarly to Buffer but is instead for writing data into a buffer.


These traits can be used by the query implementations to create packets:

In unreal 2 the old send request was implemented as

let request = [0x79, 0, 0, 0, packet_type as u8];
self.socket.send(&request)?;

this becomes

let request = PacketRequest::from(packet_type);
self.socket.send(&request.as_packet()?)?;

where PacketRequest is an exact representation of the raw packet bytes:

/// Unreal2 client request.
#[derive(Debug, Clone, PartialEq)]
pub struct PacketRequest {
    /// Should always be 0x79.
    pub request_flag: u8,
    pub padding: [u8; 3],
    pub packet_type: PacketKind,
}

impl From<PacketKind> for PacketRequest {
    fn from(packet_type: PacketKind) -> Self {
        Self {
            request_flag: REQUEST_FLAG,
            padding: [0, 0, 0],
            packet_type,
        }
    }
}

They can also be used to parse packets:

    pub fn query_server_info(&mut self) -> GDResult<ServerInfo> {
        let data = self.get_request_data(PacketKind::ServerInfo)?;

        let packet = PacketServerInfo::from_packet(&data)?;

        // TODO: Remove debug logging
        println!("{:?}", packet);

        Ok(packet.body)
    }

For the unreal2 implementation the packets all have a common outer header part, using these traits the can be represented using an outer ResponsePacket type that is generic over the inner body. I think this is quite a nice abstraction as it allows for a blanket implementation for encoding/decoding all packets where the inner body implements the ToPacket or FromPacket traits. Although there is a bit of added complexity.

Here the Packet structs are also raw representation of the bytes.

/// Unreal 2 server response.
#[derive(Debug, Clone, PartialEq)]
pub struct PacketResponse<T> {
    pub padding: [u8; 4],
    pub packet_type: PacketKind,
    pub body: T,
}

// ...

pub type PacketServerInfo = PacketResponse<ServerInfo>;

Positives of moving to this architecture:

  • Parsing packets becomes more standardized, we might be able to move to a more generic protocol query implementation where we just implement how to parse/encode each packet and the order of packets.
  • Consumers of the crate can use the encode/decode implementations to do things other than query servers for example the fake unreal2server example.
  • More data oriented packet parsing can look nicer and reduce repetition (removed the need for consume_packet_header in unreal2).

Downsides:

  • These traits don't allow for passing state into the parsing functions so for protocols where we want to slightly vary parsing based on pre-determined variables (like valve) it might cause problems.
  • Encoding packets in this way is less efficient than what we had before: creating an array in place is much more efficient than allocating a vector then pushing each part of the request to it individually.
  • Added overall code complexity, might put people off implementing new protocols/working on existing ones.
    • Some of the generics especially get pretty ugly:
impl<'a, T> ExtendFromPacket<'a> for PacketResponse<&'a mut T>
where
    for<'b> T: ExtendFromPacket<'b, Input = T>,
{
    type Input = T;
    type Output = PacketResponse<&'a mut T>;
    fn extend_from_packet(packet: &[u8], input: &'a mut Self::Input) -> GDResult<Self::Output> {
  // ...
  }
}

My thoughts:

  • The ExtendFromPacket trait seems unnecessarily complex, but simplifying it needs more thought.
  • This doesn't necessarily need to be in gamedig: gamedig doesn't need to be able to decode request packets, or encode response packets. It could be a separate crate and possibly just import the parts we need for gamedig.
  • Some of the request structs could quite literally be bit-for-bit representations of the packet (if they used #[repr(C)]) in these cases it could be a lot faster to just cast the struct to a slice of bytes rather than using a WriteBuffer.
  • ToPacket could take WriteBuffer as a parameter directly to avoid allocating a Vec.

I'll probably think of more benefits/downsides in the future but I think this is in a place where its ready for other people to give their thoughts on.

@github-actions github-actions bot added the protocol This is something regarding a protocol label Oct 30, 2023
@Douile Douile force-pushed the feat/encode-traits branch from 9d2fabb to 20a30bb Compare October 30, 2023 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol This is something regarding a protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant