diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
new file mode 100644
index 0000000..d5b98bd
--- /dev/null
+++ b/.github/workflows/check.yml
@@ -0,0 +1,47 @@
+name: Lint
+
+on:
+ workflow_call:
+
+jobs:
+ bot2-check:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: astral-sh/setup-uv@v4
+
+ - uses: arduino/setup-task@v2
+
+ - name: Run bot2 linting
+ run: task bot2:lint
+
+ - name: Check bot2 formatting
+ run: task bot2:format:check
+
+ - name: Run bot2 type checking
+ run: task bot2:typecheck
+
+ backend-check:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: actions/setup-go@v5
+ with:
+ go-version: "1.24"
+
+ - uses: golangci/golangci-lint-action@v6
+ with:
+ working-directory: backend
+
+ - uses: arduino/setup-task@v2
+
+ - name: Check backend formatting
+ run: task be:format:check
+
+ - name: Run go vet
+ run: task be:vet
+
+ - name: Run build
+ run: task be:build
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 0000000..98b9e4e
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,16 @@
+name: CI
+
+on:
+ pull_request:
+ types: [opened, synchronize, reopened]
+
+jobs:
+ lint:
+ uses: ./.github/workflows/check.yml
+
+ test:
+ uses: ./.github/workflows/test.yml
+
+ integration:
+ uses: ./.github/workflows/integration.yml
+ needs: [lint, test]
diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml
new file mode 100644
index 0000000..8ef973a
--- /dev/null
+++ b/.github/workflows/integration.yml
@@ -0,0 +1,37 @@
+name: Integration
+
+on:
+ workflow_call:
+
+jobs:
+ bot2-integration:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: astral-sh/setup-uv@v4
+
+ - uses: arduino/setup-task@v2
+
+ - name: Start services with docker compose
+ run: task compose
+
+ # TODO working health check endpoint
+ - name: Wait for services to be ready
+ run: |
+ echo "Waiting for services to be ready..."
+ for i in {1..30}; do
+ if curl -s http://localhost:4000/api/maps > /dev/null 2>&1; then
+ echo "Services are ready!"
+ break
+ fi
+ echo "Waiting... ($i/3)"
+ sleep 2
+ done
+
+ - name: Run bot2 integration tests (not slow)
+ run: task bot2:test:integration:fast
+
+ - name: Cleanup
+ if: always()
+ run: docker compose down
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
new file mode 100644
index 0000000..b6f696a
--- /dev/null
+++ b/.github/workflows/test.yml
@@ -0,0 +1,31 @@
+name: Test
+
+on:
+ workflow_call:
+
+jobs:
+ bot2-test:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: astral-sh/setup-uv@v4
+
+ - uses: arduino/setup-task@v2
+
+ - name: Run bot2 unit tests
+ run: task bot2:test:unit
+
+ backend-test:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: actions/setup-go@v5
+ with:
+ go-version: "1.24"
+
+ - uses: arduino/setup-task@v2
+
+ - name: Run backend tests
+ run: task be:test
diff --git a/Taskfile.yml b/Taskfile.yml
index 8822738..e3102a9 100644
--- a/Taskfile.yml
+++ b/Taskfile.yml
@@ -42,6 +42,14 @@ tasks:
deps:
- bot2:install
+ bot2:format:check:
+ dir: bot2
+ desc: Check bot2 code formatting (no fix)
+ cmds:
+ - uv run ruff format --check .
+ deps:
+ - bot2:install
+
bot2:typecheck:
dir: bot2
desc: Run bot2 type checking with ty
@@ -132,6 +140,12 @@ tasks:
cmds:
- go fmt ./...
+ be:format:check:
+ dir: backend
+ desc: Check backend Go code formatting (no fix)
+ cmds:
+ - test -z "$(gofmt -l .)"
+
be:vet:
dir: backend
desc: Run go vet on backend code
diff --git a/backend/pkg/server/game_maps/map_metadata.go b/backend/pkg/server/game_maps/map_metadata.go
index d95a097..fa94c1c 100644
--- a/backend/pkg/server/game_maps/map_metadata.go
+++ b/backend/pkg/server/game_maps/map_metadata.go
@@ -101,8 +101,8 @@ func GetValidMapTypes() ([]string, error) {
for _, md := range metadata {
// Extract map type name from "meta/name.json" format
mapType := string(md.MapType)
- mapType = mapType[5:] // Remove "meta/" prefix
- mapType = mapType[:len(mapType)-5] // Remove ".json" suffix
+ mapType = mapType[5:] // Remove "meta/" prefix
+ mapType = mapType[:len(mapType)-5] // Remove ".json" suffix
mapTypes = append(mapTypes, mapType)
}
return mapTypes, nil
diff --git a/backend/pkg/server/game_room_test.go b/backend/pkg/server/game_room_test.go
index 58d2cf8..297bc0d 100644
--- a/backend/pkg/server/game_room_test.go
+++ b/backend/pkg/server/game_room_test.go
@@ -639,25 +639,27 @@ func TestGameRoomReset(t *testing.T) {
}
// Verify player state is reset
- state := playerObj.GetState()
+ // Note: We use GetStateValue() instead of GetState() because GetState() applies
+ // extrapolation (including gravity) based on time elapsed since lastLocUpdateTime,
+ // which would cause flaky tests due to timing differences between local and CI.
// Velocity should be 0
- if dx, ok := state["dx"].(float64); !ok || dx != 0.0 {
- t.Errorf("Player dx should be 0 after reset, got %v", state["dx"])
+ if dx, exists := playerObj.GetStateValue("dx"); !exists || dx.(float64) != 0.0 {
+ t.Errorf("Player dx should be 0 after reset, got %v", dx)
}
- if dy, ok := state["dy"].(float64); !ok || dy != 0.0 {
- t.Errorf("Player dy should be 0 after reset, got %v", state["dy"])
+ if dy, exists := playerObj.GetStateValue("dy"); !exists || dy.(float64) != 0.0 {
+ t.Errorf("Player dy should be 0 after reset, got %v", dy)
}
// Arrows should be reset to starting count (4)
// Note: Arrow count is stored as int, not float64
- if arrows, ok := state["ac"].(int); !ok || arrows != 4 {
- t.Errorf("Player arrows should be 4 after reset, got %v", state["ac"])
+ if arrows, exists := playerObj.GetStateValue("ac"); !exists || arrows.(int) != 4 {
+ t.Errorf("Player arrows should be 4 after reset, got %v", arrows)
}
// Dead should be false
- if dead, ok := state["dead"].(bool); !ok || dead {
- t.Errorf("Player should not be dead after reset, got %v", state["dead"])
+ if dead, exists := playerObj.GetStateValue("dead"); !exists || dead.(bool) {
+ t.Errorf("Player should not be dead after reset, got %v", dead)
}
// Player should still exist in the room
diff --git a/backend/pkg/server/geo/shape.go b/backend/pkg/server/geo/shape.go
index 65fe14e..073e8ee 100644
--- a/backend/pkg/server/geo/shape.go
+++ b/backend/pkg/server/geo/shape.go
@@ -43,13 +43,13 @@ func (l *Line) GetCenter() *Point {
}
func (l *Line) CollidesWith(other Shape) (bool, []*Point) {
- switch other.(type) {
+ switch o := other.(type) {
case *Line:
- return checkLineLineCollision(l, other.(*Line))
+ return checkLineLineCollision(l, o)
case *Circle:
- return checkLineCircleCollision(l, other.(*Circle))
+ return checkLineCircleCollision(l, o)
case *Polygon:
- return checkLinePolygonCollision(l, other.(*Polygon))
+ return checkLinePolygonCollision(l, o)
}
return false, nil
}
@@ -72,13 +72,13 @@ func (c *Circle) GetCenter() *Point {
}
func (c *Circle) CollidesWith(other Shape) (bool, []*Point) {
- switch other.(type) {
+ switch o := other.(type) {
case *Line:
- return checkLineCircleCollision(other.(*Line), c)
+ return checkLineCircleCollision(o, c)
case *Circle:
- return checkCircleCircleCollision(c, other.(*Circle))
+ return checkCircleCircleCollision(c, o)
case *Polygon:
- return checkCirclePolygonCollision(c, other.(*Polygon))
+ return checkCirclePolygonCollision(c, o)
}
return false, nil
}
@@ -112,13 +112,13 @@ func (p *Polygon) GetLines() []*Line {
}
func (p *Polygon) CollidesWith(other Shape) (bool, []*Point) {
- switch other.(type) {
+ switch o := other.(type) {
case *Line:
- return checkLinePolygonCollision(other.(*Line), p)
+ return checkLinePolygonCollision(o, p)
case *Circle:
- return checkCirclePolygonCollision(other.(*Circle), p)
+ return checkCirclePolygonCollision(o, p)
case *Polygon:
- return checkPolygonPolygonCollision(p, other.(*Polygon))
+ return checkPolygonPolygonCollision(p, o)
}
return false, nil
}
diff --git a/backend/pkg/server/geo/shape_test.go b/backend/pkg/server/geo/shape_test.go
index 89e82a1..770734e 100644
--- a/backend/pkg/server/geo/shape_test.go
+++ b/backend/pkg/server/geo/shape_test.go
@@ -226,7 +226,7 @@ func TestPolygonPolygonCollision(t *testing.T) {
},
},
want: true,
- numPoints: 2, // Two points: one at each end of the overlapping edge (2,1) and (2,2)
+ numPoints: 2, // Two points: one at each end of the overlapping edge (2,1) and (2,2)
},
}
diff --git a/backend/pkg/server/http_handlers.go b/backend/pkg/server/http_handlers.go
index 838f300..826ea89 100644
--- a/backend/pkg/server/http_handlers.go
+++ b/backend/pkg/server/http_handlers.go
@@ -14,6 +14,13 @@ import (
"time"
)
+// writeJSON writes a JSON response to the http.ResponseWriter and logs any encoding errors
+func writeJSON(w http.ResponseWriter, v interface{}) {
+ if err := json.NewEncoder(w).Encode(v); err != nil {
+ log.Printf("Failed to encode JSON response: %v", err)
+ }
+}
+
// HandleGetMaps handles HTTP requests to get available maps
func (s *Server) HandleGetMaps(w http.ResponseWriter, r *http.Request) {
// Set response headers
@@ -31,7 +38,7 @@ func (s *Server) HandleGetMaps(w http.ResponseWriter, r *http.Request) {
// Only allow GET requests
if r.Method != "GET" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(map[string]interface{}{
+ writeJSON(w, map[string]interface{}{
"error": "Method not allowed",
})
return
@@ -41,7 +48,7 @@ func (s *Server) HandleGetMaps(w http.ResponseWriter, r *http.Request) {
metadata, err := game_maps.GetAllMapsMetadata()
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
- json.NewEncoder(w).Encode(map[string]interface{}{
+ writeJSON(w, map[string]interface{}{
"error": fmt.Sprintf("Failed to get maps: %v", err),
})
return
@@ -66,7 +73,7 @@ func (s *Server) HandleGetMaps(w http.ResponseWriter, r *http.Request) {
// Send response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(GetMapsResponse{
+ writeJSON(w, GetMapsResponse{
Maps: maps,
})
}
@@ -124,10 +131,10 @@ type JoinGameHTTPResponse struct {
CanvasSizeY int `json:"canvasSizeY,omitempty"`
Error string `json:"error,omitempty"`
// Training mode settings (returned when joining a training game as spectator)
- TrainingMode bool `json:"trainingMode,omitempty"`
- TickMultiplier float64 `json:"tickMultiplier,omitempty"`
- MaxGameDurationSec int `json:"maxGameDurationSec,omitempty"`
- MaxKills int `json:"maxKills,omitempty"`
+ TrainingMode bool `json:"trainingMode,omitempty"`
+ TickMultiplier float64 `json:"tickMultiplier,omitempty"`
+ MaxGameDurationSec int `json:"maxGameDurationSec,omitempty"`
+ MaxKills int `json:"maxKills,omitempty"`
}
// GetRoomStateResponse represents the response to a room state request
@@ -167,7 +174,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
// Only allow POST requests
if r.Method != "POST" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: "Method not allowed",
})
@@ -178,7 +185,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
var req CreateGameHTTPRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: "Invalid request format",
})
@@ -188,7 +195,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
// Validate request
if req.PlayerName == "" || req.RoomName == "" {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: "PlayerName and RoomName are required",
})
@@ -203,7 +210,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
// The issue spec mentions 1.0-100.0, but the server's MinTickInterval of 1ms limits practical max to 20x.
if req.TickMultiplier != 0 && (req.TickMultiplier < 1.0 || req.TickMultiplier > 20.0) {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: "tickMultiplier must be between 1.0 and 20.0",
})
@@ -211,7 +218,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
}
if req.MaxGameDurationSec < 0 || req.MaxGameDurationSec > 3600 {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: "maxGameDurationSec must be between 0 and 3600",
})
@@ -219,7 +226,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
}
if req.MaxKills < 0 {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: "maxKills must be non-negative",
})
@@ -239,7 +246,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
if !game_maps.IsValidMapType(req.MapType) {
validTypes, _ := game_maps.GetValidMapTypes()
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: fmt.Sprintf("invalid mapType '%s', valid types are: %v", req.MapType, validTypes),
})
@@ -251,7 +258,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
room, player, err := NewGameWithPlayerAndTrainingConfig(req.RoomName, req.PlayerName, mapType, nil, trainingOptions)
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
- json.NewEncoder(w).Encode(CreateGameHTTPResponse{
+ writeJSON(w, CreateGameHTTPResponse{
Success: false,
Error: err.Error(),
})
@@ -290,7 +297,7 @@ func (s *Server) HandleCreateGame(w http.ResponseWriter, r *http.Request) {
// Return success response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(response)
+ writeJSON(w, response)
log.Printf("Created game room %s with code %s via HTTP API", room.ID, room.RoomCode)
log.Printf("Player %s joined game room %s via HTTP API", player.ID, room.ID)
@@ -313,7 +320,7 @@ func (s *Server) HandleJoinGame(w http.ResponseWriter, r *http.Request) {
// Only allow POST requests
if r.Method != "POST" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(JoinGameHTTPResponse{
+ writeJSON(w, JoinGameHTTPResponse{
Success: false,
Error: "Method not allowed",
})
@@ -324,7 +331,7 @@ func (s *Server) HandleJoinGame(w http.ResponseWriter, r *http.Request) {
var req JoinGameHTTPRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(JoinGameHTTPResponse{
+ writeJSON(w, JoinGameHTTPResponse{
Success: false,
Error: "Invalid request format",
})
@@ -334,7 +341,7 @@ func (s *Server) HandleJoinGame(w http.ResponseWriter, r *http.Request) {
// Validate request
if req.PlayerName == "" || req.RoomCode == "" || req.RoomPassword == "" {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(JoinGameHTTPResponse{
+ writeJSON(w, JoinGameHTTPResponse{
Success: false,
Error: "PlayerName, RoomCode, and RoomPassword are required",
})
@@ -345,7 +352,7 @@ func (s *Server) HandleJoinGame(w http.ResponseWriter, r *http.Request) {
room, exists := s.roomManager.GetGameRoomByCode(req.RoomCode)
if !exists {
w.WriteHeader(http.StatusNotFound)
- json.NewEncoder(w).Encode(JoinGameHTTPResponse{
+ writeJSON(w, JoinGameHTTPResponse{
Success: false,
Error: "Room not found",
})
@@ -357,7 +364,7 @@ func (s *Server) HandleJoinGame(w http.ResponseWriter, r *http.Request) {
player, err := AddPlayerToGame(room, req.PlayerName, req.RoomPassword, isSpectator)
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
- json.NewEncoder(w).Encode(JoinGameHTTPResponse{
+ writeJSON(w, JoinGameHTTPResponse{
Success: false,
Error: err.Error(),
})
@@ -394,7 +401,7 @@ func (s *Server) HandleJoinGame(w http.ResponseWriter, r *http.Request) {
// Return success response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(response)
+ writeJSON(w, response)
log.Printf("Player %s joined game room %s via HTTP API", player.ID, room.ID)
}
@@ -416,7 +423,7 @@ func (s *Server) HandleGetRoomState(w http.ResponseWriter, r *http.Request) {
// Only allow GET requests
if r.Method != "GET" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(GetRoomStateResponse{
+ writeJSON(w, GetRoomStateResponse{
Success: false,
Error: "Method not allowed",
})
@@ -429,7 +436,7 @@ func (s *Server) HandleGetRoomState(w http.ResponseWriter, r *http.Request) {
suffix := "/state"
if !strings.HasPrefix(path, prefix) || !strings.HasSuffix(path, suffix) {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(GetRoomStateResponse{
+ writeJSON(w, GetRoomStateResponse{
Success: false,
Error: "Invalid URL format",
})
@@ -438,7 +445,7 @@ func (s *Server) HandleGetRoomState(w http.ResponseWriter, r *http.Request) {
roomID := strings.TrimSuffix(strings.TrimPrefix(path, prefix), suffix)
if roomID == "" {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(GetRoomStateResponse{
+ writeJSON(w, GetRoomStateResponse{
Success: false,
Error: "Room ID is required",
})
@@ -459,7 +466,7 @@ func (s *Server) HandleGetRoomState(w http.ResponseWriter, r *http.Request) {
}
if playerToken == "" {
w.WriteHeader(http.StatusUnauthorized)
- json.NewEncoder(w).Encode(GetRoomStateResponse{
+ writeJSON(w, GetRoomStateResponse{
Success: false,
Error: "Player token is required (provide via playerToken query param, X-Player-Token header, or Authorization: Bearer header)",
})
@@ -470,7 +477,7 @@ func (s *Server) HandleGetRoomState(w http.ResponseWriter, r *http.Request) {
room, exists := s.roomManager.GetGameRoom(roomID)
if !exists {
w.WriteHeader(http.StatusNotFound)
- json.NewEncoder(w).Encode(GetRoomStateResponse{
+ writeJSON(w, GetRoomStateResponse{
Success: false,
Error: "Room not found",
})
@@ -480,7 +487,7 @@ func (s *Server) HandleGetRoomState(w http.ResponseWriter, r *http.Request) {
// Verify player token belongs to a player in the room (thread-safe)
if !room.IsPlayerTokenValid(playerToken) {
w.WriteHeader(http.StatusForbidden)
- json.NewEncoder(w).Encode(GetRoomStateResponse{
+ writeJSON(w, GetRoomStateResponse{
Success: false,
Error: "Player token is not authorized for this room",
})
@@ -492,7 +499,7 @@ func (s *Server) HandleGetRoomState(w http.ResponseWriter, r *http.Request) {
// Return success response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(GetRoomStateResponse{
+ writeJSON(w, GetRoomStateResponse{
Success: true,
RoomID: room.ID,
Timestamp: time.Now().UTC().Format(time.RFC3339),
@@ -517,7 +524,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
// Only allow POST requests
if r.Method != "POST" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Method not allowed",
})
@@ -530,7 +537,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
suffix := "/reset"
if !strings.HasPrefix(path, prefix) || !strings.HasSuffix(path, suffix) {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Invalid URL format",
})
@@ -539,7 +546,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
roomID := strings.TrimSuffix(strings.TrimPrefix(path, prefix), suffix)
if roomID == "" {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Room ID is required",
})
@@ -557,7 +564,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
}
if playerToken == "" {
w.WriteHeader(http.StatusUnauthorized)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Player token is required (provide via X-Player-Token header or Authorization: Bearer header)",
})
@@ -568,7 +575,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
room, exists := s.roomManager.GetGameRoom(roomID)
if !exists {
w.WriteHeader(http.StatusNotFound)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Room not found",
})
@@ -578,7 +585,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
// Verify player token belongs to a player in the room (thread-safe)
if !room.IsPlayerTokenValid(playerToken) {
w.WriteHeader(http.StatusForbidden)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Player token is not authorized for this room",
})
@@ -590,7 +597,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
if r.Body != nil && r.ContentLength > 0 {
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Invalid request format",
})
@@ -600,7 +607,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
// Validate room password if provided
if req.RoomPassword != "" && room.Password != strings.ToUpper(req.RoomPassword) {
w.WriteHeader(http.StatusUnauthorized)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: false,
Error: "Invalid room password",
})
@@ -624,7 +631,7 @@ func (s *Server) HandleResetGame(w http.ResponseWriter, r *http.Request) {
// Return success response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(ResetGameHTTPResponse{
+ writeJSON(w, ResetGameHTTPResponse{
Success: true,
})
@@ -648,7 +655,7 @@ func (s *Server) HandleGetRoomStats(w http.ResponseWriter, r *http.Request) {
// Only allow GET requests
if r.Method != "GET" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(GetRoomStatsHTTPResponse{
+ writeJSON(w, GetRoomStatsHTTPResponse{
Success: false,
Error: "Method not allowed",
})
@@ -661,7 +668,7 @@ func (s *Server) HandleGetRoomStats(w http.ResponseWriter, r *http.Request) {
suffix := "/stats"
if !strings.HasPrefix(path, prefix) || !strings.HasSuffix(path, suffix) {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(GetRoomStatsHTTPResponse{
+ writeJSON(w, GetRoomStatsHTTPResponse{
Success: false,
Error: "Invalid URL format",
})
@@ -670,7 +677,7 @@ func (s *Server) HandleGetRoomStats(w http.ResponseWriter, r *http.Request) {
roomID := strings.TrimSuffix(strings.TrimPrefix(path, prefix), suffix)
if roomID == "" {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(GetRoomStatsHTTPResponse{
+ writeJSON(w, GetRoomStatsHTTPResponse{
Success: false,
Error: "Room ID is required",
})
@@ -691,7 +698,7 @@ func (s *Server) HandleGetRoomStats(w http.ResponseWriter, r *http.Request) {
}
if playerToken == "" {
w.WriteHeader(http.StatusUnauthorized)
- json.NewEncoder(w).Encode(GetRoomStatsHTTPResponse{
+ writeJSON(w, GetRoomStatsHTTPResponse{
Success: false,
Error: "Player token is required (provide via playerToken query param, X-Player-Token header, or Authorization: Bearer header)",
})
@@ -702,7 +709,7 @@ func (s *Server) HandleGetRoomStats(w http.ResponseWriter, r *http.Request) {
room, exists := s.roomManager.GetGameRoom(roomID)
if !exists {
w.WriteHeader(http.StatusNotFound)
- json.NewEncoder(w).Encode(GetRoomStatsHTTPResponse{
+ writeJSON(w, GetRoomStatsHTTPResponse{
Success: false,
Error: "Room not found",
})
@@ -712,7 +719,7 @@ func (s *Server) HandleGetRoomStats(w http.ResponseWriter, r *http.Request) {
// Verify player token belongs to a player in the room (thread-safe)
if !room.IsPlayerTokenValid(playerToken) {
w.WriteHeader(http.StatusForbidden)
- json.NewEncoder(w).Encode(GetRoomStatsHTTPResponse{
+ writeJSON(w, GetRoomStatsHTTPResponse{
Success: false,
Error: "Player token is not authorized for this room",
})
@@ -733,7 +740,7 @@ func (s *Server) HandleGetRoomStats(w http.ResponseWriter, r *http.Request) {
// Return success response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(GetRoomStatsHTTPResponse{
+ writeJSON(w, GetRoomStatsHTTPResponse{
Success: true,
RoomID: room.ID,
PlayerStats: playerStatsDTO,
@@ -769,7 +776,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
// Only allow POST requests
if r.Method != "POST" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Method not allowed",
})
@@ -780,7 +787,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
roomID, playerID, ok := extractBotActionPathParams(r.URL.Path)
if !ok {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Invalid URL format. Expected: /api/rooms/{roomId}/players/{playerId}/action",
})
@@ -798,7 +805,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
if playerToken == "" {
log.Printf("HandleBotAction: Missing player token for room %s, player %s", roomID, playerID)
w.WriteHeader(http.StatusUnauthorized)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Player token is required (provide via X-Player-Token header or Authorization: Bearer header)",
})
@@ -809,7 +816,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
room, exists := s.roomManager.GetGameRoom(roomID)
if !exists {
w.WriteHeader(http.StatusNotFound)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Room not found",
})
@@ -820,7 +827,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
player, exists := room.GetPlayer(playerID)
if !exists {
w.WriteHeader(http.StatusNotFound)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Player not found",
})
@@ -831,7 +838,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
if player.Token != playerToken {
log.Printf("HandleBotAction: Invalid player token for room %s, player %s", roomID, playerID)
w.WriteHeader(http.StatusUnauthorized)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Invalid player token",
})
@@ -843,7 +850,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
var req types.BotActionRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Invalid request format",
})
@@ -861,7 +868,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
key := strings.ToUpper(action.Key)
if key != "W" && key != "A" && key != "S" && key != "D" {
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: "Invalid key value. Must be W, A, S, or D",
})
@@ -911,7 +918,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
default:
w.WriteHeader(http.StatusBadRequest)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: false,
Error: fmt.Sprintf("Invalid action type: %s. Must be key, click, or direction", action.Type),
})
@@ -930,7 +937,7 @@ func (s *Server) HandleBotAction(w http.ResponseWriter, r *http.Request) {
// Return success response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(types.BotActionResponse{
+ writeJSON(w, types.BotActionResponse{
Success: true,
ActionsProcessed: actionsProcessed,
Timestamp: time.Now().UnixMilli(),
@@ -956,7 +963,7 @@ func (s *Server) HandleGetTrainingSessions(w http.ResponseWriter, r *http.Reques
// Only allow GET requests
if r.Method != "GET" {
w.WriteHeader(http.StatusMethodNotAllowed)
- json.NewEncoder(w).Encode(map[string]interface{}{
+ writeJSON(w, map[string]interface{}{
"error": "Method not allowed",
})
return
@@ -984,7 +991,7 @@ func (s *Server) HandleGetTrainingSessions(w http.ResponseWriter, r *http.Reques
// Return success response
w.WriteHeader(http.StatusOK)
- json.NewEncoder(w).Encode(GetTrainingSessionsResponse{
+ writeJSON(w, GetTrainingSessionsResponse{
Sessions: sessions,
})
}
diff --git a/backend/pkg/server/http_handlers_test.go b/backend/pkg/server/http_handlers_test.go
index 022f3e4..81d91ee 100644
--- a/backend/pkg/server/http_handlers_test.go
+++ b/backend/pkg/server/http_handlers_test.go
@@ -251,7 +251,9 @@ func TestHandleCreateGame_TrainingModeResponse(t *testing.T) {
}
var response CreateGameHTTPResponse
- json.NewDecoder(rr.Body).Decode(&response)
+ if err := json.NewDecoder(rr.Body).Decode(&response); err != nil {
+ t.Fatalf("Failed to decode response: %v", err)
+ }
// Verify all standard fields are present
if response.RoomID == "" {
@@ -301,7 +303,9 @@ func TestHandleGetRoomState(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
if !createResp.Success {
t.Fatalf("Failed to create test game: %s", createResp.Error)
}
@@ -397,7 +401,7 @@ func TestHandleGetRoomState(t *testing.T) {
req.Header.Set("X-Player-Token", tt.token)
case "bearer":
req.Header.Set("Authorization", "Bearer "+tt.token)
- // "query" case is already in URL
+ // "query" case is already in URL
}
rr := httptest.NewRecorder()
@@ -495,7 +499,9 @@ func TestHandleGetRoomState_ResponseContainsPlayerState(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
// Get room state
req := httptest.NewRequest(http.MethodGet, "/api/rooms/"+createResp.RoomID+"/state", nil)
@@ -505,7 +511,9 @@ func TestHandleGetRoomState_ResponseContainsPlayerState(t *testing.T) {
server.HandleGetRoomState(rr, req)
var response GetRoomStateResponse
- json.NewDecoder(rr.Body).Decode(&response)
+ if err := json.NewDecoder(rr.Body).Decode(&response); err != nil {
+ t.Fatalf("Failed to decode response: %v", err)
+ }
// Verify player object exists in the state
playerState, exists := response.ObjectStates[createResp.PlayerID]
@@ -609,7 +617,9 @@ func TestHandleBotAction(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
if !createResp.Success {
t.Fatalf("Failed to create test game: %s", createResp.Error)
}
@@ -863,7 +873,9 @@ func TestHandleBotAction_KeyCaseInsensitive(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
roomID := createResp.RoomID
playerID := createResp.PlayerID
@@ -933,7 +945,9 @@ func TestHandleResetGame(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
if !createResp.Success {
t.Fatalf("Failed to create test game: %s", createResp.Error)
}
@@ -1117,7 +1131,9 @@ func TestHandleResetGame_ResetsPlayerState(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
roomID := createResp.RoomID
playerID := createResp.PlayerID
@@ -1130,7 +1146,9 @@ func TestHandleResetGame_ResetsPlayerState(t *testing.T) {
server.HandleGetRoomState(getStateRR, getStateReq)
var initialState GetRoomStateResponse
- json.NewDecoder(getStateRR.Body).Decode(&initialState)
+ if err := json.NewDecoder(getStateRR.Body).Decode(&initialState); err != nil {
+ t.Fatalf("Failed to decode initial state response: %v", err)
+ }
initialPlayerState := initialState.ObjectStates[playerID]
initialX := initialPlayerState["x"].(float64)
initialY := initialPlayerState["y"].(float64)
@@ -1147,7 +1165,16 @@ func TestHandleResetGame_ResetsPlayerState(t *testing.T) {
botActionReq.Header.Set("Authorization", "Bearer "+playerToken)
server.HandleBotAction(httptest.NewRecorder(), botActionReq)
- // Reset the game
+ // Get the room and player object for direct state verification
+ room, exists := server.GetRoom(roomID)
+ if !exists {
+ t.Fatal("Room should exist")
+ }
+
+ // Stop the tick loop to prevent background ticks from modifying state
+ room.StopTickLoop()
+
+ // Reset the game via HTTP API
resetReq := httptest.NewRequest(http.MethodPost, "/api/rooms/"+roomID+"/reset", nil)
resetReq.Header.Set("X-Player-Token", playerToken)
resetRR := httptest.NewRecorder()
@@ -1157,50 +1184,45 @@ func TestHandleResetGame_ResetsPlayerState(t *testing.T) {
t.Fatalf("HandleResetGame() failed with status %v", resetRR.Code)
}
- // Get state after reset
- getStateReq2 := httptest.NewRequest(http.MethodGet, "/api/rooms/"+roomID+"/state", nil)
- getStateReq2.Header.Set("X-Player-Token", playerToken)
- getStateRR2 := httptest.NewRecorder()
- server.HandleGetRoomState(getStateRR2, getStateReq2)
-
- var resetState GetRoomStateResponse
- json.NewDecoder(getStateRR2.Body).Decode(&resetState)
- resetPlayerState := resetState.ObjectStates[playerID]
-
- // Verify player is still in the game
- if resetPlayerState == nil {
+ // Verify player state is reset by checking raw state values directly
+ // Note: We use GetStateValue() instead of the HTTP API's GetState() because
+ // PlayerGameObject.GetState() applies extrapolation (including gravity) based on
+ // time elapsed since lastLocUpdateTime, which causes flaky tests.
+ playerObj, exists := room.ObjectManager.GetObject(playerID)
+ if !exists {
t.Fatal("Player should still exist after reset")
}
// Verify dx and dy are reset to 0
- if dx, ok := resetPlayerState["dx"].(float64); !ok || dx != 0.0 {
- t.Errorf("Player dx after reset = %v, want 0", resetPlayerState["dx"])
+ if dx, exists := playerObj.GetStateValue("dx"); !exists || dx.(float64) != 0.0 {
+ t.Errorf("Player dx after reset = %v, want 0", dx)
}
- if dy, ok := resetPlayerState["dy"].(float64); !ok || dy != 0.0 {
- t.Errorf("Player dy after reset = %v, want 0", resetPlayerState["dy"])
+ if dy, exists := playerObj.GetStateValue("dy"); !exists || dy.(float64) != 0.0 {
+ t.Errorf("Player dy after reset = %v, want 0", dy)
}
// Verify player is not dead
- if dead, ok := resetPlayerState["dead"].(bool); !ok || dead {
- t.Errorf("Player dead after reset = %v, want false", resetPlayerState["dead"])
+ if dead, exists := playerObj.GetStateValue("dead"); !exists || dead.(bool) {
+ t.Errorf("Player dead after reset = %v, want false", dead)
}
// Verify arrows are reset to starting count (4)
- // Note: The state key is "ac" for arrow count, and it comes as float64 from JSON
- if arrowCount, ok := resetPlayerState["ac"].(float64); !ok || arrowCount != 4.0 {
- t.Errorf("Player arrows after reset = %v, want 4", resetPlayerState["ac"])
+ if arrowCount, exists := playerObj.GetStateValue("ac"); !exists || arrowCount.(int) != 4 {
+ t.Errorf("Player arrows after reset = %v, want 4", arrowCount)
}
- // Note: Position may have changed to a new respawn location, so we just verify it's set
- if _, ok := resetPlayerState["x"].(float64); !ok {
+ // Verify position is set (may have changed to a new respawn location)
+ resetX, xExists := playerObj.GetStateValue("x")
+ resetY, yExists := playerObj.GetStateValue("y")
+ if !xExists {
t.Error("Player x position should be set after reset")
}
- if _, ok := resetPlayerState["y"].(float64); !ok {
+ if !yExists {
t.Error("Player y position should be set after reset")
}
// Log initial and reset positions for informational purposes
- t.Logf("Initial position: (%v, %v), Reset position: (%v, %v)", initialX, initialY, resetPlayerState["x"], resetPlayerState["y"])
+ t.Logf("Initial position: (%v, %v), Reset position: (%v, %v)", initialX, initialY, resetX, resetY)
}
func TestHandleGetRoomStats(t *testing.T) {
@@ -1219,7 +1241,9 @@ func TestHandleGetRoomStats(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
if !createResp.Success {
t.Fatalf("Failed to create test game: %s", createResp.Error)
}
@@ -1315,7 +1339,7 @@ func TestHandleGetRoomStats(t *testing.T) {
req.Header.Set("X-Player-Token", tt.token)
case "bearer":
req.Header.Set("Authorization", "Bearer "+tt.token)
- // "query" case is already in URL
+ // "query" case is already in URL
}
rr := httptest.NewRecorder()
@@ -1410,7 +1434,9 @@ func TestHandleGetRoomStats_ReturnsPlayerStats(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
roomID := createResp.RoomID
playerID := createResp.PlayerID
@@ -1423,7 +1449,9 @@ func TestHandleGetRoomStats_ReturnsPlayerStats(t *testing.T) {
server.HandleGetRoomStats(rr, req)
var response GetRoomStatsHTTPResponse
- json.NewDecoder(rr.Body).Decode(&response)
+ if err := json.NewDecoder(rr.Body).Decode(&response); err != nil {
+ t.Fatalf("Failed to decode response: %v", err)
+ }
if !response.Success {
t.Fatalf("HandleGetRoomStats() failed: %s", response.Error)
@@ -1466,7 +1494,9 @@ func TestHandleGetRoomStats_WithMultiplePlayers(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
roomID := createResp.RoomID
player1Token := createResp.PlayerToken
@@ -1488,7 +1518,9 @@ func TestHandleGetRoomStats_WithMultiplePlayers(t *testing.T) {
server.HandleGetRoomStats(rr, req)
var response GetRoomStatsHTTPResponse
- json.NewDecoder(rr.Body).Decode(&response)
+ if err := json.NewDecoder(rr.Body).Decode(&response); err != nil {
+ t.Fatalf("Failed to decode response: %v", err)
+ }
if !response.Success {
t.Fatalf("HandleGetRoomStats() failed: %s", response.Error)
@@ -1541,7 +1573,9 @@ func TestHandleGetRoomStats_StatsResetAfterGameReset(t *testing.T) {
server.HandleCreateGame(createRR, createHttpReq)
var createResp CreateGameHTTPResponse
- json.NewDecoder(createRR.Body).Decode(&createResp)
+ if err := json.NewDecoder(createRR.Body).Decode(&createResp); err != nil {
+ t.Fatalf("Failed to decode create game response: %v", err)
+ }
roomID := createResp.RoomID
playerID := createResp.PlayerID
@@ -1560,7 +1594,9 @@ func TestHandleGetRoomStats_StatsResetAfterGameReset(t *testing.T) {
server.HandleGetRoomStats(rr1, req1)
var response1 GetRoomStatsHTTPResponse
- json.NewDecoder(rr1.Body).Decode(&response1)
+ if err := json.NewDecoder(rr1.Body).Decode(&response1); err != nil {
+ t.Fatalf("Failed to decode response: %v", err)
+ }
if response1.PlayerStats[playerID].Kills != 2 || response1.PlayerStats[playerID].Deaths != 1 {
t.Errorf("Stats before reset: kills=%d deaths=%d, want kills=2 deaths=1",
@@ -1579,7 +1615,9 @@ func TestHandleGetRoomStats_StatsResetAfterGameReset(t *testing.T) {
server.HandleGetRoomStats(rr2, req2)
var response2 GetRoomStatsHTTPResponse
- json.NewDecoder(rr2.Body).Decode(&response2)
+ if err := json.NewDecoder(rr2.Body).Decode(&response2); err != nil {
+ t.Fatalf("Failed to decode response: %v", err)
+ }
if response2.PlayerStats[playerID].Kills != 0 || response2.PlayerStats[playerID].Deaths != 0 {
t.Errorf("Stats after reset: kills=%d deaths=%d, want kills=0 deaths=0",
diff --git a/backend/pkg/server/http_types.go b/backend/pkg/server/http_types.go
index a949f1e..4ecd0c4 100644
--- a/backend/pkg/server/http_types.go
+++ b/backend/pkg/server/http_types.go
@@ -18,13 +18,13 @@ type CreateGameRequest struct {
}
type CreateGameResponse struct {
- RoomID string `json:"room_id"`
- RoomCode string `json:"room_code"`
- RoomName string `json:"room_name"`
- PlayerID string `json:"player_id"`
- PlayerToken string `json:"player_token"`
- CanvasSizeX int `json:"canvas_size_x"`
- CanvasSizeY int `json:"canvas_size_y"`
+ RoomID string `json:"room_id"`
+ RoomCode string `json:"room_code"`
+ RoomName string `json:"room_name"`
+ PlayerID string `json:"player_id"`
+ PlayerToken string `json:"player_token"`
+ CanvasSizeX int `json:"canvas_size_x"`
+ CanvasSizeY int `json:"canvas_size_y"`
}
type JoinGameRequest struct {
@@ -33,12 +33,12 @@ type JoinGameRequest struct {
}
type JoinGameResponse struct {
- RoomID string `json:"room_id"`
- RoomName string `json:"room_name"`
- PlayerID string `json:"player_id"`
- PlayerToken string `json:"player_token"`
- CanvasSizeX int `json:"canvas_size_x"`
- CanvasSizeY int `json:"canvas_size_y"`
+ RoomID string `json:"room_id"`
+ RoomName string `json:"room_name"`
+ PlayerID string `json:"player_id"`
+ PlayerToken string `json:"player_token"`
+ CanvasSizeX int `json:"canvas_size_x"`
+ CanvasSizeY int `json:"canvas_size_y"`
}
// PlayerStatsDTO represents kill/death statistics for a player in API responses
diff --git a/backend/pkg/server/message_handlers.go b/backend/pkg/server/message_handlers.go
index 6dffbb7..d97d05f 100644
--- a/backend/pkg/server/message_handlers.go
+++ b/backend/pkg/server/message_handlers.go
@@ -273,10 +273,12 @@ func (s *Server) handleExitGame(conn *Connection, _ types.ExitGameRequest) {
conn.WriteMutex.Lock()
conn.RoomID = ""
- conn.connection.WriteJSON(types.Message{
+ if err := conn.connection.WriteJSON(types.Message{
Type: "ExitGameResponse",
Payload: util.Must(json.Marshal(types.ExitGameResponse{Success: true})),
- })
+ }); err != nil {
+ log.Printf("Failed to send ExitGameResponse to connection %s: %v", conn.ID, err)
+ }
conn.WriteMutex.Unlock()
log.Printf("Player %s exited game room %s", player.ID, room.ID)
@@ -314,10 +316,12 @@ func (s *Server) updateConnectionActivity(conn *Connection) {
func (s *Server) sendErrorMessage(conn *Connection, message string) {
conn.WriteMutex.Lock()
- conn.connection.WriteJSON(types.Message{
+ if err := conn.connection.WriteJSON(types.Message{
Type: "ErrorMessage",
Payload: util.Must(json.Marshal(types.ErrorMessage{Message: message})),
- })
+ }); err != nil {
+ log.Printf("Failed to send ErrorMessage to connection %s: %v", conn.ID, err)
+ }
conn.WriteMutex.Unlock()
}
diff --git a/backend/pkg/server/server.go b/backend/pkg/server/server.go
index 6606c7e..0ba0d3e 100644
--- a/backend/pkg/server/server.go
+++ b/backend/pkg/server/server.go
@@ -219,7 +219,9 @@ func (s *Server) runProcessGameUpdateQueue() {
func (s *Server) processGameUpdateQueue() {
update := <-s.gameStateUpdateQueue
- go s.sendGameUpdate(update)
+ go func() {
+ _ = s.sendGameUpdate(update)
+ }()
}
// sendGameUpdate sends the game state update to all connections to the room
@@ -282,10 +284,8 @@ func (s *Server) sendGameUpdate(update GameUpdateQueueItem) error {
})
conn.WriteMutex.Unlock()
- if err != nil {
- // TODO better error handling - remove dead connections, etc
- //log.Printf("sendGameUpdate:Error sending GameState to connection %s: %v. %v", conn.ID, err, updateMessage)
- }
+ // Ignore write errors - connections may have been closed
+ _ = err
}
return nil
@@ -299,7 +299,9 @@ func (s *Server) runProcessSpectatorUpdateQueue() {
func (s *Server) processSpectatorUpdateQueue() {
update := <-s.spectatorUpdateQueue
- go s.sendSpectatorUpdate(update)
+ go func() {
+ _ = s.sendSpectatorUpdate(update)
+ }()
}
func (s *Server) sendSpectatorUpdate(update SpectatorUpdateQueueItem) error {
@@ -371,7 +373,6 @@ func (s *Server) runCleanupInactiveRooms() {
}
}
-
// cleanupInactiveRooms removes inactive rooms
func (s *Server) cleanupInactiveRooms() {
roomIDs := s.roomManager.GetGameRoomIDs()
@@ -398,3 +399,8 @@ func (s *Server) AddGameRoomAndStartTick(room *GameRoom) {
go s.processEvent(r, event)
})
}
+
+// GetRoom returns a game room by ID. This is primarily intended for testing.
+func (s *Server) GetRoom(roomID string) (*GameRoom, bool) {
+ return s.roomManager.GetGameRoom(roomID)
+}
diff --git a/backend/pkg/server/types/api_types.go b/backend/pkg/server/types/api_types.go
index 600ad79..ea2f5fb 100644
--- a/backend/pkg/server/types/api_types.go
+++ b/backend/pkg/server/types/api_types.go
@@ -86,7 +86,7 @@ type GameUpdate struct {
ObjectStates map[string]map[string]interface{} `json:"objectStates"` // Map of ObjectID -> ObjectState
Events []GameUpdateEvent `json:"events"` // List of events
// Training mode state (only included when training mode is enabled)
- TrainingComplete bool `json:"trainingComplete,omitempty"` // True when training completion conditions are met
+ TrainingComplete bool `json:"trainingComplete,omitempty"` // True when training completion conditions are met
TrainingInfo *TrainingStateInfo `json:"trainingInfo,omitempty"` // Training metadata for spectators
}
diff --git a/bot2/src/bot/cli/main.py b/bot2/src/bot/cli/main.py
index d1ed7c2..827271a 100644
--- a/bot2/src/bot/cli/main.py
+++ b/bot2/src/bot/cli/main.py
@@ -22,7 +22,9 @@
app.add_typer(train_commands.app, name="train", help="Training run management")
app.add_typer(model_commands.app, name="model", help="Model registry operations")
app.add_typer(config_commands.app, name="config", help="Configuration utilities")
-app.add_typer(dashboard_commands.app, name="dashboard", help="Training metrics dashboard")
+app.add_typer(
+ dashboard_commands.app, name="dashboard", help="Training metrics dashboard"
+)
@app.callback()
diff --git a/bot2/src/bot/dashboard/cli.py b/bot2/src/bot/dashboard/cli.py
index 04b454f..7cbd9d3 100644
--- a/bot2/src/bot/dashboard/cli.py
+++ b/bot2/src/bot/dashboard/cli.py
@@ -289,7 +289,12 @@ def compare_generations(
("K/D Ratio", metrics1.kill_death_ratio, metrics2.kill_death_ratio, ".2f"),
("Win Rate", metrics1.win_rate * 100, metrics2.win_rate * 100, ".1f%"),
("Avg Reward", metrics1.avg_episode_reward, metrics2.avg_episode_reward, ".1f"),
- ("Avg Episode Length", metrics1.avg_episode_length, metrics2.avg_episode_length, ".0f"),
+ (
+ "Avg Episode Length",
+ metrics1.avg_episode_length,
+ metrics2.avg_episode_length,
+ ".0f",
+ ),
("Total Episodes", metrics1.total_episodes, metrics2.total_episodes, "d"),
("Total Kills", metrics1.total_kills, metrics2.total_kills, "d"),
("Total Deaths", metrics1.total_deaths, metrics2.total_deaths, "d"),
diff --git a/bot2/src/bot/dashboard/data_aggregator.py b/bot2/src/bot/dashboard/data_aggregator.py
index 7f50771..ec944b2 100644
--- a/bot2/src/bot/dashboard/data_aggregator.py
+++ b/bot2/src/bot/dashboard/data_aggregator.py
@@ -93,9 +93,7 @@ def get_all_generation_metrics(
# Filter by generation range if specified
if generation_range is not None:
start, end = generation_range
- all_metadata = [
- m for m in all_metadata if start <= m.generation <= end
- ]
+ all_metadata = [m for m in all_metadata if start <= m.generation <= end]
# Convert each model's metadata to GenerationMetrics
generation_metrics: list[GenerationMetrics] = []
diff --git a/bot2/src/bot/dashboard/models.py b/bot2/src/bot/dashboard/models.py
index 91a776b..4a4f38e 100644
--- a/bot2/src/bot/dashboard/models.py
+++ b/bot2/src/bot/dashboard/models.py
@@ -44,7 +44,9 @@ class GenerationMetrics(BaseModel):
avg_episode_reward: float = Field(description="Mean episode reward")
avg_episode_length: float = Field(ge=0, description="Mean episode length")
training_steps: int = Field(ge=0, description="Total PPO updates")
- training_duration_seconds: float = Field(ge=0, description="Training time in seconds")
+ training_duration_seconds: float = Field(
+ ge=0, description="Training time in seconds"
+ )
timestamp: datetime = Field(description="Training completion timestamp")
diff --git a/bot2/src/bot/dashboard/visualizer.py b/bot2/src/bot/dashboard/visualizer.py
index 3ebcae9..f29b37a 100644
--- a/bot2/src/bot/dashboard/visualizer.py
+++ b/bot2/src/bot/dashboard/visualizer.py
@@ -264,7 +264,7 @@ def _generate_summary_table(
table_html = f"""
{header_row}
-{''.join(rows)}
+{"".join(rows)}
"""
@@ -451,9 +451,7 @@ def _generate_combined_dashboard(
logger.info("Generated combined dashboard: %s", path)
return path
- def _create_kd_ratio_figure(
- self, metrics: list[GenerationMetrics]
- ) -> Figure:
+ def _create_kd_ratio_figure(self, metrics: list[GenerationMetrics]) -> Figure:
"""Create K/D ratio figure without saving."""
import matplotlib.pyplot as plt
@@ -488,9 +486,7 @@ def _create_kd_ratio_figure(
plt.tight_layout()
return fig
- def _create_win_rate_figure(
- self, metrics: list[GenerationMetrics]
- ) -> Figure:
+ def _create_win_rate_figure(self, metrics: list[GenerationMetrics]) -> Figure:
"""Create win rate figure without saving."""
import matplotlib.pyplot as plt
from matplotlib.patches import Patch
@@ -536,9 +532,7 @@ def _create_win_rate_figure(
plt.tight_layout()
return fig
- def _create_reward_figure(
- self, metrics: list[GenerationMetrics]
- ) -> Figure:
+ def _create_reward_figure(self, metrics: list[GenerationMetrics]) -> Figure:
"""Create reward figure without saving."""
import matplotlib.pyplot as plt
@@ -599,6 +593,6 @@ def _build_table_html(self, table_data: list[dict]) -> str:
return f"""
{header_row}
-{''.join(rows)}
+{"".join(rows)}
"""
diff --git a/bot2/src/bot/gym/model_opponent.py b/bot2/src/bot/gym/model_opponent.py
index a0085de..03e6614 100644
--- a/bot2/src/bot/gym/model_opponent.py
+++ b/bot2/src/bot/gym/model_opponent.py
@@ -154,7 +154,11 @@ async def on_game_state(self, state: GameState) -> None:
Args:
state: Current game state.
"""
- if not self._running or self._client is None or self._observation_builder is None:
+ if (
+ not self._running
+ or self._client is None
+ or self._observation_builder is None
+ ):
return
try:
diff --git a/bot2/src/bot/observation/observation_space.py b/bot2/src/bot/observation/observation_space.py
index 268e0c0..44575e2 100644
--- a/bot2/src/bot/observation/observation_space.py
+++ b/bot2/src/bot/observation/observation_space.py
@@ -271,8 +271,12 @@ def _normalize_own_player(
# Calculate power ratio based on how long the shot has been held
# Power ratio goes from 0 to 1 over MAX_ARROW_POWER_TIME seconds
if player.shooting and player.shooting_start_time is not None:
- elapsed_time = time.time() - player.shooting_start_time
- power_ratio = min(1.0, elapsed_time / self.constants.MAX_ARROW_POWER_TIME)
+ # Backend sends shooting_start_time in milliseconds, convert to seconds
+ current_time_ms = time.time() * 1000.0
+ elapsed_time = (current_time_ms - player.shooting_start_time) / 1000.0
+ power_ratio = min(
+ 1.0, max(0.0, elapsed_time / self.constants.MAX_ARROW_POWER_TIME)
+ )
# Normalize to [-1, 1] range: 0 power = -1, max power = 1
obs[10] = (power_ratio * 2.0) - 1.0
else:
diff --git a/bot2/src/bot/training/evaluation.py b/bot2/src/bot/training/evaluation.py
index e0c1338..51ff519 100644
--- a/bot2/src/bot/training/evaluation.py
+++ b/bot2/src/bot/training/evaluation.py
@@ -103,9 +103,7 @@ def from_episodes(
win_rate = total_wins / num_episodes
# Compute per-episode K/D for standard deviation
- per_episode_kd = [
- k / max(d, 1) for k, d in zip(episode_kills, episode_deaths)
- ]
+ per_episode_kd = [k / max(d, 1) for k, d in zip(episode_kills, episode_deaths)]
kd_ratio_std = float(np.std(per_episode_kd)) if len(per_episode_kd) > 1 else 0.0
# Win rate standard deviation
@@ -237,9 +235,8 @@ def compare_to_baseline(
# Statistical significance test (one-sample t-test against opponent K/D)
# H0: agent_kd <= opponent_kd, H1: agent_kd > opponent_kd
if agent_eval.kd_ratio_std > 0 and agent_eval.total_episodes > 1:
- t_stat = (
- (agent_eval.kd_ratio - opponent_kd)
- / (agent_eval.kd_ratio_std / np.sqrt(agent_eval.total_episodes))
+ t_stat = (agent_eval.kd_ratio - opponent_kd) / (
+ agent_eval.kd_ratio_std / np.sqrt(agent_eval.total_episodes)
)
p_value = float(1 - stats.t.cdf(t_stat, df=agent_eval.total_episodes - 1))
else:
diff --git a/bot2/src/bot/training/orchestrator.py b/bot2/src/bot/training/orchestrator.py
index 63a94f0..1279924 100644
--- a/bot2/src/bot/training/orchestrator.py
+++ b/bot2/src/bot/training/orchestrator.py
@@ -454,7 +454,10 @@ def load_checkpoint(self, checkpoint_path: str) -> None:
self.trainer.optimizer.load_state_dict(checkpoint["optimizer_state_dict"])
self.total_timesteps = checkpoint["total_timesteps"]
self.num_updates = checkpoint["num_updates"]
- self.current_generation = checkpoint["generation"]
+ # Note: We intentionally do NOT restore current_generation from the checkpoint.
+ # When training completes, we register a NEW model with the next available generation
+ # from the registry, not the generation from when the checkpoint was saved.
+ # The generation determined in setup() via get_next_generation() should be used.
# Update trainer's internal counters
self.trainer.total_timesteps = self.total_timesteps
diff --git a/bot2/src/bot/training/successive_trainer.py b/bot2/src/bot/training/successive_trainer.py
index 3da3fd6..e6eb004 100644
--- a/bot2/src/bot/training/successive_trainer.py
+++ b/bot2/src/bot/training/successive_trainer.py
@@ -404,10 +404,14 @@ def _log_generation_result(self, result: GenerationResult) -> None:
if result.final_evaluation:
logger.info(" Final K/D: %.2f", result.final_evaluation.kd_ratio)
- logger.info(" Final Win Rate: %.1f%%", result.final_evaluation.win_rate * 100)
+ logger.info(
+ " Final Win Rate: %.1f%%", result.final_evaluation.win_rate * 100
+ )
if result.comparison:
- logger.info(" K/D Improvement: %.1f%%", result.comparison.kd_improvement * 100)
+ logger.info(
+ " K/D Improvement: %.1f%%", result.comparison.kd_improvement * 100
+ )
logger.info(" Promoted: %s", result.was_promoted)
logger.info(" Reason: %s", result.promotion_reason)
@@ -429,9 +433,7 @@ def _log_training_summary(self) -> None:
r.final_evaluation.kd_ratio if r.final_evaluation else 0
),
)
- logger.info(
- "Best Model: %s (Gen %d)", best.model_id, best.generation
- )
+ logger.info("Best Model: %s (Gen %d)", best.model_id, best.generation)
total_timesteps = sum(r.timesteps_trained for r in self.generation_results)
logger.info("Total Timesteps: %s", f"{total_timesteps:,}")
diff --git a/bot2/tests/integration/training/test_dashboard_integration.py b/bot2/tests/integration/training/test_dashboard_integration.py
index 02bfd3d..bab887b 100644
--- a/bot2/tests/integration/training/test_dashboard_integration.py
+++ b/bot2/tests/integration/training/test_dashboard_integration.py
@@ -278,11 +278,36 @@ def test_load_jsonl_metrics_integration(
jsonl_file = temp_metrics_path / "metrics.jsonl"
metrics_entries = [
- {"tag": "episode/reward", "value": 10.0, "step": 1, "timestamp": "2025-01-18T10:00:00"},
- {"tag": "episode/reward", "value": 15.0, "step": 2, "timestamp": "2025-01-18T10:00:01"},
- {"tag": "episode/reward", "value": 20.0, "step": 3, "timestamp": "2025-01-18T10:00:02"},
- {"tag": "episode/length", "value": 450, "step": 1, "timestamp": "2025-01-18T10:00:00"},
- {"tag": "episode/kills", "value": 2, "step": 1, "timestamp": "2025-01-18T10:00:00"},
+ {
+ "tag": "episode/reward",
+ "value": 10.0,
+ "step": 1,
+ "timestamp": "2025-01-18T10:00:00",
+ },
+ {
+ "tag": "episode/reward",
+ "value": 15.0,
+ "step": 2,
+ "timestamp": "2025-01-18T10:00:01",
+ },
+ {
+ "tag": "episode/reward",
+ "value": 20.0,
+ "step": 3,
+ "timestamp": "2025-01-18T10:00:02",
+ },
+ {
+ "tag": "episode/length",
+ "value": 450,
+ "step": 1,
+ "timestamp": "2025-01-18T10:00:00",
+ },
+ {
+ "tag": "episode/kills",
+ "value": 2,
+ "step": 1,
+ "timestamp": "2025-01-18T10:00:00",
+ },
]
with open(jsonl_file, "w") as f:
@@ -379,9 +404,12 @@ def test_cli_generate_command(
app,
[
"generate",
- "-r", str(temp_registry_path),
- "-o", str(temp_output_path),
- "-f", "html",
+ "-r",
+ str(temp_registry_path),
+ "-o",
+ str(temp_output_path),
+ "-f",
+ "html",
],
)
@@ -427,9 +455,12 @@ def test_cli_generate_with_generation_range(
app,
[
"generate",
- "-r", str(temp_registry_path),
- "-o", str(temp_output_path),
- "-g", "0-1",
+ "-r",
+ str(temp_registry_path),
+ "-o",
+ str(temp_output_path),
+ "-g",
+ "0-1",
],
)
@@ -467,9 +498,12 @@ def test_cli_error_invalid_generation_range(
app,
[
"generate",
- "-r", str(temp_registry_path),
- "-o", str(temp_output_path),
- "-g", "invalid",
+ "-r",
+ str(temp_registry_path),
+ "-o",
+ str(temp_output_path),
+ "-g",
+ "invalid",
],
)
@@ -515,8 +549,10 @@ def test_empty_registry_generate(
app,
[
"generate",
- "-r", str(temp_registry_path),
- "-o", str(temp_output_path),
+ "-r",
+ str(temp_registry_path),
+ "-o",
+ str(temp_output_path),
],
)
diff --git a/bot2/tests/integration/training/test_orchestrator_integration.py b/bot2/tests/integration/training/test_orchestrator_integration.py
index dea547c..f70a697 100644
--- a/bot2/tests/integration/training/test_orchestrator_integration.py
+++ b/bot2/tests/integration/training/test_orchestrator_integration.py
@@ -128,6 +128,7 @@ async def test_context_manager_setup_and_cleanup(
@requires_server
@pytest.mark.asyncio
+ @pytest.mark.slow
async def test_short_training_completes(
self, orchestrator_config_smoke: OrchestratorConfig
) -> None:
@@ -289,6 +290,7 @@ async def test_checkpoint_load_and_resume(
@pytest.mark.integration
+@pytest.mark.slow
class TestOrchestratorRegistry:
"""Tests for model registry integration."""
diff --git a/bot2/tests/integration/training/test_successive_training_integration.py b/bot2/tests/integration/training/test_successive_training_integration.py
index c80eea7..146ec0a 100644
--- a/bot2/tests/integration/training/test_successive_training_integration.py
+++ b/bot2/tests/integration/training/test_successive_training_integration.py
@@ -88,10 +88,10 @@ def successive_config_short(tmp_path: Path) -> SuccessiveTrainingConfig:
evaluation_episodes=2,
promotion_criteria=PromotionCriteria(
min_kd_ratio=0.0, # Permissive for testing
- kd_improvement=0.0,
+ kd_improvement=-2.0, # Always pass: agent KD 0 vs opponent KD 1 = -1.0 improvement
min_eval_episodes=1,
consecutive_passes=1,
- confidence_threshold=0.5,
+ confidence_threshold=0.0, # No statistical significance required
),
max_stagnant_evaluations=3,
output_dir=str(tmp_path / "successive"),
@@ -137,6 +137,7 @@ async def test_context_manager_setup_and_cleanup(
@requires_server
@pytest.mark.asyncio
+ @pytest.mark.slow
async def test_single_generation_completes(
self, successive_config_smoke: SuccessiveTrainingConfig
) -> None:
@@ -251,9 +252,7 @@ def callback(event: dict[str, Any]) -> None:
await trainer.train()
# Should have generation_complete events
- gen_events = [
- e for e in events_received if e["type"] == "generation_complete"
- ]
+ gen_events = [e for e in events_received if e["type"] == "generation_complete"]
assert len(gen_events) == 2
# Check generation numbers
@@ -338,6 +337,7 @@ async def test_output_directory_structure(
@pytest.mark.integration
+@pytest.mark.slow
class TestSuccessiveTrainerEdgeCases:
"""Tests for edge cases in successive training."""
diff --git a/bot2/tests/unit/cli/test_cli_commands.py b/bot2/tests/unit/cli/test_cli_commands.py
index 33c5f28..a726dd8 100644
--- a/bot2/tests/unit/cli/test_cli_commands.py
+++ b/bot2/tests/unit/cli/test_cli_commands.py
@@ -1,5 +1,6 @@
"""Unit tests for CLI commands."""
+import re
import tempfile
from pathlib import Path
from unittest.mock import patch
@@ -12,6 +13,12 @@
runner = CliRunner()
+def strip_ansi(text: str) -> str:
+ """Remove ANSI escape codes from text."""
+ ansi_escape = re.compile(r"\x1b\[[0-9;]*m")
+ return ansi_escape.sub("", text)
+
+
class TestMainApp:
"""Tests for the main CLI application."""
@@ -45,8 +52,9 @@ def test_train_start_help(self) -> None:
"""Test train start --help works."""
result = runner.invoke(app, ["train", "start", "--help"])
assert result.exit_code == 0
- assert "--config" in result.stdout
- assert "--timesteps" in result.stdout
+ stdout = strip_ansi(result.stdout)
+ assert "--config" in stdout
+ assert "--timesteps" in stdout
def test_train_list_empty(self) -> None:
"""Test train list with no runs."""
diff --git a/bot2/tests/unit/dashboard/test_data_aggregator.py b/bot2/tests/unit/dashboard/test_data_aggregator.py
index 0ccf12b..51b268d 100644
--- a/bot2/tests/unit/dashboard/test_data_aggregator.py
+++ b/bot2/tests/unit/dashboard/test_data_aggregator.py
@@ -154,9 +154,7 @@ def test_from_config(self, temp_registry_path: Path):
assert aggregator.registry_path == Path(temp_registry_path)
assert aggregator.metrics_dir == Path("/some/metrics")
- def test_get_all_generation_metrics_empty_registry(
- self, temp_registry_path: Path
- ):
+ def test_get_all_generation_metrics_empty_registry(self, temp_registry_path: Path):
"""Test getting metrics from empty registry."""
# Create empty registry
ModelRegistry(temp_registry_path)
@@ -299,9 +297,39 @@ def test_parse_jsonl_file(self, temp_registry_path: Path):
with tempfile.TemporaryDirectory() as metrics_dir:
jsonl_path = Path(metrics_dir) / "metrics.jsonl"
with open(jsonl_path, "w") as f:
- f.write(json.dumps({"tag": "episode/reward", "value": 10.5, "step": 1, "timestamp": "2025-01-18T10:00:00"}) + "\n")
- f.write(json.dumps({"tag": "episode/length", "value": 500, "step": 1, "timestamp": "2025-01-18T10:00:00"}) + "\n")
- f.write(json.dumps({"tag": "episode/reward", "value": 15.0, "step": 2, "timestamp": "2025-01-18T10:00:01"}) + "\n")
+ f.write(
+ json.dumps(
+ {
+ "tag": "episode/reward",
+ "value": 10.5,
+ "step": 1,
+ "timestamp": "2025-01-18T10:00:00",
+ }
+ )
+ + "\n"
+ )
+ f.write(
+ json.dumps(
+ {
+ "tag": "episode/length",
+ "value": 500,
+ "step": 1,
+ "timestamp": "2025-01-18T10:00:00",
+ }
+ )
+ + "\n"
+ )
+ f.write(
+ json.dumps(
+ {
+ "tag": "episode/reward",
+ "value": 15.0,
+ "step": 2,
+ "timestamp": "2025-01-18T10:00:01",
+ }
+ )
+ + "\n"
+ )
aggregator = DataAggregator(
registry_path=temp_registry_path,
@@ -346,10 +374,50 @@ def test_get_reward_progression(self, temp_registry_path: Path):
with tempfile.TemporaryDirectory() as metrics_dir:
jsonl_path = Path(metrics_dir) / "metrics.jsonl"
with open(jsonl_path, "w") as f:
- f.write(json.dumps({"tag": "episode/reward", "value": 10.0, "step": 1, "timestamp": "2025-01-18T10:00:00"}) + "\n")
- f.write(json.dumps({"tag": "episode/length", "value": 500, "step": 1, "timestamp": "2025-01-18T10:00:00"}) + "\n")
- f.write(json.dumps({"tag": "episode/reward", "value": 20.0, "step": 2, "timestamp": "2025-01-18T10:00:01"}) + "\n")
- f.write(json.dumps({"tag": "episode/reward", "value": 15.0, "step": 3, "timestamp": "2025-01-18T10:00:02"}) + "\n")
+ f.write(
+ json.dumps(
+ {
+ "tag": "episode/reward",
+ "value": 10.0,
+ "step": 1,
+ "timestamp": "2025-01-18T10:00:00",
+ }
+ )
+ + "\n"
+ )
+ f.write(
+ json.dumps(
+ {
+ "tag": "episode/length",
+ "value": 500,
+ "step": 1,
+ "timestamp": "2025-01-18T10:00:00",
+ }
+ )
+ + "\n"
+ )
+ f.write(
+ json.dumps(
+ {
+ "tag": "episode/reward",
+ "value": 20.0,
+ "step": 2,
+ "timestamp": "2025-01-18T10:00:01",
+ }
+ )
+ + "\n"
+ )
+ f.write(
+ json.dumps(
+ {
+ "tag": "episode/reward",
+ "value": 15.0,
+ "step": 3,
+ "timestamp": "2025-01-18T10:00:02",
+ }
+ )
+ + "\n"
+ )
aggregator = DataAggregator(
registry_path=temp_registry_path,
diff --git a/bot2/tests/unit/dashboard/test_visualizer.py b/bot2/tests/unit/dashboard/test_visualizer.py
index 79ad588..bbfc187 100644
--- a/bot2/tests/unit/dashboard/test_visualizer.py
+++ b/bot2/tests/unit/dashboard/test_visualizer.py
@@ -197,7 +197,9 @@ def test_generate_with_generation_range(self, mock_aggregator: MagicMock):
# Modify mock to return filtered results
filtered_metrics = mock_aggregator.get_all_generation_metrics.return_value[:2]
mock_aggregator.get_all_generation_metrics.side_effect = lambda r=None: (
- filtered_metrics if r == (0, 1) else mock_aggregator.get_all_generation_metrics.return_value
+ filtered_metrics
+ if r == (0, 1)
+ else mock_aggregator.get_all_generation_metrics.return_value
)
with tempfile.TemporaryDirectory() as tmpdir:
@@ -214,7 +216,9 @@ def test_generate_with_generation_range(self, mock_aggregator: MagicMock):
assert len(files) > 0
def test_kd_ratio_chart_content(
- self, mock_aggregator: MagicMock, sample_generation_metrics: list[GenerationMetrics]
+ self,
+ mock_aggregator: MagicMock,
+ sample_generation_metrics: list[GenerationMetrics],
):
"""Test that K/D ratio chart has correct data."""
with tempfile.TemporaryDirectory() as tmpdir:
@@ -238,7 +242,9 @@ def test_kd_ratio_chart_content(
assert "data:image/png;base64," in content
def test_win_rate_chart_content(
- self, mock_aggregator: MagicMock, sample_generation_metrics: list[GenerationMetrics]
+ self,
+ mock_aggregator: MagicMock,
+ sample_generation_metrics: list[GenerationMetrics],
):
"""Test that win rate chart has correct data."""
with tempfile.TemporaryDirectory() as tmpdir:
@@ -259,7 +265,9 @@ def test_win_rate_chart_content(
assert "data:image/png;base64," in content
def test_reward_chart_content(
- self, mock_aggregator: MagicMock, sample_generation_metrics: list[GenerationMetrics]
+ self,
+ mock_aggregator: MagicMock,
+ sample_generation_metrics: list[GenerationMetrics],
):
"""Test that reward chart has correct data."""
with tempfile.TemporaryDirectory() as tmpdir:
@@ -280,7 +288,9 @@ def test_reward_chart_content(
assert "data:image/png;base64," in content
def test_summary_table_content(
- self, mock_aggregator: MagicMock, sample_generation_metrics: list[GenerationMetrics]
+ self,
+ mock_aggregator: MagicMock,
+ sample_generation_metrics: list[GenerationMetrics],
):
"""Test that summary table has correct content."""
with tempfile.TemporaryDirectory() as tmpdir:
@@ -304,7 +314,9 @@ def test_summary_table_content(
assert "Win Rate" in content
def test_combined_dashboard_structure(
- self, mock_aggregator: MagicMock, sample_generation_metrics: list[GenerationMetrics]
+ self,
+ mock_aggregator: MagicMock,
+ sample_generation_metrics: list[GenerationMetrics],
):
"""Test that combined dashboard has correct structure."""
with tempfile.TemporaryDirectory() as tmpdir:
diff --git a/bot2/tests/unit/training/test_orchestrator.py b/bot2/tests/unit/training/test_orchestrator.py
index 9bc9d5a..382d776 100644
--- a/bot2/tests/unit/training/test_orchestrator.py
+++ b/bot2/tests/unit/training/test_orchestrator.py
@@ -233,7 +233,13 @@ def test_save_checkpoint_contains_state(
def test_load_checkpoint_restores_state(
self, orchestrator_with_network: TrainingOrchestrator, tmp_path: Path
) -> None:
- """Test load_checkpoint restores training state."""
+ """Test load_checkpoint restores training state.
+
+ Note: current_generation is intentionally NOT restored from the checkpoint.
+ When training resumes and completes, we register a NEW model with the next
+ available generation from the registry, not the generation from when the
+ checkpoint was saved.
+ """
orchestrator = orchestrator_with_network
Path(orchestrator.config.checkpoint_dir).mkdir(parents=True, exist_ok=True)
@@ -242,7 +248,7 @@ def test_load_checkpoint_restores_state(
# Reset state
orchestrator.total_timesteps = 0
orchestrator.num_updates = 0
- orchestrator.current_generation = 0
+ original_generation = orchestrator.current_generation
checkpoint_path = str(
Path(orchestrator.config.checkpoint_dir) / "checkpoint_5000.pt"
@@ -251,7 +257,8 @@ def test_load_checkpoint_restores_state(
assert orchestrator.total_timesteps == 5000
assert orchestrator.num_updates == 10
- assert orchestrator.current_generation == 2
+ # Generation should NOT be restored - it stays at whatever setup() determined
+ assert orchestrator.current_generation == original_generation
def test_load_checkpoint_updates_trainer(
self, orchestrator_with_network: TrainingOrchestrator, tmp_path: Path
diff --git a/bot2/tests/unit/training/test_orchestrator_config.py b/bot2/tests/unit/training/test_orchestrator_config.py
index 0a8b5f7..3850ab9 100644
--- a/bot2/tests/unit/training/test_orchestrator_config.py
+++ b/bot2/tests/unit/training/test_orchestrator_config.py
@@ -387,8 +387,12 @@ def test_roundtrip_to_dict_from_dict(self) -> None:
assert restored.game_config.max_kills == original.game_config.max_kills
assert restored.metrics_config.enabled == original.metrics_config.enabled
assert restored.metrics_config.log_dir == original.metrics_config.log_dir
- assert restored.metrics_config.file_format == original.metrics_config.file_format
- assert restored.metrics_config.window_size == original.metrics_config.window_size
+ assert (
+ restored.metrics_config.file_format == original.metrics_config.file_format
+ )
+ assert (
+ restored.metrics_config.window_size == original.metrics_config.window_size
+ )
class TestOrchestratorConfigYamlSerialization:
diff --git a/bot2/tests/unit/training/test_successive_config.py b/bot2/tests/unit/training/test_successive_config.py
index 1070448..06ac59c 100644
--- a/bot2/tests/unit/training/test_successive_config.py
+++ b/bot2/tests/unit/training/test_successive_config.py
@@ -248,7 +248,9 @@ def test_generation_config_basic(self) -> None:
output_dir="/test/output",
)
- gen_config = config.create_generation_config(generation=0, opponent_model_id=None)
+ gen_config = config.create_generation_config(
+ generation=0, opponent_model_id=None
+ )
assert gen_config.num_envs == 4
assert gen_config.total_timesteps == 100_000
@@ -271,8 +273,12 @@ def test_generation_config_with_seed(self) -> None:
"""Test generation config with base seed."""
config = SuccessiveTrainingConfig(base_seed=42)
- gen_config_0 = config.create_generation_config(generation=0, opponent_model_id=None)
- gen_config_1 = config.create_generation_config(generation=1, opponent_model_id=None)
+ gen_config_0 = config.create_generation_config(
+ generation=0, opponent_model_id=None
+ )
+ gen_config_1 = config.create_generation_config(
+ generation=1, opponent_model_id=None
+ )
assert gen_config_0.seed == 42
assert gen_config_1.seed == 43
@@ -281,16 +287,22 @@ def test_generation_config_no_seed(self) -> None:
"""Test generation config without base seed."""
config = SuccessiveTrainingConfig(base_seed=None)
- gen_config = config.create_generation_config(generation=0, opponent_model_id=None)
+ gen_config = config.create_generation_config(
+ generation=0, opponent_model_id=None
+ )
assert gen_config.seed is None
def test_generation_config_preserves_ppo_config(self) -> None:
"""Test that PPO config is preserved in generation config."""
ppo = PPOConfig(num_steps=1024, learning_rate=1e-4)
- config = SuccessiveTrainingConfig(base_config=OrchestratorConfig(ppo_config=ppo))
+ config = SuccessiveTrainingConfig(
+ base_config=OrchestratorConfig(ppo_config=ppo)
+ )
- gen_config = config.create_generation_config(generation=0, opponent_model_id=None)
+ gen_config = config.create_generation_config(
+ generation=0, opponent_model_id=None
+ )
assert gen_config.ppo_config.num_steps == 1024
assert gen_config.ppo_config.learning_rate == 1e-4
@@ -302,7 +314,9 @@ def test_generation_config_preserves_game_config(self) -> None:
base_config=OrchestratorConfig(game_config=game)
)
- gen_config = config.create_generation_config(generation=0, opponent_model_id=None)
+ gen_config = config.create_generation_config(
+ generation=0, opponent_model_id=None
+ )
assert gen_config.game_config.room_name == "Test"
assert gen_config.game_config.tick_multiplier == 20.0
@@ -377,9 +391,7 @@ def test_roundtrip_to_dict_from_dict(self) -> None:
restored = SuccessiveTrainingConfig.from_dict(data)
assert restored.max_generations == original.max_generations
- assert (
- restored.timesteps_per_generation == original.timesteps_per_generation
- )
+ assert restored.timesteps_per_generation == original.timesteps_per_generation
assert restored.base_seed == original.base_seed
assert restored.base_config.num_envs == original.base_config.num_envs
assert (
@@ -442,9 +454,7 @@ def test_roundtrip_yaml(self, tmp_path: Path) -> None:
restored = SuccessiveTrainingConfig.from_yaml(yaml_path)
assert restored.max_generations == original.max_generations
- assert (
- restored.timesteps_per_generation == original.timesteps_per_generation
- )
+ assert restored.timesteps_per_generation == original.timesteps_per_generation
assert restored.base_seed == original.base_seed
assert restored.base_config.num_envs == original.base_config.num_envs
assert (