Fix out of order crash (closed #29)

teeworlds-go · Sep 19, 2024 · 86061a3 · 86061a3
1 parent 4c1e148
commit 86061a3
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 27 deletions.
diff --git a/snapshot7/storage.go b/snapshot7/storage.go
@@ -62,6 +62,15 @@ type Storage struct {
 	// of multi part snapshots
 	// use AddIncomingData() and IncomingData() to read and write
 	multiPartIncomingData []byte
+
+	// the tick we are currently collecting parts for
+	CurrentRecvTick int
+
+	// received parts for the current tick
+	// as a bit field
+	// to check if we received all previous parts
+	// when we get the last part number
+	SnapshotParts int
 }
 
 func NewStorage() *Storage {

diff --git a/snapshot7/multi_part_out_of_order_test.go → teeworlds7/multi_part_out_of_order_test.go b/snapshot7/multi_part_out_of_order_test.go → teeworlds7/multi_part_out_of_order_test.go
@@ -1,4 +1,4 @@
-package snapshot7_test
+package teeworlds7
 
 import (
 	"testing"
@@ -7,7 +7,6 @@ import (
 	"github.com/teeworlds-go/protocol/messages7"
 	"github.com/teeworlds-go/protocol/network7"
 	"github.com/teeworlds-go/protocol/protocol7"
-	"github.com/teeworlds-go/protocol/teeworlds7"
 )
 
 // go client connected to public ~40 player ddnet Linear server
@@ -188,28 +187,21 @@ func Test2PartSnap(t *testing.T) {
 	// client with state and delta unpacker
 	// ------------------------------------
 
-	client := teeworlds7.NewClient()
+	client := NewClient()
 
-	// part 0-2
-	client.SnapshotStorage.AddIncomingData(part1.Part, part1.NumParts, part1.Data)
-	client.SnapshotStorage.AddIncomingData(part0.Part, part0.NumParts, part0.Data) // out of order
+	client.Session = protocol7.NewSession()
+	client.Game.Players = make([]Player, network7.MaxClients)
+	client.Session.ServerToken = [4]byte{0xd3, 0x1a, 0x0a, 0xd6}
 
-	// TODO: this crashes
-
-	// // this is the first snap being sent so we delta against empty
-	// prevSnap, found := client.SnapshotStorage.Get(snapshot7.EmptySnapTick)
-	// require.True(t, found)
-
-	// u := &packer.Unpacker{}
-	// u.Reset(client.SnapshotStorage.IncomingData())
-
-	// newFullSnap, err := snapshot7.UnpackDelta(prevSnap, u)
-	// require.NoError(t, err)
-
-	// // TODO: should this be part0 here?
-	// err = client.SnapshotStorage.Add(part0.GameTick, newFullSnap)
-	// require.NoError(t, err)
+	packet = protocol7.Packet{}
+	err = packet.Unpack(dumpPart1)
+	require.NoError(t, err)
+	err = client.processPacket(&packet)
+	require.NoError(t, err)
 
-	// // TODO:
-	// // require.Equal(t, 999999, len(newFullSnap.Items))
+	packet = protocol7.Packet{}
+	err = packet.Unpack(dumpPart0)
+	require.NoError(t, err)
+	err = client.processPacket(&packet)
+	require.NoError(t, err)
 }
diff --git a/teeworlds7/system.go b/teeworlds7/system.go
@@ -68,21 +68,37 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto
 			deltaTick := msg.GameTick - msg.DeltaTick
 			slog.Debug("got snap", "delta_tick", deltaTick, "raw_delta_tick", msg.DeltaTick, "game_tick", msg.GameTick, "part", msg.Part, "num_parts", msg.NumParts)
 
+			if client.SnapshotStorage.CurrentRecvTick < msg.GameTick {
+				slog.Debug("dropping snap with too old game tick", "game_tick", msg.GameTick, "current_recv_tick", client.SnapshotStorage.CurrentRecvTick)
+				return nil
+			}
+
+			if msg.GameTick != client.SnapshotStorage.CurrentRecvTick {
+				client.SnapshotStorage.SnapshotParts = 0
+				client.SnapshotStorage.CurrentRecvTick = msg.GameTick
+			}
+
 			err := client.SnapshotStorage.AddIncomingData(msg.Part, msg.NumParts, msg.Data)
 			if err != nil {
 				return fmt.Errorf("failed to store incoming data snap: %w", err)
 			}
 
-			// TODO: this is as naive as it gets
-			//       we should check if we actually received all the previous parts
-			//       teeworlds does some fancy bit stuff here
-			//       m_SnapshotParts |= 1<<Part;
+			client.SnapshotStorage.SnapshotParts  |= 1 << msg.Part
+
 			if msg.Part != msg.NumParts-1 {
 				// TODO: remove this print
 				slog.Debug("storing partial snap", "part", msg.Part, "num_parts", msg.NumParts)
 				return nil
 			}
 
+			if client.SnapshotStorage.SnapshotParts != msg.NumParts-1 {
+				// TODO: remove this print
+				slog.Debug("got last part but missing previous parts", "part", msg.Part, "num_parts", msg.NumParts)
+				return nil
+			}
+
+			client.SnapshotStorage.SnapshotParts = 0
+
 			prevSnap, found := client.SnapshotStorage.Get(deltaTick)
 			if !found {
 				// couldn't find the delta snapshots that the server used
@@ -119,6 +135,7 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto
 			client.Game.Input.PredictionTick = client.SnapshotStorage.NewestTick()
 			client.Game.Snap.fill(newFullSnap)
 			client.SnapshotStorage.SetAltSnap(msg.GameTick, newFullSnap)
+			client.SnapshotStorage.CurrentRecvTick = msg.GameTick
 
 			response.Messages = append(response.Messages, client.Game.Input)
 			return nil
@@ -127,6 +144,17 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto
 		err = userMsgCallback(client.Callbacks.SysSnapSingle, msg, func() error {
 			deltaTick := msg.GameTick - msg.DeltaTick
 			slog.Debug("got snap single", "delta_tick", deltaTick, "raw_delta_tick", msg.DeltaTick, "game_tick", msg.GameTick)
+
+			if client.SnapshotStorage.CurrentRecvTick < msg.GameTick {
+				slog.Debug("dropping snap with too old game tick", "game_tick", msg.GameTick, "current_recv_tick", client.SnapshotStorage.CurrentRecvTick)
+				return nil
+			}
+
+			if msg.GameTick != client.SnapshotStorage.CurrentRecvTick {
+				client.SnapshotStorage.SnapshotParts = 0
+				client.SnapshotStorage.CurrentRecvTick = msg.GameTick
+			}
+
 			prevSnap, found := client.SnapshotStorage.Get(deltaTick)
 
 			if !found {
@@ -167,6 +195,7 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto
 			altSnap := newFullSnap
 			client.Game.Snap.fill(altSnap)
 			client.SnapshotStorage.SetAltSnap(msg.GameTick, altSnap)
+			client.SnapshotStorage.CurrentRecvTick = msg.GameTick
 
 			err = client.SendInput()
 			if err != nil {
@@ -179,6 +208,17 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto
 		err = userMsgCallback(client.Callbacks.SysSnapEmpty, msg, func() error {
 			deltaTick := msg.GameTick - msg.DeltaTick
 			slog.Debug("got snap empty", "delta_tick", deltaTick, "raw_delta_tick", msg.DeltaTick, "game_tick", msg.GameTick)
+
+			if client.SnapshotStorage.CurrentRecvTick < msg.GameTick {
+				slog.Debug("dropping snap with too old game tick", "game_tick", msg.GameTick, "current_recv_tick", client.SnapshotStorage.CurrentRecvTick)
+				return nil
+			}
+
+			if msg.GameTick != client.SnapshotStorage.CurrentRecvTick {
+				client.SnapshotStorage.SnapshotParts = 0
+				client.SnapshotStorage.CurrentRecvTick = msg.GameTick
+			}
+
 			prevSnap, found := client.SnapshotStorage.Get(deltaTick)
 
 			if !found {
@@ -211,6 +251,7 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto
 			// reuse the old game state
 			// there is no need to refill if it is the same snapshot anyways
 			// client.Game.Snap.fill(prevSnap)
+			client.SnapshotStorage.CurrentRecvTick = msg.GameTick
 
 			response.Messages = append(response.Messages, client.Game.Input)
 			return nil