fix: agility course creation fixed and improved in admin web gui

1 files changed, 107 insertions, 140 deletions

diff --git a/internal/admin/api_room_courses.go b/internal/admin/api_room_courses.go
index 740a102..e4cf849 100644
--- a/internal/admin/api_room_courses.go
+++ b/internal/admin/api_room_courses.go
@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"gopkg.in/yaml.v3"
@@ -229,8 +230,13 @@ func (s *AdminServer) handleCourseByID(w http.ResponseWriter, r *http.Request) {
 			NewContent:  newContent,
 			IsCreate:    isCreate,
 		})
+		warnings := s.reconcileCourseExits(body)
 		s.reloadCoursesIfWired()
-		writeJSON(w, map[string]any{"ok": true, "id": id})
+		resp := map[string]any{"ok": true, "id": id}
+		if len(warnings) > 0 {
+			resp["warnings"] = warnings
+		}
+		writeJSON(w, resp)
 
 	case http.MethodDelete:
 		oldContent, err := snapshotFile(path)
@@ -366,6 +372,33 @@ func (s *AdminServer) handleRoomAttachCourse(w http.ResponseWriter, r *http.Requ
 	}
 	m["obstacles"] = anyList
 
+	// --- course map exit management ---
+	// Set exit_dir on the previous obstacle.
+	// Append-only: the new obstacle is last so no new->next link is needed.
+	if insertAt > 0 {
+		prevIdx := insertAt - 1
+		prevRoom := obstacleRoomID(newList[prevIdx])
+		if prevRoom > 0 {
+			dir := strings.TrimSpace(strings.ToLower(body.Direction))
+			if dir == "auto" || dir == "" {
+				dir = "north"
+			}
+			if !isHorizontalDir(dir) {
+				writeJSON(w, map[string]any{"error": "invalid direction: " + dir})
+				return
+			}
+
+			prevObstacle := newList[prevIdx]
+			prevObstacle["exit_dir"] = dir
+			newList[prevIdx] = prevObstacle
+		}
+	}
+	anyList = make([]any, len(newList))
+	for i, o := range newList {
+		anyList[i] = o
+	}
+	m["obstacles"] = anyList
+
 	newContent, err := writeMapAsYAML(path, m)
 	if err != nil {
 		writeJSON(w, map[string]any{"error": "write error: " + err.Error()})
@@ -378,55 +411,18 @@ func (s *AdminServer) handleRoomAttachCourse(w http.ResponseWriter, r *http.Requ
 		NewContent:  newContent,
 	})
 
-	// --- course map exit management ---
-	// Create always-blocked exits so the map BFS lays the course rooms in sequence.
-	var prevRoom, nextRoom int
-	if insertAt > 0 {
-		prevRoom = obstacleRoomID(newList[insertAt-1])
-	}
-	if insertAt < len(newList)-1 {
-		nextRoom = obstacleRoomID(newList[insertAt+1])
-	}
-
-	if prevRoom > 0 {
-		pm, _, _, perr := s.loadRoomMapByID(prevRoom)
-		if perr == nil {
-			// Delete the old blocked link from prevRoom to nextRoom (now bypassed).
-			if nextRoom > 0 {
-				oldDir := findCourseExitDirToTarget(pm, nextRoom)
-				if oldDir != "" {
-					removeExit(pm, oldDir)
-				}
-			}
-			dir := ensureValidDirection(body.Direction, pm)
-			setCourseBlockedExit(pm, dir, roomID)
-			s.saveRoomMapByID(prevRoom, pm, fmt.Sprintf("blocked exit %d->%d for course %s", prevRoom, roomID, body.CourseID))
-		}
-	}
-	if nextRoom > 0 {
-		cm, _, _, cerr := s.loadRoomMapByID(roomID)
-		if cerr == nil {
-			dir := autoPickDirection(cm)
-			setCourseBlockedExit(cm, dir, nextRoom)
-			s.saveRoomMapByID(roomID, cm, fmt.Sprintf("blocked exit %d->%d for course %s", roomID, nextRoom, body.CourseID))
-		}
-	}
-
 	s.reloadCoursesIfWired()
 	writeJSON(w, map[string]any{"ok": true, "course_id": body.CourseID, "obstacle_index": insertAt, "total_obstacles": len(newList)})
 }
 
 // Detach a room from its course; delete the course file if it becomes empty
 
-func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Request, roomID int) {
-	if r.Method != http.MethodPost {
-		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
-		return
-	}
+// detachRoomFromCourse removes roomID from the course it belongs to, if any.
+// Returns the course ID that was modified, or "" if none match.
+func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted bool) {
 	ids, err := s.listCourseIDs()
 	if err != nil {
-		writeJSON(w, map[string]any{"error": err.Error()})
-		return
+		return "", false
 	}
 	for _, id := range ids {
 		m, _, path, err := s.loadCourseMap(id)
@@ -440,44 +436,8 @@ func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Requ
 		}
 		oldContent, _ := snapshotFile(path)
 
-		var prevRoom, nextRoom int
 		if idx > 0 {
-			prevRoom = obstacleRoomID(obstacles[idx-1])
-		}
-		if idx < len(obstacles)-1 {
-			nextRoom = obstacleRoomID(obstacles[idx+1])
-		}
-
-		// --- course map exit cleanup ---
-		var relinkDir string
-		if prevRoom > 0 {
-			pm, _, _, perr := s.loadRoomMapByID(prevRoom)
-			if perr == nil {
-				relinkDir = findCourseExitDirToTarget(pm, roomID)
-				if relinkDir != "" {
-					removeExit(pm, relinkDir)
-					s.saveRoomMapByID(prevRoom, pm, fmt.Sprintf("remove blocked exit to %d (course %s)", roomID, id))
-				}
-			}
-		}
-		if nextRoom > 0 {
-			cm, _, _, cerr := s.loadRoomMapByID(roomID)
-			if cerr == nil {
-				d := findCourseExitDirToTarget(cm, nextRoom)
-				if d != "" {
-					removeExit(cm, d)
-					s.saveRoomMapByID(roomID, cm, fmt.Sprintf("remove blocked exit to %d (course %s)", nextRoom, id))
-				}
-			}
-		}
-		// Relink prevRoom -> nextRoom
-		if prevRoom > 0 && nextRoom > 0 {
-			pm, _, _, perr := s.loadRoomMapByID(prevRoom)
-			if perr == nil {
-				dir := ensureValidDirection(relinkDir, pm)
-				setCourseBlockedExit(pm, dir, nextRoom)
-				s.saveRoomMapByID(prevRoom, pm, fmt.Sprintf("relink blocked exit %d->%d (course %s)", prevRoom, nextRoom, id))
-			}
+			delete(obstacles[idx-1], "exit_dir")
 		}
 
 		remaining := make([]map[string]any, 0, len(obstacles)-1)
@@ -485,10 +445,7 @@ func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Requ
 		remaining = append(remaining, obstacles[idx+1:]...)
 
 		if len(remaining) == 0 {
-			if err := os.Remove(path); err != nil {
-				writeJSON(w, map[string]any{"error": "delete error: " + err.Error()})
-				return
-			}
+			os.Remove(path)
 			s.undoStack.Push(ChangeDesc{
 				Description: fmt.Sprintf("delete course %s (last obstacle removed)", id),
 				FilePath:    path,
@@ -496,8 +453,7 @@ func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Requ
 				IsDelete:    true,
 			})
 			s.reloadCoursesIfWired()
-			writeJSON(w, map[string]any{"ok": true, "course_id": id, "deleted": true})
-			return
+			return id, true
 		}
 
 		anyList := make([]any, len(remaining))
@@ -507,8 +463,7 @@ func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Requ
 		m["obstacles"] = anyList
 		newContent, err := writeMapAsYAML(path, m)
 		if err != nil {
-			writeJSON(w, map[string]any{"error": "write error: " + err.Error()})
-			return
+			return "", false
 		}
 		s.undoStack.Push(ChangeDesc{
 			Description: fmt.Sprintf("detach room %d from course %s", roomID, id),
@@ -517,13 +472,23 @@ func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Requ
 			NewContent:  newContent,
 		})
 		s.reloadCoursesIfWired()
-		writeJSON(w, map[string]any{"ok": true, "course_id": id, "obstacle_index": idx, "total_obstacles": len(remaining)})
-		return
+		return id, false
 	}
-	writeJSON(w, map[string]any{"error": "room is not part of any course"})
+	return "", false
 }
 
-// --- hidden exit management on room files -------------------------------
+func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Request, roomID int) {
+	if r.Method != http.MethodPost {
+		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+		return
+	}
+	courseID, deleted := s.detachRoomFromCourse(roomID)
+	if courseID == "" && !deleted {
+		writeJSON(w, map[string]any{"error": "room is not part of any course"})
+		return
+	}
+	writeJSON(w, map[string]any{"ok": true, "course_id": courseID, "deleted": deleted})
+}
 
 // loadRoomMapByID reads a room YAML file as a generic map.
 func (s *AdminServer) loadRoomMapByID(roomID int) (map[string]any, []byte, string, error) {
@@ -562,25 +527,6 @@ func (s *AdminServer) saveRoomMapByID(roomID int, m map[string]any, desc string)
 	return nil
 }
 
-// roomExitsMap returns the exits sub-map from a room map, initializing it
-// as an empty map if absent.
-func roomExitsMap(m map[string]any) map[string]any {
-	if ex, ok := m["exits"].(map[string]any); ok {
-		return ex
-	}
-	ex := make(map[string]any)
-	m["exits"] = ex
-	return ex
-}
-
-// setCourseBlockedExit adds or replaces an always-blocked exit in direction dir
-// targeting targetRoomID. Always-blocked exits are blocked to players but
-// render on the map; used to lay out course rooms in visual sequence.
-func setCourseBlockedExit(m map[string]any, dir string, targetRoomID int) {
-	ex := roomExitsMap(m)
-	ex[dir] = map[string]any{"room": targetRoomID, "always_blocked": true}
-}
-
 // removeExit deletes an exit by direction. Returns true if it existed.
 func removeExit(m map[string]any, dir string) bool {
 	ex, ok := m["exits"].(map[string]any)
@@ -619,45 +565,66 @@ func exitIsAlwaysBlocked(v any) bool {
 	return false
 }
 
-// findCourseExitDirToTarget returns the direction of an always-blocked exit from m
-// targeting targetRoomID, or "" if none.
-func findCourseExitDirToTarget(m map[string]any, targetRoomID int) string {
-	ex, ok := m["exits"].(map[string]any)
-	if !ok {
-		return ""
-	}
-	for dir, v := range ex {
-		if exitIsAlwaysBlocked(v) && exitTargetRoom(v) == targetRoomID {
-			return dir
+
+
+// isHorizontalDir returns whether d is one of the 8 horizontal directions.
+func isHorizontalDir(d string) bool {
+	for _, h := range horizontalExitDirs {
+		if d == h {
+			return true
 		}
 	}
-	return ""
+	return false
 }
 
-// autoPickDirection returns the first horizontal direction that has no exit
-// on room map m.
-func autoPickDirection(m map[string]any) string {
+
+
+// reconcileExitOnRoom validates that roomID has an exit in dir.
+// If expectedTarget > 0, also checks the exit points to that room.
+// Returns an error string or empty string on success.
+func (s *AdminServer) validateExitDir(roomID int, dir string, expectedTarget int) string {
+	m, _, _, err := s.loadRoomMapByID(roomID)
+	if err != nil {
+		return "cannot load room " + strconv.Itoa(roomID)
+	}
 	ex, _ := m["exits"].(map[string]any)
-	for _, d := range horizontalExitDirs {
-		if _, occupied := ex[d]; !occupied {
-			return d
+
+	existing := ex[dir]
+	if existing == nil {
+		return "room " + strconv.Itoa(roomID) + " has no exit in direction " + dir
+	}
+	if expectedTarget > 0 {
+		target := exitTargetRoom(existing)
+		if target != expectedTarget {
+			return "direction " + dir + " on room " + strconv.Itoa(roomID) + " points to room " + strconv.Itoa(target) + ", expected " + strconv.Itoa(expectedTarget)
 		}
 	}
-	return "north"
+	return ""
 }
 
-// ensureValidDirection returns dir if it's a valid horizontal direction,
-// otherwise auto-picks one from m.
-func ensureValidDirection(dir string, m map[string]any) string {
-	dir = strings.TrimSpace(strings.ToLower(dir))
-	for _, d := range horizontalExitDirs {
-		if d == dir {
-			ex, _ := m["exits"].(map[string]any)
-			if _, occupied := ex[dir]; !occupied {
-				return dir
-			}
-			break
+// reconcileCourseExits validates every obstacle's exit_dir against the actual
+// exits on the corresponding room. Returns warnings for any mismatches found.
+func (s *AdminServer) reconcileCourseExits(m map[string]any) []string {
+	var warnings []string
+	obstacles := asMapList(m["obstacles"])
+
+	for i, obs := range obstacles {
+		roomID := obstacleRoomID(obs)
+		dir, _ := obs["exit_dir"].(string)
+		if dir == "" {
+			continue
+		}
+
+		var nextRoom int
+		if i < len(obstacles)-1 {
+			nextRoom = obstacleRoomID(obstacles[i+1])
+		}
+
+		errMsg := s.validateExitDir(roomID, dir, nextRoom)
+		if errMsg != "" {
+			warnings = append(warnings, errMsg)
 		}
 	}
-	return autoPickDirection(m)
+
+	return warnings
 }
\ No newline at end of file