Network stability pass (#36)

* Fix and cleanup on the counter aspects of client

* add process() options, add FGE_ENABLE_PACKET_DEBUG_VERBOSE, fixing

* WIP: change PacketCache to pause transmission when packet can't be sent
- Not an circular buffer anymore
- Add a try count to packets inside the cache
- Currently there is now a problem with the client reorder WIP
- Client: add isReadyToAcceptMorePendingPackets() it should now be used instead of isPendingPacketsEmpty()

* refactoring the PacketReorderer, fix network issues, remove duplicate client context
- Also cleanup debug printing
- WIP There is still a issue where not acknowledged packet is not correctly re-sent in certain situation

* Add FGE_SOCKET_MAX_MTU of 16384 because we can't go further with DTLS (openssl) for now
- This fix the transmission for big packets
- Also add more debug printing

* made Packet class copy/move assignable

* remove ERR_NONE as this class is meant to be used with std::optional

Author: JonathSpirit

CMakeLists.txt

@@ -101,6 +101,7 @@ option(FGE_BUILD_TESTS "Build tests" ON)
 
 option(FGE_ENABLE_SERVER_NETWORK_RANDOM_LOST "enable/disable random packet lost for server" OFF)
 option(FGE_ENABLE_CLIENT_NETWORK_RANDOM_LOST "enable/disable random packet lost for client" OFF)
+option(FGE_ENABLE_PACKET_DEBUG_VERBOSE "enable/disable verbose packet debug printing" OFF)
 
 #Check if Doxygen is installed
 if (FGE_BUILD_DOC)
@@ -184,6 +185,14 @@ if (${FGE_ENABLE_CLIENT_NETWORK_RANDOM_LOST})
     endif()
 endif()
 
+if (${FGE_ENABLE_PACKET_DEBUG_VERBOSE})
+    target_compile_definitions(${FGE_LIB_NAME} PUBLIC FGE_ENABLE_PACKET_DEBUG_VERBOSE)
+    target_compile_definitions(${FGE_SERVER_LIB_NAME} PUBLIC FGE_ENABLE_PACKET_DEBUG_VERBOSE)
+    if (NOT ${FGE_DEBUG})
+        message(WARNING "FGE_ENABLE_PACKET_DEBUG_VERBOSE is enabled, but FGE_DEBUG is disabled")
+    endif()
+endif()
+
 target_compile_definitions(${FGE_SERVER_LIB_NAME} PUBLIC FGE_DEF_SERVER)
 
 #Includes path

examples/clientServerLifeSimulator_004/client/main.cpp

@@ -275,7 +275,7 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
                 std::cout << "connection ok" << std::endl;
 
                 server.enableReturnPacket(true);
-                server.getClientContext()._reorderer.setMaximumSize(FGE_NET_PACKET_REORDERER_CACHE_COMPUTE(
+                server._client._context._reorderer.setMaximumSize(FGE_NET_PACKET_REORDERER_CACHE_COMPUTE(
                         FGE_NET_DEFAULT_RETURN_PACKET_RATE.count(), LIFESIM_SERVER_TICK));
 
                 auto transmissionPacket = fge::net::CreatePacket(ls::LS_PROTOCOL_C_PLEASE_CONNECT_ME);
@@ -369,9 +369,15 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
                                   << '\n'
                                   << "Update count: " << mainScene->getUpdateCount() << '\n'
                                   << "Lost packets: " << server._client.getLostPacketCount() << '\n'
-                                  << "Realm: " << static_cast<unsigned int>(server._client.getCurrentRealm())
-                                  << ", CurrentCounter: " << server._client.getCurrentPacketCounter()
-                                  << ", ClientCounter: " << server._client.getClientPacketCounter();
+                                  << "Realm: " << static_cast<unsigned int>(server._client.getCurrentRealm()) << '\n'
+                                  << "Peer counter: "
+                                  << server._client.getPacketCounter(fge::net::Client::Targets::PEER)
+                                  << ", Peer reorderedCounter: "
+                                  << server._client.getReorderedPacketCounter(fge::net::Client::Targets::PEER) << '\n'
+                                  << "Host counter: "
+                                  << server._client.getPacketCounter(fge::net::Client::Targets::HOST)
+                                  << ", Host reorderedCounter: "
+                                  << server._client.getReorderedPacketCounter(fge::net::Client::Targets::HOST);
 
                 latencyText->setString(tiny_utf8::string(latencyTextStream.str()));
 

examples/clientServerLifeSimulator_004/server/main.cpp

@@ -343,7 +343,7 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
                 }
 
                 //Make sure that the client is not busy with another packet
-                if (!it->second->isPendingPacketsEmpty())
+                if (!it->second->isReadyToAcceptMorePendingPackets())
                 {
                     continue;
                 }

includes/FastEngine/manager/network_manager.hpp

@@ -74,7 +74,7 @@ class ChainedArguments
 {
 public:
     constexpr ChainedArguments(Packet const& pck, TValue* existingValue = nullptr);
-    constexpr ChainedArguments(Packet const& pck, Error&& err, TValue* existingValue = nullptr);
+    constexpr ChainedArguments(Packet const& pck, std::optional<Error>&& err, TValue* existingValue = nullptr);
     constexpr ChainedArguments(ChainedArguments const& r) = default;
     constexpr ChainedArguments(ChainedArguments&& r) noexcept = default;
 
@@ -245,7 +245,7 @@ class ChainedArguments
 
     Packet const* g_pck;
     Value g_value;
-    Error g_error;
+    std::optional<Error> g_error;
 };
 
 /**

includes/FastEngine/manager/network_manager.inl

@@ -23,12 +23,14 @@ template<class TValue>
 constexpr ChainedArguments<TValue>::ChainedArguments(Packet const& pck, TValue* existingValue) :
         g_pck(&pck),
         g_value(existingValue == nullptr ? Value{std::in_place_type<TValue>} : Value{existingValue}),
-        g_error(pck.isValid() ? Error{}
-                              : Error{Error::Types::ERR_ALREADY_INVALID, pck.getReadPos(), "already invalid packet",
-                                      __func__})
+        g_error(pck.isValid() ? std::nullopt
+                              : std::optional{Error{Error::Types::ERR_ALREADY_INVALID, pck.getReadPos(),
+                                                    "already invalid packet", __func__}})
 {}
 template<class TValue>
-constexpr ChainedArguments<TValue>::ChainedArguments(Packet const& pck, Error&& err, TValue* existingValue) :
+constexpr ChainedArguments<TValue>::ChainedArguments(Packet const& pck,
+                                                     std::optional<Error>&& err,
+                                                     TValue* existingValue) :
         g_pck(&pck),
         g_value(existingValue == nullptr ? Value{std::in_place_type<TValue>} : Value{existingValue}),
         g_error(std::move(err))

includes/FastEngine/network/C_client.hpp

@@ -240,6 +240,13 @@ class FGE_API Client
 public:
     Client();
     ~Client();
+
+    enum class Targets
+    {
+        PEER,
+        HOST
+    };
+
     /**
      * \brief Constructor with default latencies
      *
@@ -372,6 +379,7 @@ class FGE_API Client
      */
     void pushPacket(TransmitPacketPtr pck);
     void pushForcedFrontPacket(TransmitPacketPtr pck);
+    [[nodiscard]] bool isReadyToAcceptMorePendingPackets() const;
     /**
      * \brief Pop a packet from the queue
      *
@@ -384,6 +392,7 @@ class FGE_API Client
      * \return True if the queue is empty, false otherwise
      */
     bool isPendingPacketsEmpty() const;
+    void allowMorePendingPackets(bool allow);
 
     void disconnect(bool pushDisconnectPacket = true);
 
@@ -392,15 +401,15 @@ class FGE_API Client
     void setCurrentRealm(ProtocolPacket::RealmType realm);
     ProtocolPacket::RealmType advanceCurrentRealm();
 
-    [[nodiscard]] ProtocolPacket::CounterType getCurrentPacketCounter() const;
-    ProtocolPacket::CounterType advanceCurrentPacketCounter();
-    void setCurrentPacketCounter(ProtocolPacket::CounterType counter);
+    [[nodiscard]] ProtocolPacket::CounterType getPacketCounter(Targets target) const;
+    ProtocolPacket::CounterType advancePacketCounter(Targets target);
+    void setPacketCounter(Targets target, ProtocolPacket::CounterType count);
 
-    [[nodiscard]] ProtocolPacket::CounterType getClientPacketCounter() const;
-    ProtocolPacket::CounterType advanceClientPacketCounter();
-    void setClientPacketCounter(ProtocolPacket::CounterType counter);
-    void resetLastReorderedPacketCounter();
-    [[nodiscard]] ProtocolPacket::CounterType getLastReorderedPacketCounter() const;
+    [[nodiscard]] ProtocolPacket::CounterType getReorderedPacketCounter(Targets target) const;
+    ProtocolPacket::CounterType advanceReorderedPacketCounter(Targets target);
+    void setReorderedPacketCounter(Targets target, ProtocolPacket::CounterType count);
+
+    void resetAllCounters();
 
     void acknowledgeReception(ReceivedPacketPtr const& packet);
     [[nodiscard]] std::unordered_set<PacketCache::Label, PacketCache::Label::Hash> const& getAcknowledgedList() const;
@@ -443,9 +452,11 @@ class FGE_API Client
 
     std::chrono::steady_clock::time_point g_lastRealmChangeTimePoint;
     ProtocolPacket::RealmType g_currentRealm{FGE_NET_DEFAULT_REALM};
-    ProtocolPacket::CounterType g_currentPacketCounter{0};
-    ProtocolPacket::CounterType g_lastReorderedPacketCounter{0};
-    ProtocolPacket::CounterType g_clientPacketCounter{0};
+
+    ProtocolPacket::CounterType g_hostPacketCounter{0};
+    ProtocolPacket::CounterType g_hostReorderedPacketCounter{0};
+    ProtocolPacket::CounterType g_peerPacketCounter{0};
+    ProtocolPacket::CounterType g_peerReorderedPacketCounter{0};
 
     std::unordered_set<PacketCache::Label, PacketCache::Label::Hash> g_acknowledgedPackets;
     uint32_t g_lostPacketCount{0};
@@ -457,6 +468,8 @@ class FGE_API Client
 
     ClientStatus g_status;
     CryptInfo g_cryptInfo;
+
+    bool g_allowMorePackets{true};
 };
 
 /**

includes/FastEngine/network/C_error.hpp

@@ -28,7 +28,6 @@ struct FGE_API Error
 {
     enum class Types
     {
-        ERR_NONE,
         ERR_ALREADY_INVALID,
         ERR_EXTRACT,
         ERR_RULE,
@@ -38,7 +37,22 @@ struct FGE_API Error
         ERR_SCENE_OLD_PACKET
     };
 
-    Types _type{Types::ERR_NONE};
+    constexpr Error(Types type) :
+            _type(type)
+    {}
+    constexpr Error(Types type, std::size_t readPos, char const* error, char const* function) :
+            _type(type),
+            _readPos(readPos),
+            _error(error),
+            _function(function)
+    {}
+    constexpr Error(Types type, char const* error, char const* function) :
+            _type(type),
+            _error(error),
+            _function(function)
+    {}
+
+    Types _type;
     std::size_t _readPos{0};
     char const* _error{nullptr};
     char const* _function{nullptr};

includes/FastEngine/network/C_packet.hpp

@@ -53,11 +53,13 @@ class FGE_API Packet
 public:
     Packet();
     Packet(Packet&& pck) noexcept;
-    Packet(Packet& pck) = default;
     Packet(Packet const& pck) = default;
     explicit Packet(std::size_t reserveSize);
     virtual ~Packet() = default;
 
+    Packet& operator=(Packet const& pck) = default;
+    Packet& operator=(Packet&& pck) noexcept;
+
     void clear();
     void flush();
     void reserve(std::size_t reserveSize);

includes/FastEngine/network/C_protocol.hpp

@@ -42,14 +42,17 @@
 
 #define FGE_NET_BAD_ID 0
 
-#define FGE_NET_PACKET_CACHE_DELAY_FACTOR 1.2f
+#define FGE_NET_PACKET_CACHE_DELAY_FACTOR 2.2f
 #define FGE_NET_PACKET_CACHE_MAX 100
 #define FGE_NET_PACKET_CACHE_MIN_LATENCY_MS 10
 
 #define FGE_NET_DEFAULT_REALM 0
 #define FGE_NET_DEFAULT_PACKET_REORDERER_CACHE_SIZE 5
 #define FGE_NET_PACKET_REORDERER_CACHE_COMPUTE(_clientReturnRate, _serverTickRate)                                     \
-    (((_clientReturnRate) * FGE_NET_PACKET_CACHE_DELAY_FACTOR / (_serverTickRate) + 1) * 2)
+    ((static_cast<unsigned>(static_cast<float>(_clientReturnRate) * FGE_NET_PACKET_CACHE_DELAY_FACTOR /                \
+                            static_cast<float>(_serverTickRate)) +                                                     \
+      1) *                                                                                                             \
+     2)
 
 #define FGE_NET_HANDSHAKE_STRING "FGE:HANDSHAKE:AZCgMVg4d4Sl2xYvZcqXqljIOqSrKX6H"
 
@@ -62,7 +65,7 @@ class Compressor;
 
 namespace fge::net
 {
-
+class NetFluxUdp;
 class Client;
 
 using Timestamp = uint16_t;
@@ -122,7 +125,7 @@ class FGE_API ProtocolPacket : public Packet, public std::enable_shared_from_thi
     constexpr static std::size_t IdPosition = 0;
     constexpr static std::size_t RealmPosition = sizeof(IdType);
     constexpr static std::size_t CounterPosition = sizeof(IdType) + sizeof(RealmType);
-    constexpr static std::size_t LastCounterPosition = sizeof(IdType) + sizeof(RealmType) + sizeof(CounterType);
+    constexpr static std::size_t ReorderedCounterPosition = sizeof(IdType) + sizeof(RealmType) + sizeof(CounterType);
 
     inline ProtocolPacket(Packet const& pck,
                           Identity const& id,
@@ -152,7 +155,7 @@ class FGE_API ProtocolPacket : public Packet, public std::enable_shared_from_thi
     [[nodiscard]] inline std::optional<IdType> retrieveFullHeaderId() const;
     [[nodiscard]] inline std::optional<RealmType> retrieveRealm() const;
     [[nodiscard]] inline std::optional<CounterType> retrieveCounter() const;
-    [[nodiscard]] inline std::optional<CounterType> retrieveLastCounter() const;
+    [[nodiscard]] inline std::optional<CounterType> retrieveReorderedCounter() const;
     [[nodiscard]] inline std::optional<Header> retrieveHeader() const;
 
     [[nodiscard]] inline bool isFragmented() const;
@@ -170,7 +173,7 @@ class FGE_API ProtocolPacket : public Packet, public std::enable_shared_from_thi
 
     inline ProtocolPacket& setRealm(RealmType realm);
     inline ProtocolPacket& setCounter(CounterType counter);
-    inline ProtocolPacket& setLastReorderedPacketCounter(CounterType counter);
+    inline ProtocolPacket& setReorderedCounter(CounterType counter);
 
     inline void setTimestamp(Timestamp timestamp);
     [[nodiscard]] inline Timestamp getTimeStamp() const;
@@ -356,16 +359,26 @@ class FGE_API PacketReorderer
 
     void clear();
 
+    bool process(Client& client, NetFluxUdp& flux, bool ignoreRealm);
+
     void push(ReceivedPacketPtr&& packet);
+
+    [[nodiscard]] static Stats checkStat(ReceivedPacketPtr const& packet, Client const& client, bool ignoreRealm);
     [[nodiscard]] static Stats checkStat(ReceivedPacketPtr const& packet,
-                                         ProtocolPacket::CounterType currentCounter,
-                                         ProtocolPacket::RealmType currentRealm);
-    [[nodiscard]] bool isForced() const;
-    [[nodiscard]] std::optional<Stats> checkStat(ProtocolPacket::CounterType currentCounter,
-                                                 ProtocolPacket::RealmType currentRealm) const;
+                                         ProtocolPacket::CounterType peerCounter,
+                                         ProtocolPacket::CounterType peerReorderedCounter,
+                                         ProtocolPacket::RealmType peerRealm,
+                                         bool ignoreRealm);
+    [[nodiscard]] std::optional<Stats> checkStat(Client const& client, bool ignoreRealm) const;
+    [[nodiscard]] std::optional<Stats> checkStat(ProtocolPacket::CounterType peerCounter,
+                                                 ProtocolPacket::CounterType peerReorderedCounter,
+                                                 ProtocolPacket::RealmType peerRealm,
+                                                 bool ignoreRealm) const;
+
     [[nodiscard]] ReceivedPacketPtr pop();
 
     [[nodiscard]] bool isEmpty() const;
+    [[nodiscard]] bool isForced() const;
 
     void setMaximumSize(std::size_t size);
     [[nodiscard]] std::size_t getMaximumSize() const;
@@ -381,12 +394,14 @@ class FGE_API PacketReorderer
         Data& operator=(Data const& r) = delete;
         Data& operator=(Data&& r) noexcept;
 
-        [[nodiscard]] Stats checkStat(ProtocolPacket::CounterType currentCounter,
-                                      ProtocolPacket::RealmType currentRealm) const;
+        [[nodiscard]] Stats checkStat(ProtocolPacket::CounterType peerCounter,
+                                      ProtocolPacket::CounterType peerReorderedCounter,
+                                      ProtocolPacket::RealmType peerRealm,
+                                      bool ignoreRealm) const;
 
         ReceivedPacketPtr _packet;
         ProtocolPacket::CounterType _counter;
-        ProtocolPacket::CounterType _lastCounter;
+        ProtocolPacket::CounterType _reorderedCounter;
         ProtocolPacket::RealmType _realm;
 
         struct Compare
@@ -395,7 +410,7 @@ class FGE_API PacketReorderer
             {
                 if (l._realm == r._realm)
                 {
-                    return l._counter > r._counter;
+                    return l._reorderedCounter > r._reorderedCounter;
                 }
                 return l._realm > r._realm;
             }
@@ -438,7 +453,7 @@ class FGE_API PacketCache
         };
     };
 
-    PacketCache() = default;
+    PacketCache();
     PacketCache(PacketCache const& r) = delete;
     PacketCache(PacketCache&& r) noexcept;
     ~PacketCache() = default;
@@ -449,6 +464,7 @@ class FGE_API PacketCache
     void clear();
     [[nodiscard]] bool isEmpty() const;
     [[nodiscard]] bool isEnabled() const;
+    [[nodiscard]] bool isAlarmed() const;
     void enable(bool enable);
 
     //Transmit
@@ -458,9 +474,7 @@ class FGE_API PacketCache
     void acknowledgeReception(std::span<Label> labels);
 
     //Check for unacknowledged packet
-    [[nodiscard]] bool check(std::chrono::steady_clock::time_point const& timePoint,
-                             std::chrono::milliseconds clientDelay);
-    [[nodiscard]] TransmitPacketPtr pop();
+    bool process(std::chrono::steady_clock::time_point const& timePoint, Client& client);
 
 private:
     struct Data
@@ -473,15 +487,14 @@ class FGE_API PacketCache
         TransmitPacketPtr _packet;
         Label _label;
         std::chrono::steady_clock::time_point _time{};
+        unsigned int _tryCount{0};
     };
 
     mutable std::mutex g_mutex;
 
-    //Circular buffer
-    std::vector<Data> g_cache{FGE_NET_PACKET_CACHE_MAX};
-    std::size_t g_start{0};
-    std::size_t g_end{0};
+    std::vector<Data> g_cache;
 
+    bool g_alarm{false};
     bool g_enable{false};
 };