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 +++++++++ internal/behavior/roomrefs.go | 231 ++++++++++++++++++++++++++ internal/behavior/roomrefs_test.go | 130 +++++++++++++++ internal/game/act.go | 40 ++--- internal/game/cmd_registry.go | 5 +- internal/game/cmd_room_insert.go | 47 ++++-- internal/game/cmd_room_remove.go | 33 ++-- internal/game/cmd_room_txn_regression_test.go | 119 +++++++++++++ internal/game/cmd_swapid.go | 24 ++- internal/game/core_production.go | 6 +- internal/game/exit_migrate.go | 15 ++ internal/game/swapid_content_test.go | 64 +++++++ internal/game/tick.go | 2 +- 16 files changed, 874 insertions(+), 58 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 create mode 100644 internal/behavior/roomrefs.go create mode 100644 internal/behavior/roomrefs_test.go create mode 100644 internal/game/cmd_room_txn_regression_test.go create mode 100644 internal/game/swapid_content_test.go (limited to 'internal') 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) + } +} diff --git a/internal/behavior/roomrefs.go b/internal/behavior/roomrefs.go new file mode 100644 index 0000000..4953092 --- /dev/null +++ b/internal/behavior/roomrefs.go @@ -0,0 +1,231 @@ +package behavior + +import ( + "bytes" + "os" + + "gopkg.in/yaml.v3" +) + +// Structural room-ID references in non-room content files. +// +// Rooms carry their room references in typed structs (world.Room.RewriteRoomIDs +// walks those). Content outside data/rooms — courses, modules, triggers, items, +// objects, mobs — carries the same references in YAML that the admin editor +// reads and writes as generic maps. This walker remaps those references by +// field name so a room rename/swap cannot leave a course obstacle, a transport +// destination, or a trigger teleport pointing at the old ID. +// +// It is deliberately key-aware: only the known structural keys are touched, so +// an author's flag value, delay, quantity, or color index that happens to equal +// a room ID is never rewritten. That is the same rule world.Room.RewriteRoomIDs +// applies to typed fields. + +// scalarRoomRefKeys are keys whose value is a single room ID. +var scalarRoomRefKeys = map[string]bool{ + "room": true, // world.ExitDef.Room, behavior.Condition.Room + "teleport": true, // behavior.Step.Teleport + "destination": true, // game.ModDef.Destination + "on_fail": true, // game.ObstacleDef.OnFailRoom + "room_id": true, // game.ObstacleDef.RoomID + "start_room": true, // game.CourseConfig.StartRoom +} + +// listRoomRefKeys are keys whose value is a list of room IDs. +var listRoomRefKeys = map[string]bool{ + "wander_rooms": true, // world.RoomMob/RoomObject.WanderRooms + "despawn_rooms": true, // behavior.SpawnMobConfig.DespawnRooms +} + +// RemapRoomRefs remaps every structural room-ID reference in a decoded YAML +// value per idMap (oldID -> newID), recursing through maps and lists. It +// returns whether anything changed. idMap is assumed to be a bijection. +// +// The value is expected to be the result of yaml.Unmarshal into any (maps and +// slices of any); unknown shapes are ignored. +func RemapRoomRefs(v any, idMap map[int]int) bool { + if len(idMap) == 0 { + return false + } + return remapRoomRefs(v, idMap) +} + +func remapRoomRefs(v any, idMap map[int]int) bool { + switch node := v.(type) { + case map[string]any: + changed := false + for key, val := range node { + if scalarRoomRefKeys[key] { + if n, ok := asInt(val); ok { + if newID, mapped := idMap[n]; mapped && newID != n { + node[key] = newID + changed = true + } + continue + } + } + if listRoomRefKeys[key] { + if list, ok := val.([]any); ok && remapRoomRefList(list, idMap) { + changed = true + } + continue + } + if remapRoomRefs(val, idMap) { + changed = true + } + } + return changed + case map[any]any: + // yaml.v3 decodes into map[string]any, but a caller that round-tripped + // through a different decoder can hand us map[any]any. + changed := false + for key, val := range node { + ks, ok := key.(string) + if !ok { + continue + } + if scalarRoomRefKeys[ks] { + if n, ok := asInt(val); ok { + if newID, mapped := idMap[n]; mapped && newID != n { + node[key] = newID + changed = true + } + continue + } + } + if listRoomRefKeys[ks] { + if list, ok := val.([]any); ok && remapRoomRefList(list, idMap) { + changed = true + } + continue + } + if remapRoomRefs(val, idMap) { + changed = true + } + } + return changed + case []any: + changed := false + for _, item := range node { + if remapRoomRefs(item, idMap) { + changed = true + } + } + return changed + default: + return false + } +} + +func remapRoomRefList(list []any, idMap map[int]int) bool { + changed := false + for i, item := range list { + n, ok := asInt(item) + if !ok { + continue + } + if newID, mapped := idMap[n]; mapped && newID != n { + list[i] = newID + changed = true + } + } + return changed +} + +// MarshalYAMLMap encodes a decoded YAML map with the same settings the admin +// editor uses, so a rewritten content file matches what the editor would write +// and cannot drop a key a typed struct does not model. +func MarshalYAMLMap(m map[string]any) ([]byte, error) { + var buf bytes.Buffer + enc := yaml.NewEncoder(&buf) + enc.SetIndent(2) + if err := enc.Encode(m); err != nil { + return nil, err + } + if err := enc.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// asInt coerces the numeric types a YAML decoder (or a JSON round-trip) can +// produce for an integer field. +func asInt(v any) (int, bool) { + switch n := v.(type) { + case int: + return n, true + case int8: + return int(n), true + case int16: + return int(n), true + case int32: + return int(n), true + case int64: + return int(n), true + case uint: + return int(n), true + case uint8: + return int(n), true + case uint16: + return int(n), true + case uint32: + return int(n), true + case uint64: + return int(n), true + case float64: + return int(n), true + case float32: + return int(n), true + default: + return 0, false + } +} + +// RoomRefContentDirs are the data subdirectories whose files can carry a +// structural room reference outside data/rooms. Directories not listed here +// (help, players) never do, and players are migrated separately by the game. +var RoomRefContentDirs = []string{"courses", "modules", "triggers", "items", "objects", "mobs"} + +// RoomRefFileChange is a content file whose room references moved, with its +// original and rewritten content. Callers stage these into a journaled +// transaction (or write them) and record the before/after images for undo. +type RoomRefFileChange struct { + Path string + Old []byte + Content []byte +} + +// RewriteRoomRefsInDir walks dir and returns every YAML file whose structural +// room references changed under idMap, re-encoded. A file that cannot be read +// or parsed is skipped: one malformed content file must not block a room +// rename, and the startup validator reports it independently. A missing dir is +// not an error (the content family may be absent). +func RewriteRoomRefsInDir(dir string, idMap map[int]int) ([]RoomRefFileChange, error) { + if len(idMap) == 0 { + return nil, nil + } + var changes []RoomRefFileChange + err := WalkYAMLDir(dir, func(path, id string, data []byte) error { + var v any + if err := yaml.Unmarshal(data, &v); err != nil { + return nil + } + if !RemapRoomRefs(v, idMap) { + return nil + } + m, ok := v.(map[string]any) + if !ok { + return nil + } + encoded, err := MarshalYAMLMap(m) + if err != nil { + return err + } + changes = append(changes, RoomRefFileChange{Path: path, Old: data, Content: encoded}) + return nil + }) + if os.IsNotExist(err) { + return nil, nil + } + return changes, err +} diff --git a/internal/behavior/roomrefs_test.go b/internal/behavior/roomrefs_test.go new file mode 100644 index 0000000..9651b17 --- /dev/null +++ b/internal/behavior/roomrefs_test.go @@ -0,0 +1,130 @@ +package behavior + +// Structural room-reference remapping for non-room content files. The walker +// must rewrite only known room-ID keys and must not touch author values that +// merely happen to equal a room ID. + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +func decode(t *testing.T, src string) any { + t.Helper() + var v any + if err := yaml.Unmarshal([]byte(src), &v); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return v +} + +func TestRemapRoomRefsRewritesStructuralKeys(t *testing.T) { + src := ` +destination: 10 +on_fail: 11 +room_id: 12 +start_room: 13 +wander_rooms: [14, 15] +despawn_rooms: [16] +teleport: 17 +nested: + room: 18 + condition: + room: 19 +` + v := decode(t, src) + idMap := map[int]int{ + 10: 110, 11: 111, 12: 112, 13: 113, 14: 114, 15: 115, + 16: 116, 17: 117, 18: 118, 19: 119, + } + if !RemapRoomRefs(v, idMap) { + t.Fatal("expected changed=true") + } + out, err := MarshalYAMLMap(v.(map[string]any)) + if err != nil { + t.Fatal(err) + } + s := string(out) + for _, want := range []string{ + "destination: 110", "on_fail: 111", "room_id: 112", "start_room: 113", + "teleport: 117", + } { + if !strings.Contains(s, want) { + t.Errorf("missing rewritten %q in:\n%s", want, s) + } + } + // Lists are rewritten in place. + for _, want := range []string{"- 114", "- 115", "- 116"} { + if !strings.Contains(s, want) { + t.Errorf("missing rewritten list entry %q in:\n%s", want, s) + } + } + for _, want := range []string{"room: 118", "room: 119"} { + if !strings.Contains(s, want) { + t.Errorf("missing nested rewritten %q in:\n%s", want, s) + } + } +} + +func TestRemapRoomRefsLeavesAuthorValues(t *testing.T) { + // A delay, quantity, color index, and author flag value that equal room + // IDs must never be rewritten: only structural keys are room references. + src := ` +value: 10 +delay: 11 +quantity: 12 +color: "13" +set_global_flags: + last_room: 14 +steps: + - wait: 15 + credits: 16 +` + v := decode(t, src) + idMap := map[int]int{10: 110, 11: 111, 12: 112, 13: 113, 14: 114, 15: 115, 16: 116} + if RemapRoomRefs(v, idMap) { + t.Fatal("author values were remapped; only structural room keys may change") + } +} + +func TestRewriteRoomRefsInDir(t *testing.T) { + dir := t.TempDir() + write := func(name, body string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + write("course.yaml", "start_room: 5\nobstacles:\n - room_id: 6\n") + write("unrelated.yaml", "value: 5\nname: keeps its value\n") + + changes, err := RewriteRoomRefsInDir(dir, map[int]int{5: 50, 6: 60}) + if err != nil { + t.Fatal(err) + } + if len(changes) != 1 { + t.Fatalf("changed %d files, want 1: %+v", len(changes), changes) + } + if !strings.Contains(string(changes[0].Content), "start_room: 50") || + !strings.Contains(string(changes[0].Content), "room_id: 60") { + t.Fatalf("course not rewritten: %s", changes[0].Content) + } + if changes[0].Old == nil { + t.Fatal("before-image not captured for undo") + } +} + +// Missing content directories are not an error (a family may be absent). +func TestRewriteRoomRefsInDirMissingDir(t *testing.T) { + changes, err := RewriteRoomRefsInDir(t.TempDir()+"/nope", map[int]int{1: 2}) + if err != nil { + t.Fatalf("missing dir must not error: %v", err) + } + if len(changes) != 0 { + t.Fatalf("expected no changes, got %+v", changes) + } +} diff --git a/internal/game/act.go b/internal/game/act.go index 449b2da..7ff805c 100644 --- a/internal/game/act.go +++ b/internal/game/act.go @@ -19,8 +19,8 @@ import ( // cut, talk/speak/ask, steal/thieve, hide) must not be duplicated here: a // registered command is looked up first, so an entry would be dead and its // removal untested. Only shear remains as a pure action alias. -var verbAliases = map[string]string{ - "shear": "gather", +var verbAliases = map[string]behavior.ActionType{ + "shear": behavior.TypeGather, } var verbSkill = map[string]string{ @@ -35,7 +35,7 @@ var verbSkill = map[string]string{ func normalizeVerb(v string) string { if a, ok := verbAliases[v]; ok { - return a + return string(a) } return v } @@ -161,14 +161,14 @@ func (g *Game) startAction(sess *net.Session, verb, target string) { return } - switch bhType { - case "gather": + switch behavior.ActionType(bhType) { + case behavior.TypeGather: if obj == nil || chosen == nil { sess.WriteLine(fmt.Sprintf("You can't %s that.", verb)) return } g.startGather(sess, p, obj, chosen) - case "talk": + case behavior.TypeTalk: if mob != nil { g.startMobTalk(sess, p, mob) } else { @@ -205,27 +205,27 @@ func (g *Game) AdvanceActions() { continue } switch p.Action.Type { - case "gather": + case behavior.TypeGather: g.advanceGather(sess, p) - case "burn": + case behavior.TypeBurn: g.advanceBurn(sess, p) - case "stoke": + case behavior.TypeStoke: g.advanceStoke(sess, p) - case "identify": + case behavior.TypeIdentify: g.advanceIdentify(sess, p) - case "steal": + case behavior.TypeSteal: g.advanceSteal(sess, p) - case "plant": + case behavior.TypePlant: g.advancePlant(sess, p) - case "harvest": + case behavior.TypeHarvest: g.advanceHarvest(sess, p) - case "rake": + case behavior.TypeRake: g.advanceRake(sess, p) - case "water": + case behavior.TypeWater: g.advanceWater(sess, p) - case "cure": + case behavior.TypeCure: g.advanceCure(sess, p) - case "obstacle": + case behavior.TypeObstacle: g.advanceObstacle(sess, p) case behavior.TypeTalk: g.advanceTalk(sess, p) @@ -234,7 +234,7 @@ func (g *Game) AdvanceActions() { case behavior.TypeCombine: g.advanceCombine(sess, p) default: - if productionActionTypes[string(p.Action.Type)] { + if productionActionTypes[p.Action.Type] { g.advanceProduction(sess, p) } } @@ -252,9 +252,9 @@ func (g *Game) AdvanceActions() { continue } switch p.BackgroundAction.Type { - case "fletch": + case behavior.TypeFletch: g.advanceFletch(sess, p) - case "cleaning": + case behavior.TypeClean: g.advanceClean(sess, p) } if p.BackgroundAction == nil { diff --git a/internal/game/cmd_registry.go b/internal/game/cmd_registry.go index fb72c99..ae4c895 100644 --- a/internal/game/cmd_registry.go +++ b/internal/game/cmd_registry.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "thehouseoficarus/internal/behavior" "thehouseoficarus/internal/net" ) @@ -225,7 +226,7 @@ func (g *Game) executeCommand(sess *net.Session, cmd string, args []string, rawI if a, ok := verbAliases[cmd]; ok { g.cancelAction(p) switch a { - case "gather": + case behavior.TypeGather: if len(args) == 0 { target := g.defaultTarget(p.RoomID, cmd) if target == "" { @@ -236,7 +237,7 @@ func (g *Game) executeCommand(sess *net.Session, cmd string, args []string, rawI } else { g.startAction(sess, cmd, strings.Join(args, " ")) } - case "talk": + case behavior.TypeTalk: if len(args) == 0 { sess.WriteLine("Talk to whom?") } else { diff --git a/internal/game/cmd_room_insert.go b/internal/game/cmd_room_insert.go index 2d7ff58..09b2e74 100644 --- a/internal/game/cmd_room_insert.go +++ b/internal/game/cmd_room_insert.go @@ -127,12 +127,6 @@ func (g *Game) roomInsert(sess *net.Session, args []string) { sess.WriteLine("Error creating directory.") return } - if err := writeRoomFile(newPath, data); err != nil { - sess.WriteLine("Error writing room file.") - return - } - - g.World.AddRoomPath(newID, newPath) curRoom.Exits[dir] = world.RewirePreserving(exitDef, newID) curData, err := yaml.Marshal(curRoom) @@ -140,23 +134,42 @@ func (g *Game) roomInsert(sess *net.Session, args []string) { sess.WriteLine("Error updating current room YAML.") return } - if err := writeRoomFile(curPath, curData); err != nil { - sess.WriteLine("Error writing current room file.") + + // Stage all three files (the new room, the current room, and the far + // room's reciprocal exit when it points back) into ONE journal + // transaction. The previous sequential writes could stop after the new + // room existed but before the current room's exit was rewritten, leaving + // a dangling exit with no recovery record; the transaction rolls the + // whole insert back on any failure or crash. + txn := g.newTxn() + if err := txn.Replace(newPath, data, 0644); err != nil { + sess.WriteLine(fmt.Sprintf("Error writing room file: %v", err)) + return + } + if err := txn.Replace(curPath, curData, 0644); err != nil { + sess.WriteLine(fmt.Sprintf("Error writing current room file: %v", err)) return } - if targetExit, tok := targetRoom.Exits[oppositeDir]; tok && targetExit.Room == p.RoomID { - targetRoom.Exits[oppositeDir] = world.RewirePreserving(targetExit, newID) - } - targetPath, tok2 := g.World.GetRoomPath(targetID) - if tok2 { - targetData, terr := yaml.Marshal(targetRoom) - if terr == nil { - if werr := writeRoomFile(targetPath, targetData); werr != nil { - sess.WriteLine(fmt.Sprintf("Warning: could not link the opposite exit in room %d: %v", targetID, werr)) + if targetPath, tok2 := g.World.GetRoomPath(targetID); tok2 { + targetRoom.Exits[oppositeDir] = world.RewirePreserving(targetExit, newID) + targetData, terr := yaml.Marshal(targetRoom) + if terr != nil { + sess.WriteLine(fmt.Sprintf("Error updating room %d YAML: %v", targetID, terr)) + return + } + if err := txn.Replace(targetPath, targetData, 0644); err != nil { + sess.WriteLine(fmt.Sprintf("Error linking the opposite exit in room %d: %v", targetID, err)) + return } } } + if err := txn.Commit(); err != nil { + sess.WriteLine(fmt.Sprintf("Error committing room insert: %v", err)) + return + } + + g.World.AddRoomPath(newID, newPath) oldRoom := p.RoomID p.ClearMoveState() diff --git a/internal/game/cmd_room_remove.go b/internal/game/cmd_room_remove.go index 361cc74..79b84d8 100644 --- a/internal/game/cmd_room_remove.go +++ b/internal/game/cmd_room_remove.go @@ -166,15 +166,21 @@ func (g *Game) roomRemove(sess *net.Session, args []string) { return } - // Apply: rewire A's dir exit to C, preserving A's current dir-exit props. + // Stage A, C, and B's deletion into ONE journal transaction: the previous + // sequential writes could delete B after rewriting A but before C, leaving + // the map half-pulled with no recovery record. The transaction restores + // every before-image on any failure or crash. + txn := g.newTxn() + + // Rewire A's dir exit to C, preserving A's current dir-exit props. aRoom.Exits[dir] = world.RewirePreserving(aExit, cID) aData, err := yaml.Marshal(aRoom) if err != nil { sess.WriteLine("Error updating current room YAML.") return } - if err := writeRoomFile(aPath, aData); err != nil { - sess.WriteLine("Error writing current room file.") + if err := txn.Replace(aPath, aData, 0644); err != nil { + sess.WriteLine(fmt.Sprintf("Error writing current room file: %v", err)) return } @@ -183,21 +189,28 @@ func (g *Game) roomRemove(sess *net.Session, args []string) { cPath, cPathOK := g.World.GetRoomPath(cID) if cPathOK { cData, cerr := yaml.Marshal(cRoom) - if cerr == nil { - if werr := writeRoomFile(cPath, cData); werr != nil { - sess.WriteLine(fmt.Sprintf("Warning: could not link the opposite exit in room %d: %v", cID, werr)) - } + if cerr != nil { + sess.WriteLine(fmt.Sprintf("Error updating room %d YAML: %v", cID, cerr)) + return + } + if err := txn.Replace(cPath, cData, 0644); err != nil { + sess.WriteLine(fmt.Sprintf("Error linking the opposite exit in room %d: %v", cID, err)) + return } } - // Delete B's file. + // Delete B's file in the same transaction. bPath, bPathOK := g.World.GetRoomPath(bID) if !bPathOK { sess.WriteLine(fmt.Sprintf("Room file for %d not found.", bID)) return } - if rerr := os.Remove(bPath); rerr != nil { - sess.WriteLine(fmt.Sprintf("Error deleting room file: %v", rerr)) + if err := txn.Delete(bPath); err != nil { + sess.WriteLine(fmt.Sprintf("Error deleting room file: %v", err)) + return + } + if err := txn.Commit(); err != nil { + sess.WriteLine(fmt.Sprintf("Error committing room remove: %v", err)) return } diff --git a/internal/game/cmd_room_txn_regression_test.go b/internal/game/cmd_room_txn_regression_test.go new file mode 100644 index 0000000..3b5e864 --- /dev/null +++ b/internal/game/cmd_room_txn_regression_test.go @@ -0,0 +1,119 @@ +package game + +// Regression: the in-game `room insert`/`room remove` commands must commit all +// their file changes as one journaled transaction. A failure partway through +// (or a crash) must leave the world exactly as before, with no dangling exit +// and no half-deleted room. + +import ( + "os" + "path/filepath" + "testing" + + "thehouseoficarus/internal/player" + "thehouseoficarus/internal/world" +) + +func TestRoomInsertCommitsTransactionally(t *testing.T) { + dir := t.TempDir() + writeRemoveRoomFile(t, dir, 1, "name: A\nexits:\n east: {room: 2}\n") + writeRemoveRoomFile(t, dir, 2, "name: B\nexits:\n west: {room: 1}\n") + g := newRemoveGame(t, dir) + sess := newRemoveSession(&player.Player{Name: "tester", RoomID: 1}) + + g.roomInsert(sess, []string{"east", "New Room"}) + + // A now leads east to the new room, which leads east to B and west to A; + // B's reciprocal west exit points at the new room. + a := reloadRoom(t, g, 1) + newID := a.Exits[world.East].Room + if newID == 2 { + t.Fatalf("insert did not create a new room: A.east=%d", newID) + } + newRoom := reloadRoom(t, g, newID) + if newRoom.Exits[world.East].Room != 2 || newRoom.Exits[world.West].Room != 1 { + t.Fatalf("new room exits wrong: %+v", newRoom.Exits) + } + b := reloadRoom(t, g, 2) + if b.Exits[world.West].Room != newID { + t.Fatalf("B.west = %d, want %d", b.Exits[world.West].Room, newID) + } + + // A committed transaction leaves no journal behind. + if entries, err := os.ReadDir(filepath.Join(dir, ".journal")); err == nil { + for _, e := range entries { + t.Fatalf("leftover journal after insert: %s", e.Name()) + } + } +} + +// A failure committing the current-room rewrite must roll back the already +// staged new room, leaving no dangling exit and no orphan file. +func TestRoomInsertRollsBackOnCommitFailure(t *testing.T) { + dir := t.TempDir() + writeRemoveRoomFile(t, dir, 1, "name: A\nexits:\n east: {room: 2}\n") + writeRemoveRoomFile(t, dir, 2, "name: B\nexits:\n west: {room: 1}\n") + g := newRemoveGame(t, dir) + sess := newRemoveSession(&player.Player{Name: "tester", RoomID: 1}) + + // Make the current room file unwritable by replacing it with a directory: + // the transaction's replace of curPath fails during apply and rolls back. + curPath := filepath.Join(dir, "rooms", "1.yaml") + if err := os.Remove(curPath); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(curPath, 0o755); err != nil { + t.Fatal(err) + } + + g.roomInsert(sess, []string{"east", "New Room"}) + + // The insert picks the first unused ID (3). The rolled-back insert must + // not leave that file behind. + if _, err := os.Stat(filepath.Join(dir, "rooms", "3.yaml")); !os.IsNotExist(err) { + t.Fatalf("rolled-back insert left the new room file (err=%v)", err) + } + // B's reciprocal exit must be unchanged (no dangling reference to a + // room that was rolled back). + b := reloadRoom(t, g, 2) + if b.Exits[world.West].Room != 1 { + t.Fatalf("B.west = %d, want 1 after rollback", b.Exits[world.West].Room) + } +} + +func TestRoomRemoveRollsBackOnCommitFailure(t *testing.T) { + dir := t.TempDir() + writeRemoveRoomFile(t, dir, 1, "name: A\nexits:\n east: {room: 2}\n") + writeRemoveRoomFile(t, dir, 2, "name: B\nexits:\n east: {room: 3}\n west: {room: 1}\n") + writeRemoveRoomFile(t, dir, 3, "name: C\nexits:\n west: {room: 2}\n") + g := newRemoveGame(t, dir) + sess := newRemoveSession(&player.Player{Name: "tester", RoomID: 1}) + + // Replace C's file with a directory so the transaction's replace of C fails + // AFTER A has been staged. The old sequential code wrote A, only warned on + // the C failure, and still deleted B — leaving A pointing at C while C did + // not point back. The transaction must roll A back and keep B. + cPath := filepath.Join(dir, "rooms", "3.yaml") + if err := os.Remove(cPath); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(cPath, 0o755); err != nil { + t.Fatal(err) + } + + g.roomRemove(sess, []string{"east"}) + + // B must still exist: the remove must not have half-applied. + if _, err := os.Stat(filepath.Join(dir, "rooms", "2.yaml")); err != nil { + t.Fatalf("rolled-back remove deleted B: %v", err) + } + // A must still point at B, not at the C rewrite that was rolled back. + a := reloadRoom(t, g, 1) + if a.Exits[world.East].Room != 2 { + t.Fatalf("A.east = %d, want 2 after rollback", a.Exits[world.East].Room) + } + b := reloadRoom(t, g, 2) + if b.Exits[world.West].Room != 1 || b.Exits[world.East].Room != 3 { + t.Fatalf("B exits changed after rollback: %+v", b.Exits) + } +} diff --git a/internal/game/cmd_swapid.go b/internal/game/cmd_swapid.go index d1dbd68..b0863d4 100644 --- a/internal/game/cmd_swapid.go +++ b/internal/game/cmd_swapid.go @@ -3,10 +3,12 @@ package game import ( "fmt" "os" + "path/filepath" "strconv" "gopkg.in/yaml.v3" + "thehouseoficarus/internal/behavior" "thehouseoficarus/internal/net" ) @@ -149,6 +151,26 @@ func (g *Game) executeSwapID(sess *net.Session, args []string, rawInput string) } updated++ } + // Non-room content (courses, modules, triggers, items, objects, mobs) + // carries structural room references too. Stage it into the SAME journal + // transaction as the room files, so a swap never leaves a course obstacle, + // transport destination, or trigger teleport pointing at the wrong room + // even if one side's write fails. + contentUpdated := 0 + for _, sub := range behavior.RoomRefContentDirs { + changes, cerr := behavior.RewriteRoomRefsInDir(filepath.Join(g.DataDir, sub), idMap) + if cerr != nil { + sess.WriteLine(fmt.Sprintf("Error scanning %s for room references: %v", sub, cerr)) + return + } + for _, ch := range changes { + if werr := rewriteTxn.Replace(ch.Path, ch.Content, 0644); werr != nil { + sess.WriteLine(fmt.Sprintf("Error rewriting %s: %v", ch.Path, werr)) + return + } + contentUpdated++ + } + } if rerr := rewriteTxn.Commit(); rerr != nil { sess.WriteLine(fmt.Sprintf("Error rewriting room references: %v", rerr)) return @@ -162,5 +184,5 @@ func (g *Game) executeSwapID(sess *net.Session, args []string, rawInput string) // from the owner command loop) and must not dispatch again. g.MigrateRoomIDsOnOwner(idMap) - sess.WriteLine(fmt.Sprintf("Swapped room IDs %d and %d (%d files updated).", id1, id2, updated)) + sess.WriteLine(fmt.Sprintf("Swapped room IDs %d and %d (%d room files, %d content files updated).", id1, id2, updated, contentUpdated)) } diff --git a/internal/game/core_production.go b/internal/game/core_production.go index 7a4b6e9..3aaa6b5 100644 --- a/internal/game/core_production.go +++ b/internal/game/core_production.go @@ -109,11 +109,11 @@ var productionTypes = map[string]productionTypeInfo{ }, } -var productionActionTypes map[string]bool +var productionActionTypes map[behavior.ActionType]bool func init() { - productionActionTypes = make(map[string]bool) + productionActionTypes = make(map[behavior.ActionType]bool) for _, pt := range productionTypes { - productionActionTypes[string(pt.ActionType)] = true + productionActionTypes[pt.ActionType] = true } } diff --git a/internal/game/exit_migrate.go b/internal/game/exit_migrate.go index 66fccfe..f6df05d 100644 --- a/internal/game/exit_migrate.go +++ b/internal/game/exit_migrate.go @@ -1,6 +1,7 @@ package game import ( + "log" "strconv" "strings" @@ -190,4 +191,18 @@ func (g *Game) MigrateRoomIDsOnOwner(idMap map[int]int) { // lookups agree with the post-migration layout. g.World.RebuildRoomIndex(g.DataDir) g.RewriteRoomIDs(idMap) + + // Non-room content files (courses, modules, file-backed triggers) were + // rewritten with the same idMap. Reload their in-memory caches so gameplay + // follows the rename immediately instead of at the next restart. In-flight + // sequences hold pointers into the old trigger/step data, so abort them the + // same way a full content reload does. + if g.CourseStore != nil { + g.CourseStore.ReloadAll() + } + if err := g.LoadMods(); err != nil { + log.Printf("room-id migration: reload modules: %v", err) + } + g.abortAllSequences() + g.loadAllFlagTriggers() } diff --git a/internal/game/swapid_content_test.go b/internal/game/swapid_content_test.go new file mode 100644 index 0000000..f1eb978 --- /dev/null +++ b/internal/game/swapid_content_test.go @@ -0,0 +1,64 @@ +package game + +// Regression: `swapid` rewrites structural room references in non-room content +// files (modules, triggers, items, ...) as well as in data/rooms. Before this, +// only room YAML was scanned, so a transport destination or trigger teleport +// silently kept pointing at the pre-swap ID. + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSwapIDRewritesNonRoomContent(t *testing.T) { + h := newHarness(t) + h.seedAccount("admin") + h.seedCharacter("admin", "admin") + acc, err := h.g.AccountStore.LoadAccount("admin") + if err != nil { + t.Fatal(err) + } + acc.Admin = true + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatal(err) + } + sess, _ := h.connect("admin") + + // A transport module destination and an author value that equals a room ID. + modulesDir := filepath.Join(h.dir, "modules") + if err := os.MkdirAll(modulesDir, 0o755); err != nil { + t.Fatal(err) + } + modulePath := filepath.Join(modulesDir, "transport_home.yaml") + if err := os.WriteFile(modulePath, []byte("name: Home\ncategory: transport\ndestination: 1\n"), 0o644); err != nil { + t.Fatal(err) + } + itemsDir := filepath.Join(h.dir, "items") + if err := os.MkdirAll(itemsDir, 0o755); err != nil { + t.Fatal(err) + } + itemPath := filepath.Join(itemsDir, "gem.yaml") + if err := os.WriteFile(itemPath, []byte("name: gem\nvalue: 1\n"), 0o644); err != nil { + t.Fatal(err) + } + + h.g.RunOnOwner(func() { h.g.executeSwapID(sess, []string{"1", "2"}, "swapid 1 2") }) + + module, err := os.ReadFile(modulePath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(module), "destination: 2") { + t.Errorf("module destination not swapped: %s", module) + } + // value: 1 is an author value, not a structural room reference. + item, err := os.ReadFile(itemPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(item), "value: 1") { + t.Errorf("author value equal to a swapped ID was rewritten: %s", item) + } +} diff --git a/internal/game/tick.go b/internal/game/tick.go index a57ea26..6c3eb7d 100644 --- a/internal/game/tick.go +++ b/internal/game/tick.go @@ -208,7 +208,7 @@ func (g *Game) SharedDepletionTick() { activeKeys := make(map[string]bool) for _, sess := range g.Hub.AllSessions() { p := sess.Player - if p == nil || p.Action == nil || p.Action.Type != "gather" { + if p == nil || p.Action == nil || p.Action.Type != behavior.TypeGather { continue } if gd, ok := p.Action.Data.(*behavior.GatherData); ok { -- cgit v1.2.3