minecraft: Revert deferred packet re-check, expect ResourcePacksInfo with LoginSuccess; fix pong game type fields

minecraft/conn.go

@@ -1032,14 +1032,14 @@ func (conn *Conn) handleResourcePacksInfo(pk *packet.ResourcePacksInfo) error {
 	}
 
 	if len(packsToDownload) != 0 {
-		conn.expect(packet.IDResourcePackDataInfo, packet.IDResourcePackChunkData)
+		conn.expect(packet.IDResourcePackDataInfo, packet.IDResourcePackChunkData, packet.IDPlayStatus)
 		_ = conn.WritePacket(&packet.ResourcePackClientResponse{
 			Response:        packet.PackResponseSendPacks,
 			PacksToDownload: packsToDownload,
 		})
 		return nil
 	}
-	conn.expect(packet.IDResourcePackStack)
+	conn.expect(packet.IDResourcePackStack, packet.IDPlayStatus)
 
 	_ = conn.WritePacket(&packet.ResourcePackClientResponse{Response: packet.PackResponseAllPacksDownloaded})
 	return nil
@@ -1147,6 +1147,8 @@ func (conn *Conn) handleResourcePackClientResponse(pk *packet.ResourcePackClient
 
 // startGame sends a StartGame packet using the game data of the connection.
 func (conn *Conn) startGame() {
+	// The client may answer before the packets below are all written, so expect its replies first.
+	conn.expect(packet.IDRequestChunkRadius, packet.IDSetLocalPlayerAsInitialised)
 	data := conn.gameData
 	if len(data.Dimensions) > 0 {
 		_ = conn.WritePacket(&packet.DimensionData{Definitions: data.Dimensions})
@@ -1206,7 +1208,6 @@ func (conn *Conn) startGame() {
 	})
 	_ = conn.WritePacket(&packet.ItemRegistry{Items: data.Items})
 	_ = conn.Flush()
-	conn.expect(packet.IDRequestChunkRadius, packet.IDSetLocalPlayerAsInitialised)
 }
 
 // nextResourcePackDownload moves to the next resource pack to download and sends a resource pack data info
@@ -1291,7 +1292,7 @@ func (conn *Conn) handleResourcePackDataInfo(pk *packet.ResourcePackDataInfo) er
 		conn.packMu.Unlock()
 
 		if packAmount == 0 {
-			conn.expect(packet.IDResourcePackStack)
+			conn.expect(packet.IDResourcePackStack, packet.IDPlayStatus)
 			_ = conn.WritePacket(&packet.ResourcePackClientResponse{Response: packet.PackResponseAllPacksDownloaded})
 		}
 		conn.storeResourcePack(pack.cacheKey, newPack)
@@ -1478,7 +1479,9 @@ func (conn *Conn) handleChunkRadiusUpdated(pk *packet.ChunkRadiusUpdated) error
 	if pk.ChunkRadius < 1 {
 		return fmt.Errorf("expected chunk radius of at least 1, got %v", pk.ChunkRadius)
 	}
-	conn.expect(packet.IDPlayStatus)
+	// Some servers send ResourcePacksInfo before PlayStatus(LoginSuccess); the vanilla client accepts either
+	// order, so both are expected from here on.
+	conn.expect(packet.IDPlayStatus, packet.IDResourcePacksInfo)
 
 	conn.gameData.ChunkRadius = pk.ChunkRadius
 	conn.gameDataReceived.Store(true)
@@ -1508,8 +1511,7 @@ func (conn *Conn) handlePlayStatus(pk *packet.PlayStatus) error {
 		if err := conn.WritePacket(&packet.ClientCacheStatus{Enabled: conn.cacheEnabled}); err != nil {
 			return fmt.Errorf("send ClientCacheStatus: %w", err)
 		}
-		// The next packet we expect is the ResourcePacksInfo packet.
-		conn.expect(packet.IDResourcePacksInfo)
+		// ResourcePacksInfo is already expected, and may even have been handled if the server sent it first.
 		return conn.Flush()
 	case packet.PlayStatusLoginFailedClient:
 		_ = conn.close(conn.closeErr("client outdated"))
@@ -1610,33 +1612,11 @@ func (conn *Conn) encryptionKey(salt []byte, pub *ecdsa.PublicKey) ([32]byte, er
 	return sha256.Sum256(append(salt, sharedSecret...)), nil
 }
 
-// expect sets the packet IDs that are next expected to arrive and re-checks
-// any deferred packets against the new expected set. This prevents a deadlock
-// when a packet arrives before its ID is added to the expected set.
+// expect sets the packet IDs that are next expected to arrive. A packet that was deferred before its ID was
+// expected is not handled later: expect everything a peer may send at a stage before writing the packet it
+// answers to.
 func (conn *Conn) expect(packetIDs ...uint32) {
 	conn.expectedIDs.Store(packetIDs)
-	conn.handleDeferredPackets()
-}
-
-// handleDeferredPackets passes all currently deferred packets back through
-// handle(). Packets that now match expectedIDs are processed; the rest are
-// re-deferred by handle() automatically.
-func (conn *Conn) handleDeferredPackets() {
-	conn.deferredPacketMu.Lock()
-	if len(conn.deferredPackets) == 0 {
-		conn.deferredPacketMu.Unlock()
-		return
-	}
-	deferred := conn.deferredPackets
-	conn.deferredPackets = conn.deferredPackets[len(deferred):]
-	conn.deferredPacketMu.Unlock()
-
-	for _, pkData := range deferred {
-		if err := conn.handle(pkData); err != nil {
-			_ = conn.close(err)
-			return
-		}
-	}
 }
 
 // closeTransport closes conn without waiting for pending packets to be written. The context is cancelled

minecraft/dial.go

@@ -350,7 +350,9 @@ func (d Dialer) DialContextNetwork(ctx context.Context, network Network, address
 		return nil, conn.closeErr("dial")
 	case <-readyForLogin:
 		// We've received our network settings, so we can now send our login request.
-		conn.expect(packet.IDServerToClientHandshake, packet.IDPlayStatus)
+		// ResourcePacksInfo is expected too: servers without encryption skip the handshake and some send it
+		// before PlayStatus(LoginSuccess).
+		conn.expect(packet.IDServerToClientHandshake, packet.IDPlayStatus, packet.IDResourcePacksInfo)
 		if err := conn.WritePacket(&packet.Login{ConnectionRequest: request, ClientProtocol: d.Protocol.ID()}); err != nil {
 			return nil, conn.wrap(fmt.Errorf("send login: %w", err), "dial")
 		}

minecraft/listener.go

@@ -397,28 +397,19 @@ func (listener *Listener) PlayerCount() int {
 // server name of the listener, provided the listener isn't currently hijacking the pong of another server.
 func (listener *Listener) updatePongData() {
 	var (
-		s        = listener.status()
-		port     uint16
-		gameType string
+		s    = listener.status()
+		port uint16
 	)
-	switch s.GameType {
-	case 0:
-		gameType = "Survival"
-	case 1:
-		gameType = "Creative"
-	case 2:
-		gameType = "Adventure"
-	default:
-		gameType = "Survival"
-	}
 	if a, ok := listener.Addr().(interface {
 		AddrPort() netip.AddrPort
 	}); ok {
 		port = a.AddrPort().Port()
 	}
+	// The field after the game mode is a separate flag: clients drop pongs of ten or more fields unless it
+	// is exactly 1.
 	listener.listener.PongData([]byte(fmt.Sprintf("MCPE;%v;%v;%v;%v;%v;%v;%v;%v;%v;%v;%v;%v;%v;",
 		s.ServerName, protocol.CurrentProtocol, protocol.CurrentVersion, s.PlayerCount, s.MaxPlayers,
-		listener.listener.ID(), s.ServerSubName, gameType, s.GameType, port, port, 0, 0,
+		listener.listener.ID(), s.ServerSubName, gameTypeName(s.GameType), 1, port, port, 0, 0,
 	)))
 }
 

minecraft/server_status_provider.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/sandertv/go-raknet"
+	"github.com/sandertv/gophertunnel/minecraft/protocol/packet"
 )
 
 // ServerStatusProvider represents a type that is able to provide the visual status of a server, in specific
@@ -36,8 +37,9 @@ type ServerStatus struct {
 	// MaxPlayers is the maximum amount of players in the server. If set to 0, MaxPlayers is set to
 	// PlayerCount + 1.
 	MaxPlayers int
-	// GameType is the default game mode configured in the server, as shown in the friend list.
-	// It is 0 for Survival, 1 for Creative, and 2 for Adventure.
+	// GameType is the default game mode of the server, as shown in the friend list. It is one of
+	// packet.GameTypeSurvival, packet.GameTypeCreative, packet.GameTypeAdventure or packet.GameTypeSpectator;
+	// any other value is advertised as Survival.
 	GameType int
 }
 
@@ -127,7 +129,7 @@ func (f *ForeignStatusProvider) update() {
 // ParsePongData parses the unconnected pong data passed into the relevant fields of a ServerStatus struct.
 func ParsePongData(pong []byte) ServerStatus {
 	frag := splitPong(string(pong))
-	if len(frag) < 9 {
+	if len(frag) < 8 {
 		return ServerStatus{ServerName: "Invalid pong data"}
 	}
 	serverName := frag[1]
@@ -140,29 +142,43 @@ func ParsePongData(pong []byte) ServerStatus {
 	if err != nil {
 		return ServerStatus{ServerName: "Invalid max player count"}
 	}
-	gameType, ok := parseGameType(frag[8])
-	if !ok {
-		return ServerStatus{ServerName: "Invalid game type"}
-	}
-	return ServerStatus{
+	status := ServerStatus{
 		ServerName:    serverName,
 		ServerSubName: serverSubName,
 		PlayerCount:   online,
 		MaxPlayers:    max,
-		GameType:      gameType,
 	}
+	// The game mode field is optional and, like the client, an unknown mode does not invalidate the pong.
+	if len(frag) > 8 {
+		status.GameType = parseGameType(frag[8])
+	}
+	return status
+}
+
+// gameTypeName returns the pong string for a ServerStatus.GameType. Types without a pong string are
+// advertised as Survival.
+func gameTypeName(gameType int) string {
+	switch gameType {
+	case packet.GameTypeCreative:
+		return "Creative"
+	case packet.GameTypeAdventure:
+		return "Adventure"
+	case packet.GameTypeSpectator:
+		return "Spectator"
+	}
+	return "Survival"
 }
 
-// parseGameType converts the game type string from the pong data to its int representation.
-// Returns 0 and false if the game type is not valid.
-func parseGameType(v string) (int, bool) {
+// parseGameType converts the game mode string of a pong to a ServerStatus.GameType. Unknown modes are
+// treated as Survival.
+func parseGameType(v string) int {
 	switch strings.ToLower(strings.TrimSpace(v)) {
-	case "survival":
-		return 0, true
 	case "creative":
-		return 1, true
+		return packet.GameTypeCreative
 	case "adventure":
-		return 2, true
+		return packet.GameTypeAdventure
+	case "spectator":
+		return packet.GameTypeSpectator
 	}
-	return 0, false
+	return packet.GameTypeSurvival
 }