diff --git a/code/network/multi.cpp b/code/network/multi.cpp index d30f7017cce..d1df1225ce0 100644 --- a/code/network/multi.cpp +++ b/code/network/multi.cpp @@ -323,6 +323,9 @@ void multi_level_init() // initialize respawning multi_respawn_init(); + // forget ordered lua packet timestamps from any previous game + multi_lua_reset_packet_order(); + // initialize all netgame timestamps multi_reset_timestamps(); diff --git a/code/network/multi_lua.cpp b/code/network/multi_lua.cpp index b41fd0b9c9b..8c0dbcf3bf9 100644 --- a/code/network/multi_lua.cpp +++ b/code/network/multi_lua.cpp @@ -13,42 +13,44 @@ static SCP_unordered_map rpc_map; enum class lua_net_data_type : uint8_t { NIL, BOOL, NUMBER, STRING8, STRING16, USERDATA, TABLE }; -static bool need_toss_packet(ushort target, short packet_source, ushort packetTime, UI_TIMESTAMP localTime) { - //Ordering is enforced for packet source and execution targets. Meaning we neither order packets form different sources - //(due to incomparibility of timestamps) or to different targets (due to semantic insignificance) - static SCP_map, std::pair> received_packets; +//Ordering is enforced for packet source and execution targets. Meaning we neither order packets form different sources +//(due to incomparibility of timestamps) or to different targets (due to semantic insignificance) +static SCP_map, std::pair> received_packets; + +//Once this much local time has passed since the last accepted packet, the 16-bit remote difference may have wrapped and can't be trusted +constexpr int LUA_PACKET_ORDER_WINDOW_MS = 0x4000; - //The idea is, that we reject packets which are in the past, unless the last packet we actually got is so far in the past, we're not sure if it might have overflowed - //Basically, assume that if packetTime - lastPacketTime < 1000, then it's either delayed over 60 seconds, or not a past packet. If it's larger than that, compare it to - //the difference of local timestamps. If the local timestamp is over 2^16, it's also safe, otherwise, allow a 10% delay margin. +bool multi_lua_packet_is_stale(ushort lastRemote, ushort newRemote, int localDeltaMs) { + if (localDeltaMs >= LUA_PACKET_ORDER_WINDOW_MS) + return false; + + //Serial number arithmetic: a wrapped difference in the upper half of the range means the new packet was sent before the last one + return static_cast(newRemote - lastRemote) >= 0x8000U; +} +static bool need_toss_packet(ushort target, short packet_source, ushort packetTime, UI_TIMESTAMP localTime) { auto& channel = received_packets[{target, packet_source}]; - std::pair newTimestamp = { std::move(packetTime), std::move(localTime) }; - if (!channel.second.isValid()) { - //First packet for this channel - channel = std::move(newTimestamp); - return false; + if (channel.second.isValid() && multi_lua_packet_is_stale(channel.first, packetTime, ui_timestamp_get_delta(channel.second, localTime))) { + //Since we toss the packet, we DON'T update the received packet index. + return true; } - if (newTimestamp.first - channel.first < 1000) { - //The previous packet is less than a second ago. If this were a delayed packet, as a ushort, it'd have to be >60 seconds late, so effectively impossible. - channel = std::move(newTimestamp); - return false; - } + channel = { packetTime, localTime }; + return false; +} - if (static_cast(newTimestamp.first - channel.first) * 110 < ui_timestamp_get_delta(channel.second, newTimestamp.second)) { - //The previous packet is, at most, 10% older than its age when it arrived. We expect that most delayed packets will happen when sent in quick succession, - //so what needs to be guarded here are underflows, which produce very large values but will have very small timestamps. So as long as local and remote timestamp - //somewhat agree, the packet's good. In addition, once ui_timestamp_get_delta exceeds 65 seconds, we can't tell anymore due to the remote timestamp being uint16, - //but at that point we can just accept the packet as likely new. - channel = std::move(newTimestamp); - return false; - } +void multi_lua_reset_packet_order() { + received_packets.clear(); +} - //If we're still here, then we've got a packet that has a very high remote time but a very short local time, so likely a packet whose high remote time indicates a - //negative remote time. Toss is then. Also, since we toss the packet, we DON'T update the received packet index. - return true; +void multi_lua_forget_player(short player_id) { + for (auto it = received_packets.begin(); it != received_packets.end();) { + if (it->first.second == player_id) + it = received_packets.erase(it); + else + it++; + } } diff --git a/code/network/multi_lua.h b/code/network/multi_lua.h index 5fd638c76b0..e78717485d9 100644 --- a/code/network/multi_lua.h +++ b/code/network/multi_lua.h @@ -17,4 +17,9 @@ void process_lua_packet(ubyte* data, header* hinfo, bool reliable); bool send_lua_packet(const luacpp::LuaValue& value, ushort target, lua_net_mode mode, lua_net_reciever reciever); bool add_rpc(scripting::api::rpc_h_ref ref); -void clean_rpc_refs(); \ No newline at end of file +void clean_rpc_refs(); + +//True if an ordered packet stamped newRemote is older than the last accepted packet (stamped lastRemote) on its channel +bool multi_lua_packet_is_stale(ushort lastRemote, ushort newRemote, int localDeltaMs); +void multi_lua_reset_packet_order(); +void multi_lua_forget_player(short player_id); diff --git a/code/network/multiutil.cpp b/code/network/multiutil.cpp index 47f6a014913..05ddc8cd717 100644 --- a/code/network/multiutil.cpp +++ b/code/network/multiutil.cpp @@ -63,6 +63,7 @@ #include "network/multiteamselect.h" #include "network/multiui.h" #include "network/multi_kick.h" +#include "network/multi_lua.h" #include "network/multi_data.h" #include "network/multi_voice.h" #include "network/multi_team.h" @@ -894,6 +895,8 @@ void delete_player(int player_num,int kicked_reason) psnet_rel_close_socket(Net_players[player_num].reliable_socket); // close out the reliable socket + multi_lua_forget_player(Net_players[player_num].player_id); + // if this guy was ingame joining, the remove my netgame flag so others may join if ( Net_players[player_num].flags & NETINFO_FLAG_INGAME_JOIN ) { Netgame.flags &= ~NG_FLAG_INGAME_JOINING; diff --git a/test/src/network/test_multi_lua.cpp b/test/src/network/test_multi_lua.cpp new file mode 100644 index 00000000000..60c9379a67d --- /dev/null +++ b/test/src/network/test_multi_lua.cpp @@ -0,0 +1,58 @@ + +#include + +#include "network/multi_lua.h" + +TEST(multi_lua, in_order_packet_is_not_stale) +{ + EXPECT_FALSE(multi_lua_packet_is_stale(100, 200, 100)); +} + +TEST(multi_lua, same_timestamp_is_not_stale) +{ + EXPECT_FALSE(multi_lua_packet_is_stale(100, 100, 0)); +} + +TEST(multi_lua, reordered_packet_is_stale) +{ + EXPECT_TRUE(multi_lua_packet_is_stale(200, 100, 5)); +} + +TEST(multi_lua, wrap_forward_is_not_stale) +{ + EXPECT_FALSE(multi_lua_packet_is_stale(65000, 10, 546)); +} + +TEST(multi_lua, wrap_backward_is_stale) +{ + EXPECT_TRUE(multi_lua_packet_is_stale(10, 65000, 5)); +} + +TEST(multi_lua, delayed_previous_packet_does_not_toss_newer_packet) +{ + // sent 1500ms apart, but the previous packet arrived 200ms late + EXPECT_FALSE(multi_lua_packet_is_stale(1000, 2500, 1300)); +} + +TEST(multi_lua, long_local_gap_is_not_stale) +{ + // a remote gap of 40s looks negative as a 16-bit difference, but the local gap shows the packet is new + EXPECT_FALSE(multi_lua_packet_is_stale(0, 40000, 39000)); +} + +TEST(multi_lua, short_local_gap_with_negative_remote_gap_is_stale) +{ + EXPECT_TRUE(multi_lua_packet_is_stale(0, 40000, 100)); +} + +TEST(multi_lua, remote_gap_boundary) +{ + EXPECT_FALSE(multi_lua_packet_is_stale(0, 0x7FFF, 0)); + EXPECT_TRUE(multi_lua_packet_is_stale(0, 0x8000, 0)); +} + +TEST(multi_lua, local_window_boundary) +{ + EXPECT_TRUE(multi_lua_packet_is_stale(200, 100, 16383)); + EXPECT_FALSE(multi_lua_packet_is_stale(200, 100, 16384)); +} diff --git a/test/src/source_groups.cmake b/test/src/source_groups.cmake index 1c268dda86e..4c1c3916eba 100644 --- a/test/src/source_groups.cmake +++ b/test/src/source_groups.cmake @@ -49,6 +49,10 @@ add_file_folder("model" model/test_modelread.cpp ) +add_file_folder("Network" + network/test_multi_lua.cpp +) + add_file_folder("Options" options/test_options_manager.cpp )