From c3f2cd15433ea2f29a8a36476a61e9f7f10cfb84 Mon Sep 17 00:00:00 2001 From: Kodi Craft Date: Wed, 9 Oct 2024 17:12:54 +0200 Subject: [PATCH] More progress on the refactoring --- apps/amethyst/.credo.exs | 6 +- apps/amethyst/lib/apps/connection_handler.ex | 236 ++++++++++++------ apps/amethyst/lib/apps/connection_receiver.ex | 34 ++- apps/amethyst/lib/states/behaviour.ex | 48 ++++ apps/amethyst/lib/states/configuration.ex | 1 + apps/amethyst/lib/states/handhsake.ex | 1 + apps/amethyst/lib/states/login.ex | 1 + apps/amethyst/lib/states/macros.ex | 2 + apps/amethyst/lib/states/play.ex | 1 + apps/amethyst/lib/states/status.ex | 1 + 10 files changed, 228 insertions(+), 103 deletions(-) create mode 100644 apps/amethyst/lib/states/behaviour.ex diff --git a/apps/amethyst/.credo.exs b/apps/amethyst/.credo.exs index 3335ccf..3dfe9b5 100644 --- a/apps/amethyst/.credo.exs +++ b/apps/amethyst/.credo.exs @@ -87,7 +87,6 @@ {Credo.Check.Readability.VariableNames, []}, {Credo.Check.Readability.WithSingleClause, []}, {Credo.Check.Readability.BlockPipe, []}, - {Credo.Check.Readability.SinglePipe, []}, # ## Refactoring Opportunities @@ -135,8 +134,8 @@ {Credo.Check.Warning.UnusedStringOperation, []}, {Credo.Check.Warning.UnusedTupleOperation, []}, {Credo.Check.Warning.WrongTestFileExtension, []} - ], - disabled: [ + ], + disabled: [ # # Checks scheduled for next check update (opt-in for now) {Credo.Check.Refactor.UtcNowTruncate, []}, @@ -157,6 +156,7 @@ {Credo.Check.Readability.OnePipePerLine, []}, {Credo.Check.Readability.SeparateAliasRequire, []}, {Credo.Check.Readability.SingleFunctionToBlockPipe, []}, + {Credo.Check.Readability.SinglePipe, []}, {Credo.Check.Readability.Specs, []}, {Credo.Check.Readability.StrictModuleLayout, []}, {Credo.Check.Readability.WithCustomTaggedTuple, []}, diff --git a/apps/amethyst/lib/apps/connection_handler.ex b/apps/amethyst/lib/apps/connection_handler.ex index e39d186..112c5ac 100644 --- a/apps/amethyst/lib/apps/connection_handler.ex +++ b/apps/amethyst/lib/apps/connection_handler.ex @@ -22,7 +22,10 @@ defmodule Amethyst.ConnectionHandler do alias Amethyst.Minecraft.Write require Logger - defstruct handler: self(), + @enforce_keys [:handler, :socket] + defstruct [ + :handler, + :socket, version: 0, connection_state: Amethyst.ConnectionState.Handshake, game: nil, @@ -32,6 +35,85 @@ defmodule Amethyst.ConnectionHandler do authenticated: false, position: {0, 0, 0}, player_state: %{} + ] + @type t :: %__MODULE__{ + handler: pid(), + socket: :gen_tcp.socket(), + version: 0 | 767, + connection_state: Amethyst.ConnectionState.t(), + game: nil | Amethyst.GameCoordinator.Game, + encryption_state: nil | :crypto.crypto_state(), + decryption_state: nil | :crypto.crypto_state(), + compression_threshold: nil | non_neg_integer(), + authenticated: boolean(), + position: {x :: number(), y :: number(), z :: number()}, + player_state: map() + } + + @type operation :: (__MODULE__.t() -> __MODULE__.t()) + defmodule Operations do + @moduledoc """ + This module includes generators for common operations + which the `ConnectionHandler` should perform. + """ + alias Amethyst.ConnectionHandler + + @spec put(key :: atom(), value :: any()) :: ConnectionHandler.operation() + def put(key, value) do + fn state -> + Map.put(state, key, value) + end + end + + @spec put_ps(key :: any(), value :: any()) :: ConnectionHandler.operation() + def put_ps(key, value) do + fn state -> + Map.put(state, :player_state, Map.put(state.player_state, key, value)) + end + end + + @spec send_packet(%{packet_type: atom()}) :: ConnectionHandler.operation() + def send_packet(packet) do + fn state -> + try do + data = Amethyst.ConnectionState.serialize(packet, state) + + container = if state.compression_threshold == nil do + # Packet ID is included in data + Write.varint(byte_size(data)) <> data + else + threshold = state.compression_threshold + data_length = byte_size(data) + if data_length >= threshold do + compressed = Write.varint(data_length) <> :zlib.compress(data) + Write.varint(byte_size(compressed)) <> compressed + else + compressed = Write.varint(0) <> data + Write.varint(byte_size(compressed)) <> compressed + end + end + + encrypted = if state.encryption_state == nil do + container + else + state.encryption_state |> :crypto.crypto_update(container) + end + :gen_tcp.send(state.socket, encrypted) + rescue + e -> + Logger.error("Error sending packet #{inspect(packet)}: #{Exception.format(:error, e, __STACKTRACE__)}") + end + end + end + + @spec disconnect(reason :: String.t()) :: ConnectionHandler.operation() + def disconnect(reason) do + fn state -> + func = Amethyst.ConnectionState.disconnect(state, reason) |> send_packet() + func.(state) + end + end + end @spec child_spec(:gen_tcp.socket()) :: Supervisor.child_spec() def child_spec(socket) do @@ -41,50 +123,97 @@ defmodule Amethyst.ConnectionHandler do } end - @spec start(:gen_tcp.socket(), atom(), integer()) :: no_return() + @spec start(:gen_tcp.socket()) :: no_return() def start(socket) do {:ok, spawn(fn -> Process.set_label("ConnectionHandler for #{inspect(socket)}") - loop(socket, %__MODULE__{handler: self()}) + loop(%__MODULE__{socket: socket, handler: self()}) end)} end - @spec start_link(:gen_tcp.socket(), atom(), integer()) :: no_return() + @spec start_link(:gen_tcp.socket()) :: no_return() def start_link(socket) do {:ok, spawn_link(fn -> Process.set_label("ConnectionHandler for #{inspect(socket)}") - loop(socket, %__MODULE__{handler: self()}) + loop(%__MODULE__{socket: socket, handler: self()}) end)} end @doc """ Informs the `handler` that a packet was received and that it should handle it. """ - @spec packet(handler :: pid(), id :: pos_integer(), data :: binary()) + @spec packet(handler :: pid(), id :: pos_integer(), data :: binary()) :: :ok def packet(handler, id, data) do send(handler, {:packet, id, data}) + :ok end - @spec close(handler :: pid()) + @doc """ + Informs the `handler` that its client's socket closed the connection and that it + should proceed with cleaning it up. + """ + @spec close(handler :: pid()) :: :ok def close(handler) do - + send(handler, :closed) + :ok end - @spec loop(:gen_tcp.socket(), %__MODULE__{}) :: no_return() - defp loop(socket, state) do + @doc """ + Runs `ops` on the handler's state. All operations in `ops` will be ran sequentially + and will not be interrupted by another operation. + """ + @spec run(handler :: pid(), ops :: [operation()]) :: :ok + def run(handler, ops) do + send(handler, {:run, ops}) + end + + @doc """ + Runs a function on the handler's state and returns the result. This cannot update the state, + this may only be used to get data. + """ + @spec get(handler :: pid(), f :: (__MODULE__.t() -> item)) :: item when item: any() + def get(handler, f) do + nonce = :rand.bytes(4) + send(handler, {:get, self(), f, nonce}) + receive do + {:respond, ret, ^nonce} -> ret + end + end + + @spec run_ops([operation()], __MODULE__.t()) :: {state :: __MODULE__.t(), successes :: [boolean()]} + defp run_ops(ops, state) do + Enum.reduce(Enum.with_index(ops), {state, []}, fn {op, i}, {state, sacc} -> + try do + {op.(state), [true | sacc]} + rescue + e -> + Logger.error("Operation error in op #{i}, continuing regardless: #{Exception.format(:error, e, __STACKTRACE__)}") + {state, [false | sacc]} + end + end) + end + + @spec loop(__MODULE__.t()) :: no_return() + defp loop(state) do receive do :closed -> # Socket is closed, handle our side of the disconnect Process.exit(self(), :normal) - {:packet, id, data} -> - state = Enum.reduce(handle_packet(id, data, state), state, fn op, state -> op.(state) end) - loop(socket, state) + {:packet, id, data} when is_integer(id) and id >= 0 and is_binary(data) -> + {state, successes} = run_ops(handle_packet(id, data, state), state) + if !Enum.all?(successes) do + Logger.error("Failed to run operations of packet #{inspect(id, base: :hex)}") + end + loop(state) {:run, ops} -> - state = Enum.reduce(ops, state, fn op, state -> op.(state) end) - loop(socket, state) - {:get, from, op} -> - send(from, op.(state)) - loop(socket, state) + {state, successes} = run_ops(ops, state) + if !Enum.all?(successes) do + Logger.error("Failed to run requested operations.") + end + loop(state) + {:get, from, op, nonce} -> + send(from, {:respond, op.(state), nonce}) + loop(state) end end @@ -92,7 +221,6 @@ defmodule Amethyst.ConnectionHandler do {floor(round(x) / 16.0), floor(round(z) / 16.0)} end - # x, z here is chunk position defp visible_chunks_from(x, z, view_distance) do (x - view_distance - 3 .. x + view_distance + 3) |> Enum.flat_map(fn ix -> (z - view_distance - 3 .. z + view_distance + 3) |> Enum.map(fn iz -> @@ -212,73 +340,21 @@ defmodule Amethyst.ConnectionHandler do end) end - @spec handle_packet(pos_integer(), binary(), %__MODULE__{}) :: [(%__MODULE__{} -> %__MODULE__{})] - defp handle_packet(id, data, connstate, version, state) do + @spec handle_packet(non_neg_integer(), binary(), __MODULE__.t()) :: [operation()] + defp handle_packet(id, data, state) when is_struct(state, __MODULE__) do try do - packet = connstate.deserialize(id, version, data) - case connstate.handle(packet, version, state) do - :ok -> state - {:error, reason} -> - Logger.error("Error handling packet with ID #{inspect(id, base: :hex)} in state #{connstate}: #{reason}") - send(self(), {:disconnect, "§cError handling packet #{inspect(id, base: :hex)}:\n#{reason}"}) - state - newstate -> - if is_map(newstate) do - newstate - else - Logger.warning("State change to #{inspect(newstate)} is not a map! Did you forget to return :ok?") - state - end - end + packet = Amethyst.ConnectionState.deserialize(state, id, data) + Amethyst.ConnectionState.handle(state, packet) rescue e -> + Logger.error("Error handling packet with ID #{inspect(id, base: :hex)}: #{Exception.format(:error, e, __STACKTRACE__)}") if Application.get_env(:amethyst, :release, false) do - send(self(), {:disconnect, "§cError handling packet #{inspect(id, base: :hex)}:\n#{Exception.format(:error, e, __STACKTRACE__)}"}) - Logger.error("Error handling packet with ID #{inspect(id, base: :hex)} in state #{connstate}: #{Exception.format(:error, e, __STACKTRACE__)}") + [Operations.disconnect( + "§cError handling packet #{inspect(id, base: :hex)}:\n#{Exception.format(:error, e, __STACKTRACE__)}" + )] else - Logger.error("Error handling packet with ID #{inspect(id, base: :hex)} in state #{connstate}: #{Exception.format(:error, e, __STACKTRACE__)}") + [] end - state end end - - defp send_packet(socket, connstate, packet, version, state) do - try do - data = connstate.serialize(packet, version) - container = if Map.get(state, :compression) == nil do - # Packet ID is included in data - Write.varint(byte_size(data)) <> data - else - threshold = Map.get(state, :compression, 0) - data_length = byte_size(data) - if data_length >= threshold do - compressed = Write.varint(data_length) <> :zlib.compress(data) - Write.varint(byte_size(compressed)) <> compressed - else - compressed = Write.varint(0) <> data - Write.varint(byte_size(compressed)) <> compressed - end - end - encrypted = if Map.get(state, :encryption_state) == nil do - container - else - Map.get(state, :encryption_state) |> :crypto.crypto_update(container) - end - :gen_tcp.send(socket, encrypted) - rescue - e -> - Logger.error("Error sending packet #{inspect(packet)} in state #{connstate}: #{Exception.format(:error, e, __STACKTRACE__)}") - send(self(), {:disconnect, "§cError sending packet #{inspect(packet)}:\n#{Exception.format(:error, e, __STACKTRACE__)}"}) - end - end - - defp disconnect(socket, reason, connstate, version, state) do - Logger.info("Disconnecting connection #{inspect(socket)}") - Logger.debug("Disconnecting connection #{inspect(socket)}: #{reason}") - case connstate.disconnect(reason) do - nil -> nil - packet -> send_packet(socket, connstate, packet, version, state) - end - :gen_tcp.close(socket) - end end diff --git a/apps/amethyst/lib/apps/connection_receiver.ex b/apps/amethyst/lib/apps/connection_receiver.ex index 3b8e033..a0f8ce5 100644 --- a/apps/amethyst/lib/apps/connection_receiver.ex +++ b/apps/amethyst/lib/apps/connection_receiver.ex @@ -34,7 +34,7 @@ defmodule Amethyst.ConnectionReceiver do def start(socket) do {:ok, spawn(fn -> Process.set_label("ConnectionReceiver for #{inspect(socket)}") - {:ok, pid} = Amethyst.ConnectionHandler.start_link(socket, Amethyst.ConnectionState.Handshake, 0) + {:ok, pid} = Amethyst.ConnectionHandler.start_link(socket) receive(socket, pid, nil, nil) end)} end @@ -43,39 +43,33 @@ defmodule Amethyst.ConnectionReceiver do def start_link(socket) do {:ok, spawn_link(fn -> Process.set_label("ConnectionReceiver for #{inspect(socket)}") - {:ok, pid} = Amethyst.ConnectionHandler.start_link(socket, Amethyst.ConnectionState.Handshake, 0) + {:ok, pid} = Amethyst.ConnectionHandler.start_link(socket) receive(socket, pid, nil, nil) end)} end @spec receive(:gen_tcp.socket(), pid(), nil | :crypto.crypto_state(), nil | pos_integer()) :: no_return() - def receive(socket, sender, estate, cstate) do + def receive(socket, handler, estate, cstate) do case get_packet(socket, estate, cstate) do - :closed -> send(sender, :closed) + :closed -> Amethyst.ConnectionHandler.close(handler) Process.exit(self(), :normal) {:error, error} -> Logger.error("Error reading packet: #{error}") - {id, data} -> send(sender, {:packet, id, data}) + {id, data} -> Amethyst.ConnectionHandler.packet(handler, id, data) end estate = if estate == nil do - # Ask the handler if we have encryption now - send(sender, {:get_encryption, self()}) - receive do - nil -> nil - some -> some - end - else estate end + Amethyst.ConnectionHandler.get(handler, fn s -> s.encryption_state end) + else + estate + end cstate = if cstate == nil do - # Ask the handler if we have encryption now - send(sender, {:get_compression, self()}) - receive do - nil -> nil - some -> some - end - else cstate end + Amethyst.ConnectionHandler.get(handler, fn s -> s.compression_threshold end) + else + cstate + end - receive(socket, sender, estate, cstate) + receive(socket, handler, estate, cstate) end def get_packet(client, estate, cstate) do diff --git a/apps/amethyst/lib/states/behaviour.ex b/apps/amethyst/lib/states/behaviour.ex new file mode 100644 index 0000000..0b548e9 --- /dev/null +++ b/apps/amethyst/lib/states/behaviour.ex @@ -0,0 +1,48 @@ +# Amethyst - An experimental Minecraft server written in Elixir. +# Copyright (C) 2024 KodiCraft +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +defmodule Amethyst.ConnectionState do + @moduledoc """ + This behaviour defines the behaviour that all connection states must implement. + """ + + alias Amethyst.ConnectionHandler + @type t :: module() + + @callback serialize(packet :: %{packet_type: atom()}, state :: ConnectionHandler.t()) :: binary() + @spec serialize(state :: ConnectionHandler.t(), packet :: %{packet_type: atom()}) :: binary() + def serialize(state, packet) do + state.connection_state.serialize(packet, state) + end + + @callback deserialize(id :: non_neg_integer(), data :: binary(), state :: ConnectionHandler.t()) :: %{packet_type: atom()} + @spec deserialize(state :: ConnectionHandler.t(), id :: pos_integer(), data :: binary()) :: %{packet_type: atom()} + def deserialize(state, id, data) do + state.connection_state.serialize(id, data, state) + end + + @callback handle(packet :: %{packet_type: atom()}, state :: ConnectionHandler.t()) :: [ConnectionHandler.operation()] + @spec handle(state :: ConnectionHandler.t(), packet :: %{packet_type: atom()}) :: [ConnectionHandler.operation()] + def handle(state, packet) do + state.connection_state.handle(packet) + end + + @callback disconnect(reason :: String.t(), state :: ConnectionHandler.t()) :: %{packet_type: atom()} + @spec disconnect(state :: ConnectionHandler.t(), reason :: String.t()) :: %{packet_type: atom()} + def disconnect(state, reason) do + state.connection_state.disconnect(reason) + end +end diff --git a/apps/amethyst/lib/states/configuration.ex b/apps/amethyst/lib/states/configuration.ex index 50462ff..6138308 100644 --- a/apps/amethyst/lib/states/configuration.ex +++ b/apps/amethyst/lib/states/configuration.ex @@ -15,6 +15,7 @@ # along with this program. If not, see . defmodule Amethyst.ConnectionState.Configuration do + @behaviour Amethyst.ConnectionState require Amethyst.ConnectionState.Macros alias Amethyst.ConnectionState.Macros diff --git a/apps/amethyst/lib/states/handhsake.ex b/apps/amethyst/lib/states/handhsake.ex index 1aa6f88..6e9c175 100644 --- a/apps/amethyst/lib/states/handhsake.ex +++ b/apps/amethyst/lib/states/handhsake.ex @@ -15,6 +15,7 @@ # along with this program. If not, see . defmodule Amethyst.ConnectionState.Handshake do + @behaviour Amethyst.ConnectionState require Amethyst.ConnectionState.Macros alias Amethyst.ConnectionState.Macros diff --git a/apps/amethyst/lib/states/login.ex b/apps/amethyst/lib/states/login.ex index 14f005b..4c306a8 100644 --- a/apps/amethyst/lib/states/login.ex +++ b/apps/amethyst/lib/states/login.ex @@ -15,6 +15,7 @@ # along with this program. If not, see . defmodule Amethyst.ConnectionState.Login do + @behaviour Amethyst.ConnectionState require Amethyst.ConnectionState.Macros alias Amethyst.ConnectionState.Macros diff --git a/apps/amethyst/lib/states/macros.ex b/apps/amethyst/lib/states/macros.ex index fdeb172..88554ac 100644 --- a/apps/amethyst/lib/states/macros.ex +++ b/apps/amethyst/lib/states/macros.ex @@ -23,6 +23,7 @@ defmodule Amethyst.ConnectionState.Macros do require Logger defmacro defpacket_serverbound(name, id, version, signature, where \\ true) do quote do + @impl true def deserialize(unquote(id), unquote(version), data) when unquote(where) do {packet, ""} = Amethyst.ConnectionState.Macros.read_signature(data, unquote(signature)) packet |> Map.put(:packet_type, unquote(name)) @@ -32,6 +33,7 @@ defmodule Amethyst.ConnectionState.Macros do defmacro defpacket_clientbound(name, id, version, signature, where \\ true) do quote do + @impl true def serialize(%{packet_type: unquote(name)} = packet, unquote(version)) when unquote(where) do # Don't check types if we are in release mode if Application.get_env(:amethyst, :release, false) || Amethyst.ConnectionState.Macros.check_type(packet, unquote(signature)) do diff --git a/apps/amethyst/lib/states/play.ex b/apps/amethyst/lib/states/play.ex index 92447b3..e11bbde 100644 --- a/apps/amethyst/lib/states/play.ex +++ b/apps/amethyst/lib/states/play.ex @@ -15,6 +15,7 @@ # along with this program. If not, see . defmodule Amethyst.ConnectionState.Play do + @behaviour Amethyst.ConnectionState require Amethyst.ConnectionState.Macros alias Amethyst.ConnectionState.Macros diff --git a/apps/amethyst/lib/states/status.ex b/apps/amethyst/lib/states/status.ex index 2b7d489..0f549a4 100644 --- a/apps/amethyst/lib/states/status.ex +++ b/apps/amethyst/lib/states/status.ex @@ -15,6 +15,7 @@ # along with this program. If not, see . defmodule Amethyst.ConnectionState.Status do + @behaviour Amethyst.ConnectionState require Amethyst.ConnectionState.Macros alias Amethyst.ConnectionState.Macros