amber tangle Feb 5, 2024, 9:05 PM

#

Hey. I'm writing a server for a popular game (bonus points if you can tell which from the code snippet) and I've come to find the way I'm structuring my packets.. unweildy. Let's take a look at a server-bound packet as an example:

defmodule Erebus.Packet.ServerBound.PingRequest do
  defstruct payload: nil

  def parse(data) do
    <<payload::size(64), rest::binary>> = data

    packet = %__MODULE__{
      payload: payload
    }

    {packet, rest}
  end
end
```and a client-bound packet:```exs
defmodule Erebus.Packet.ClientBound.PingResponse do
  defstruct payload: nil

  def encode(packet) do
    body = <<packet.payload::integer-size(64)>>

    {1, body}
  end
end

In order to parse a packet coming in from the server, we first:

Decrypt the incoming packet stream (irrelevant to this question)
Determine the size of the packet
Determine the ID of the packet
Send the remaining data to a big parse function, which takes the packet id, connection state, and data:

  def parse_packet(data, state, bound_to \\ :server) do
    {:ok, {packet_id, packet_data}, rest} = Erebus.Protocol.parse(data)

    {pack, unparsed_data} = case {packet_id, state, bound_to} do
      {0, :handshaking, :server} ->
        Erebus.Packet.ServerBound.Handshake.parse(packet_data)

      {0, :status, :server} ->
        Erebus.Packet.ServerBound.StatusRequest.parse(packet_data)

      # ...
  end

  if unparsed_data != "" do
    Logger.warning("Unparsed trailing data on packet: #{unparsed_data}")
  end

  {pack, rest}
end

#

This isn't too bad, but there might be a better way to do it. Anyway, my real question is how I can clean up the sending side of the solution. Once a packet is received, Erebus.Packet.Handler gets called with it: ```exs
defmodule Erebus.Packet.Handler do
alias Erebus.Packet.ServerBound
alias Erebus.Packet.ClientBound
alias Erebus.Connection

require Logger

def handle(%ServerBound.PingRequest{} = packet, conn) do
{:ok, ClientBound.PingResponse.encode(
%ClientBound.PingResponse{ payload: packet.payload }
), conn}
end

...

Some packets do additional work, like `ServerBound.Handshake`:exs
def handle(%ServerBound.Handshake{protocol_version: 765} = pack, conn) do
conn =
conn
|> Connection.put_state(pack.next_state)

{:ok, :noreply, conn}
end```

My question is, how can I not make this so... messy? If I want to add a new serverbound packet, I have to add it in both handler and packet (the big case statement), and when I want to send a packet I have to call encode on it, repeat the client name, and duplicate a lot of the encoding logic (see PingResponse.encode) in multiple packet files. All suggestions welcome.

sullen forge Feb 5, 2024, 9:08 PM

#

I'm not familiar enough with them to be certain, and not quite in a position to double check right now, but I think Protocols may be able to help you here

amber tangle Feb 5, 2024, 9:10 PM

#

Maybe.

#

Just to show you what the parse looks like on another packet: ```exs
defmodule Erebus.Packet.ServerBound.PluginMessage do
defstruct channel: nil,
data: nil

def parse(data) do
{channel, rest} = Erebus.Protocol.parse_string(data)
{data, rest} = Erebus.Protocol.parse_string(rest)

packet = %__MODULE__{
  channel: channel,
  data: data
}

{packet, rest}

end
end

#

you can see there's a lot of dupe code

#

between packets

#

prob could use a macro

distant blade Feb 5, 2024, 11:27 PM

#

Do you control this protocol? If you could pull the packet size out of the encrypted data, gen_tcp has some tricks for you.

#

Having the packet size encrypted is a bummer. It makes it impossible to efficiently read

amber tangle Feb 5, 2024, 11:30 PM

#

I'm afraid I don't control the protocol

#

Although, reading the packet size isn't too much of an issue

#

it's a stream cipher with block length 8

#

somewhat relevant snippet:```exs
defp serve(client), do: serve(client, Erebus.Connection.init(client))
defp serve(client, conn) do
conn = if Erebus.Connection.has_complete_packet_queued?(conn) do
conn
|> process_packet()
else
client
|> read_packet(conn)
end

serve(client, conn)
end

defp read_packet(client, conn) do
case :gen_tcp.recv(client, 0, 5000) do
{:ok, data} ->
conn
|> Erebus.Connection.decrypt_if_enabled(data)
|> Erebus.Connection.put_data(data)
{:error, :closed} ->
Logger.debug("Client disconnect")
:gen_tcp.close(conn.socket)
exit(:normal)
{:error, :timeout} ->
Logger.info("Client died")
Erebus.Connection.kick_player(conn, "Timeout")
exit(:normal)
end
end```

distant blade Feb 5, 2024, 11:43 PM

#

It's just that if the protocol was framed, gen_tcp would read it for you

#

Also, style note, don't use pipelines for single calls.

#

i.e. prefer process_packet(conn) over conn |> process_packet()

#

But because of the packet structure, there are a bunch of optimizations you're going to miss.

amber tangle Feb 5, 2024, 11:52 PM

#

yeah, its unfortunate

#

but

#

nothing I can do :/

#

other than clean the code up

distant blade Feb 6, 2024, 1:00 AM

#

amber tangle other than clean the code up

One thing you can try is to see if you can apply the bin_opt_info optimization
https://www.erlang.org/doc/efficiency_guide/binaryhandling#matching-binaries

#

Is the magic number 5000 meaningful above? If not, replace it with 0

#

Or use active mode instead.

amber tangle Feb 6, 2024, 1:04 AM

#

distant blade Is the magic number 5000 meaningful above? If not, replace it with 0

just a timeout

distant blade Feb 6, 2024, 1:04 AM

#

Oh sorry, I misread that as the length argument

amber tangle Feb 6, 2024, 1:05 AM

#

nw

amber tangle Feb 6, 2024, 4:42 PM

#

I have beat the packet parsing into relative submission: ```exs
defmodule Erebus.Packet do
require Logger
import CodeGen.Gen

defpacket ClientBound.PluginMessage, 0x18, :play, :client,
[
channel: Erebus.Protocol.String,
data: Erebus.Protocol.String
]

defpacket ServerBound.PluginMessage, 0x01, :configuration, :server,
[
channel: Erebus.Protocol.String,
data: Erebus.Protocol.String
]

amber tangle Feb 7, 2024, 5:12 PM

#

someone please provide guidance on how i can do this but not utterly terrible

#

📎 yuck.ex

distant blade Feb 7, 2024, 5:13 PM

#

this is the problem with macros, they're tempting but it's a vortex of complexity

distant blade Feb 7, 2024, 5:14 PM

#

amber tangle someone please provide guidance on how i can do this but not utterly terrible

maybe it would help if you'd talk about what you want to achieve first?

amber tangle Feb 7, 2024, 5:16 PM

#

right yeah

#

the end goal is to have some kind of file like packet/client_bound/play.ex which contains something like ```exs
defmodule Erebus.ClientModeWhatever do
import CodeGen.Gen

packetgroup ClientBound.Play, :play, :client do
packet PluginMessage, 0x18,
[
channel: Erebus.Protocol.String,
data: Erebus.Protocol.String
]

packet SomeOtherPacket, 0x19,
[
  channel: Erebus.Protocol.String,
  data: Erebus.Protocol.String
]

end
end

#

and then be able to pull all the packet/client_bound/play.ex or packet/client_bound/configuration.ex into one file so the function which the packet macro generates can be used to match:

#

defmodule ClientBound.Play.PluginMessage do
  @type t :: %__MODULE__{
          channel: String.t(),
          data: String.t()
        }
  defstruct channel: Elixir.Erebus.Protocol.String, data: Elixir.Erebus.Protocol.String

  def parse(packet_data__) do
    {channel, packet_data__} = Elixir.Erebus.Protocol.String.parse(packet_data__)
    {data, packet_data__} = Elixir.Erebus.Protocol.String.parse(packet_data__)
    {%__MODULE__{channel: channel, data: data}, packet_data__}
  end

  def encode(packet) do
    {24,
     <<Elixir.Erebus.Protocol.String.encode(packet.channel)::binary,
       Elixir.Erebus.Protocol.String.encode(packet.data)::binary>>}
  end
end

def parse_packet_match({24, :play, :client}, packet_data__) do
  PluginMessage.parse(packet_data__)
end

# more packets would go here```

distant blade Feb 7, 2024, 6:04 PM

#

that's going to be super tricky

#

do you need the packet_group thing?

#

can you just define things via convention?

#

like, define nested modules

defmodule ClinetBound.Play.PluginMessage do 
  ...
end

defmodule ClientBound.Play.SomeOtherPacket do 
 ...
end

#

you're likely going to have to place all the generated modules somewhere gloabally during compilation, and then select them and create parsers for the groups

#

also, isn't this:

#

<<Elixir.Erebus.Protocol.String.encode(packet.channel)::binary,
       Elixir.Erebus.Protocol.String.encode(packet.data)::binary>>

just this:

Elixir.Erebus.Protocol.String.encode(packet.channel) <> Elixir.Erebus.Protocol.String.encode(packet.data)

#

so you can use something like :persistent_term to store groups

amber tangle Feb 8, 2024, 10:51 AM

#

distant blade do you need the packet_group thing?

id prefer to have them in separate files

#

theres a lot of packets

#

the packet groups are used for the pattern matching

distant blade Feb 8, 2024, 3:59 PM

#

amber tangle id prefer to have them in separate files

you can have them in separate files, files and modules have no relation to one another

distant blade Feb 8, 2024, 4:00 PM

#

amber tangle the packet groups are used for the pattern matching

can you show me?

amber tangle Feb 8, 2024, 4:03 PM

#

def parse_packet_match({24, :play, :client}, packet_data__) do
  PluginMessage.parse(packet_data__)
end```

distant blade Feb 8, 2024, 4:05 PM

#

PluginMessage is a packet group?

distant blade Feb 8, 2024, 4:36 PM

#

there are a couple of approaches that I can imagine. One is to explicitly define a packet group like this:

defmodule ClientBound.Play do 
  alias ClinetBound.Play
  packet_group [Play.PluginMessage, Play.SomeOtherPacket]
end

#

another would be to accumulate the packet groups globally when you define the packets

defmodule ClientBound.Play.PluginMessage do 
  alias Erebus.Protocol
  import CodeGen.Gen
  packet 0x18, group: ClientBound.Play, channel: Protocol.String, data: Protocol.String
end

# client_bound/play.ex
defmodule ClientBound.Play do 
  use CodeGen.PacketGroup
end

#

the packet macro would then write to :persistent_term

#

alias CodeGen.PacketGroup 

defmacro packet(id, opts) do 
  group = opts |> Keyword.fetch!(:group) |> PacketGroup.add_to_group(__CALLER__)
  ...
end

# packet_group.ex
defmodule CodeGen.PacketGroup do
  defmacro __using__ do 
    packet_modules = packets_for(__CALLER__)
    ...
  end

  def add_to_group(group_module, packet_module) do 
    new_groups =
      case :persistent_term.get({:packet_groups, group_module}, :not_found) do 
       :not_found ->
         [packet_module]
       modules -> 
         [packet_module | modules]
      end
   :persistent_term.put(group_module, new_groups)
  end

  defp packets_for(module) do 
    :persistent_term.get(module, [])
  end
end

#

...and the __using__ function in the packet group module would read from persistent_term and write out the parsing code

#Code Refactor: Packet parse / Encode loop

...