From bab5dd653039d026eb3d4e46da7aafef7137fe7c Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Sat, 19 Sep 2026 03:52:04 -0400 Subject: fix: journal in-game room insert/remove, rewrite non-room room refs on rename/swap, seed admin grid from edited room, type action switches --- internal/admin/api_room_grid_seed_test.go | 71 +++++++++++++++++++++++++ internal/admin/api_rooms.go | 24 +++++++-- internal/admin/room_content_migrate.go | 38 ++++++++++++++ internal/admin/room_rename_content_test.go | 83 ++++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 internal/admin/api_room_grid_seed_test.go create mode 100644 internal/admin/room_content_migrate.go create mode 100644 internal/admin/room_rename_content_test.go (limited to 'internal/admin') diff --git a/internal/admin/api_room_grid_seed_test.go b/internal/admin/api_room_grid_seed_test.go new file mode 100644 index 0000000..f1b347d --- /dev/null +++ b/internal/admin/api_room_grid_seed_test.go @@ -0,0 +1,71 @@ +package admin + +// Regression: admin create/link grid prechecks seeded the layout from the +// configured starting room. A detached in-progress area (not yet reachable +// from the start room) was therefore never laid out, so a link that introduced +// a grid twist was accepted, and auto-detecting a direction rejected two +// adjacent detached rooms as "not reachable from the seed room". The prechecks +// now seed from the room being edited. + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "thehouseoficarus/internal/config" +) + +// newDetachedAdminServer builds a server whose configured starting room (999) +// is deliberately outside every room under test. +func newDetachedAdminServer(t *testing.T, rooms map[int]string) *AdminServer { + t.Helper() + s := newTestAdminServer(t, rooms) + s.cfg = config.Default() + s.cfg.StartingRoom = 999 + return s +} + +func postLink(t *testing.T, s *AdminServer, body string) (map[string]any, int) { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/api/rooms/link", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + s.handleRoomLink(rr, req) + var resp map[string]any + _ = json.Unmarshal(rr.Body.Bytes(), &resp) + return resp, rr.Code +} + +func TestLinkConflictDetectedInDetachedArea(t *testing.T) { + s := newDetachedAdminServer(t, map[int]string{ + 10: "id: 10\nname: A\nexits:\n north:\n room: 11\n", + 11: "id: 11\nname: B\nexits:\n south:\n room: 10\n east:\n room: 12\n", + 12: "id: 12\nname: C\n", + // Starting room outside the component under edit. + 999: "id: 999\nname: Start\n", + }) + + // 10-e north->11 and 11-east->12 place 12 at (1,-1); adding 10-east->12 + // would also place it at (1,0) — a twist the detached precheck must catch. + if resp, code := postLink(t, s, `{"from":10,"to":12,"dir":"east"}`); code != http.StatusConflict { + t.Fatalf("detached link that creates a grid twist: status = %d (%v), want %d", code, resp, http.StatusConflict) + } +} + +func TestAutoDirectionWorksInDetachedArea(t *testing.T) { + s := newDetachedAdminServer(t, map[int]string{ + 10: "id: 10\nname: A\nexits:\n east:\n room: 11\n", + 11: "id: 11\nname: B\nexits:\n west:\n room: 10\n", + 999: "id: 999\nname: Start\n", + }) + + resp, code := postLink(t, s, `{"from":10,"to":11}`) + if code != http.StatusOK { + t.Fatalf("auto-direction in a detached area: status = %d (%v), want 200", code, resp) + } + if got := roomExitTarget(s, 10, "east"); got != 11 { + t.Fatalf("room 10 east exit = %d, want 11", got) + } +} diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index ca7ad06..efc466c 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -101,7 +101,7 @@ func (s *AdminServer) handleRooms(w http.ResponseWriter, r *http.Request) { opp, validDir := world.OppositeExit[dir] if validDir { conflicts := 0 - world.BuildGrid(s.cfg.StartingRoom, func(rid int) (*world.Room, bool) { + world.BuildGrid(*linkFrom, func(rid int) (*world.Room, bool) { if rid == id { ex := make(map[world.ExitDir]world.ExitDef) if !linkOneWay { @@ -571,7 +571,11 @@ func (s *AdminServer) handleRoomLink(w http.ResponseWriter, r *http.Request) { return } } else { - grid := world.BuildGrid(s.cfg.StartingRoom, func(id int) (*world.Room, bool) { + // Auto-detect the direction from the room being edited, not the + // configured starting room: a detached in-progress area is not + // reachable from the start room, and seeding there would report + // both rooms as unreachable even though they are adjacent. + grid := world.BuildGrid(body.From, func(id int) (*world.Room, bool) { room, err := s.world.LoadRoom(id) if err != nil { log.Printf("link: BuildGrid failed to load room %d: %v", id, err) @@ -583,7 +587,7 @@ func (s *AdminServer) handleRoomLink(w http.ResponseWriter, r *http.Request) { cA, okA := grid.Coord[body.From] cB, okB := grid.Coord[body.To] if !okA || !okB { - writeJSONError(w, "one or both rooms are not reachable from the seed room", http.StatusBadRequest) + writeJSONError(w, "rooms are not in the same connected component; specify a direction explicitly", http.StatusBadRequest) return } if cA[2] != cB[2] { @@ -636,7 +640,7 @@ func (s *AdminServer) handleRoomLink(w http.ResponseWriter, r *http.Request) { } } - conflicts := world.BuildGridConflicts(s.cfg.StartingRoom, func(id int) (*world.Room, bool) { + conflicts := world.BuildGridConflicts(body.From, func(id int) (*world.Room, bool) { room, err := s.world.LoadRoom(id) if err != nil { log.Printf("link: conflict test BuildGrid failed to load room %d: %v", id, err) @@ -1062,6 +1066,15 @@ func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) { } refUpdates++ } + // Non-room content (courses, modules, triggers, items, objects, mobs) + // carries structural room references too. Update it in the same journaled + // commit, or a renamed room leaves a course obstacle or transport + // destination pointing at an ID that no longer exists. + contentExtras, err := rewriteContentRoomRefs(s.dataDir, idMap, txn) + if err != nil { + writeJSONError(w, "plan content write: "+err.Error(), http.StatusInternalServerError) + return + } if err := txn.Commit(); err != nil { writeJSONError(w, "commit rename: "+err.Error(), http.StatusInternalServerError) return @@ -1101,6 +1114,9 @@ func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) { }) } } + // Content-file rewrites are part of the same change, so undo restores + // them alongside the room files. + change.ExtraFiles = append(change.ExtraFiles, contentExtras...) s.undoStack.Push(change) writeJSON(w, map[string]any{"ok": true, "refs": refUpdates}) } diff --git a/internal/admin/room_content_migrate.go b/internal/admin/room_content_migrate.go new file mode 100644 index 0000000..47353fd --- /dev/null +++ b/internal/admin/room_content_migrate.go @@ -0,0 +1,38 @@ +package admin + +import ( + "fmt" + "path/filepath" + + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/persistence" +) + +// rewriteContentRoomRefs rewrites structural room-ID references in every +// non-room content file under dataDir per idMap, stages the changed files into +// txn, and returns the before/after images so the caller can record them for +// undo. This keeps a room rename's content updates in the SAME journaled +// commit as the room files. +func rewriteContentRoomRefs(dataDir string, idMap map[int]int, txn *persistence.Transaction) ([]ExtraFile, error) { + if len(idMap) == 0 { + return nil, nil + } + var extras []ExtraFile + for _, sub := range behavior.RoomRefContentDirs { + changes, err := behavior.RewriteRoomRefsInDir(filepath.Join(dataDir, sub), idMap) + if err != nil { + return extras, fmt.Errorf("scan %s: %w", sub, err) + } + for _, ch := range changes { + if err := txn.Replace(ch.Path, ch.Content, 0644); err != nil { + return extras, fmt.Errorf("stage %s: %w", ch.Path, err) + } + extras = append(extras, ExtraFile{ + FilePath: ch.Path, + OldContent: ch.Old, + NewContent: ch.Content, + }) + } + } + return extras, nil +} diff --git a/internal/admin/room_rename_content_test.go b/internal/admin/room_rename_content_test.go new file mode 100644 index 0000000..179b9f0 --- /dev/null +++ b/internal/admin/room_rename_content_test.go @@ -0,0 +1,83 @@ +package admin + +// Regression: renaming a room must rewrite structural room references in +// non-room content files (courses, modules, triggers, items, objects, mobs), +// and undo must restore those files too. Before this, only data/rooms was +// scanned, so a course obstacle or transport destination silently kept the +// old ID. + +import ( + "net/http" + "os" + "path/filepath" + "strings" + "testing" +) + +func writeContent(t *testing.T, s *AdminServer, sub, name, body string) string { + t.Helper() + dir := filepath.Join(s.dataDir, sub) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + return path +} + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +func TestRoomRenameRewritesNonRoomContent(t *testing.T) { + s := newTestAdminServer(t, map[int]string{ + 5: "id: 5\nname: Lab\nexits:\n north:\n room: 6\n", + 6: "id: 6\nname: Hall\nexits:\n south:\n room: 5\n", + }) + s.rewriteRoomIDs = func(map[int]int) {} + + modulePath := writeContent(t, s, "modules", "transport_home.yaml", + "name: Home\ncategory: transport\ndestination: 5\n") + coursePath := writeContent(t, s, "courses", "agility.yaml", + "name: Agility\nstart_room: 6\nobstacles:\n - room_id: 5\n verb: climb\n") + // An author value equal to a room ID must survive untouched. + itemPath := writeContent(t, s, "items", "gem.yaml", + "name: gem\nvalue: 5\n") + + if rec := renameRoom(t, s, 5, 7); rec.Code != http.StatusOK { + t.Fatalf("rename failed: %d %s", rec.Code, rec.Body.String()) + } + + if got := readFile(t, modulePath); !strings.Contains(got, "destination: 7") { + t.Errorf("module destination not rewritten: %s", got) + } + course := readFile(t, coursePath) + if !strings.Contains(course, "room_id: 7") { + t.Errorf("course obstacle room_id not rewritten: %s", course) + } + // start_room: 6 is not the renamed ID, so it stays. + if !strings.Contains(course, "start_room: 6") { + t.Errorf("unrelated course start_room changed: %s", course) + } + if got := readFile(t, itemPath); !strings.Contains(got, "value: 5") { + t.Errorf("author value equal to the renamed ID was rewritten: %s", got) + } + + // Undo restores the content files alongside the room. + if s.undoStack.Undo() == nil { + t.Fatal("undo failed") + } + if got := readFile(t, modulePath); !strings.Contains(got, "destination: 5") { + t.Errorf("undo did not restore module destination: %s", got) + } + if got := readFile(t, coursePath); !strings.Contains(got, "room_id: 5") { + t.Errorf("undo did not restore course obstacle: %s", got) + } +} -- cgit v1.2.3