feat: old standalone trigger systems completely unified into trigger->condition->action system

2 files changed, 173 insertions, 119 deletions

diff --git a/internal/validate/checks.go b/internal/validate/checks.go
index 2e70323..c1e8f19 100644
--- a/internal/validate/checks.go
+++ b/internal/validate/checks.go
@@ -67,11 +67,9 @@ func validateRooms(s Source) []Issue {
 					})
 				}
 			}
-			for i, it := range exit.OnTraverse {
-				issues = append(issues, validateStepAction(
-					fmt.Sprintf("Room %d: exit %q on_traverse[%d]", id, dir, i),
-					it.Action, itemIDs, roomIndex, mobIDs)...)
-			}
+			issues = append(issues, validateTriggerBlock(
+				fmt.Sprintf("Room %d: exit %q on_traverse", id, dir),
+				exit.OnTraverse, false, itemIDs, roomIndex, mobIDs)...)
 		}
 
 		for _, rm := range room.Mobs {
@@ -223,7 +221,7 @@ func validateLocalObject(roomID int, robj world.RoomObject, itemIDs map[string]b
 	}
 
 	if len(def.OnLook) > 0 {
-		issues = append(issues, validateOnLook(prefix+": on_look", def.OnLook, itemIDs, roomIndex, mobIDs)...)
+		issues = append(issues, validateTriggerBlock(prefix+": on_look", def.OnLook, false, itemIDs, roomIndex, mobIDs)...)
 	}
 
 	return issues
@@ -487,10 +485,8 @@ func validateMobs(s Source) []Issue {
 			}
 		}
 
-		for i, it := range def.OnKill {
-			issues = append(issues, validateStepAction(
-				fmt.Sprintf("Mob %q: on_kill[%d]", id, i), it.Action, itemIDs, roomIndex, mobIDs)...)
-		}
+		issues = append(issues, validateTriggerBlock(
+			fmt.Sprintf("Mob %q: on_kill", id), def.OnKill, false, itemIDs, roomIndex, mobIDs)...)
 	}
 
 	return issues
@@ -594,19 +590,8 @@ func validateObjects(s Source) []Issue {
 			}
 		}
 
-		for _, ui := range obj.OnUse {
-			if ui.Item != "" && !itemIDs[ui.Item] {
-				issues = append(issues, Issue{
-					Level: "ERROR",
-					Type:  "reference",
-					Message: fmt.Sprintf("Object %q: on_use item %q does not exist",
-						id, ui.Item),
-				})
-			}
-			if ui.Action != nil {
-				issues = append(issues, validateStepAction(fmt.Sprintf("Object %q: on_use", id), ui.Action, itemIDs, roomIndex, mobIDs)...)
-			}
-		}
+		issues = append(issues, validateTriggerBlock(
+			fmt.Sprintf("Object %q: on_use", id), obj.OnUse, false, itemIDs, roomIndex, mobIDs)...)
 
 		if obj.Gather != nil {
 			issues = append(issues, validateGather(fmt.Sprintf("Object %q: gather", id), obj.Gather, itemIDs, dropIDs)...)
@@ -617,7 +602,7 @@ func validateObjects(s Source) []Issue {
 		}
 
 		if len(obj.OnLook) > 0 {
-			issues = append(issues, validateOnLook(fmt.Sprintf("Object %q: on_look", id), obj.OnLook, itemIDs, roomIndex, mobIDs)...)
+			issues = append(issues, validateTriggerBlock(fmt.Sprintf("Object %q: on_look", id), obj.OnLook, false, itemIDs, roomIndex, mobIDs)...)
 		}
 	}
 
@@ -802,7 +787,10 @@ func validateCourses(s Source) []Issue {
 	return issues
 }
 
-func validateRoomEnterSteps(s Source) []Issue {
+// validateRoomTriggers validates every trigger-shaped block on every room:
+// on_enter, on_exit (verb blocks), and on_flag_change / on_global_flag_change
+// (flag blocks, which require on_player_flag/on_global_flag).
+func validateRoomTriggers(s Source) []Issue {
 	var issues []Issue
 	roomIndex := s.World.RoomIndex()
 	itemIDs := s.Items.IDSet()
@@ -810,35 +798,24 @@ func validateRoomEnterSteps(s Source) []Issue {
 
 	for id := range roomIndex {
 		room, err := s.World.LoadRoom(id)
-		if err != nil || len(room.OnEnter) == 0 {
+		if err != nil {
 			continue
 		}
-		for si := range room.OnEnter {
-			stepPrefix := fmt.Sprintf("Room %d: on_enter[%d]", id, si)
-			issues = append(issues, validateStepAction(stepPrefix, &room.OnEnter[si], itemIDs, roomIndex, mobIDs)...)
+		if len(room.OnEnter) > 0 {
+			issues = append(issues, validateTriggerBlock(
+				fmt.Sprintf("Room %d: on_enter", id), room.OnEnter, false, itemIDs, roomIndex, mobIDs)...)
 		}
-	}
-
-	return issues
-}
-
-func validateRoomTriggers(s Source) []Issue {
-	var issues []Issue
-	roomIndex := s.World.RoomIndex()
-	itemIDs := s.Items.IDSet()
-	mobIDs := s.Mobs.AllDefIDs()
-
-	for id := range roomIndex {
-		room, err := s.World.LoadRoom(id)
-		if err != nil || len(room.Triggers) == 0 {
-			continue
+		if len(room.OnExit) > 0 {
+			issues = append(issues, validateTriggerBlock(
+				fmt.Sprintf("Room %d: on_exit", id), room.OnExit, false, itemIDs, roomIndex, mobIDs)...)
 		}
-		for ti, trigger := range room.Triggers {
-			prefix := fmt.Sprintf("Room %d: trigger[%d]", id, ti)
-			for si := range trigger.Steps {
-				stepPrefix := fmt.Sprintf("%s: step[%d]", prefix, si)
-				issues = append(issues, validateStepAction(stepPrefix, &trigger.Steps[si], itemIDs, roomIndex, mobIDs)...)
-			}
+		if len(room.OnFlagChange) > 0 {
+			issues = append(issues, validateTriggerBlock(
+				fmt.Sprintf("Room %d: on_flag_change", id), room.OnFlagChange, true, itemIDs, roomIndex, mobIDs)...)
+		}
+		if len(room.OnGlobalFlagChange) > 0 {
+			issues = append(issues, validateTriggerBlock(
+				fmt.Sprintf("Room %d: on_global_flag_change", id), room.OnGlobalFlagChange, true, itemIDs, roomIndex, mobIDs)...)
 		}
 	}
 
@@ -1128,7 +1105,7 @@ func validateTalkConfig(prefix string, cfg *behavior.TalkConfig, itemIDs map[str
 	for nodeID, node := range cfg.Nodes {
 		nodePrefix := fmt.Sprintf("%s: node %q", prefix, nodeID)
 		if node.Action != nil {
-			issues = append(issues, validateNodeAction(nodePrefix, node.Action, itemIDs, roomIndex)...)
+			issues = append(issues, validateTalkStep(nodePrefix, node.Action, itemIDs, roomIndex)...)
 		}
 		if node.Goto != "" {
 			if _, ok := cfg.Nodes[node.Goto]; !ok {
@@ -1142,7 +1119,7 @@ func validateTalkConfig(prefix string, cfg *behavior.TalkConfig, itemIDs map[str
 		for i, opt := range node.Options {
 			optPrefix := fmt.Sprintf("%s: option %d", nodePrefix, i+1)
 			if opt.Action != nil {
-				issues = append(issues, validateNodeAction(optPrefix, opt.Action, itemIDs, roomIndex)...)
+				issues = append(issues, validateTalkStep(optPrefix, opt.Action, itemIDs, roomIndex)...)
 			}
 		}
 	}
@@ -1196,66 +1173,25 @@ func validateExitReciprocity(s Source) []Issue {
 
 	return issues
 }
-func validateNodeAction(prefix string, na *behavior.NodeAction, itemIDs map[string]bool, roomIndex map[int]bool) []Issue {
-	var issues []Issue
-	if na == nil {
-		return issues
-	}
 
-	if na.GiveItem != "" && !itemIDs[na.GiveItem] {
-		issues = append(issues, Issue{
-			Level: "ERROR",
-			Type:  "reference",
-			Message: fmt.Sprintf("%s: give_item %q does not exist",
-				prefix, na.GiveItem),
-		})
-	}
-
-	if na.TakeItem != "" && !itemIDs[na.TakeItem] {
-		issues = append(issues, Issue{
-			Level: "ERROR",
-			Type:  "reference",
-			Message: fmt.Sprintf("%s: take_item %q does not exist",
-				prefix, na.TakeItem),
-		})
-	}
-
-	if na.Teleport > 0 {
-		if _, ok := roomIndex[na.Teleport]; !ok {
-			issues = append(issues, Issue{
-				Level: "ERROR",
-				Type:  "reference",
-				Message: fmt.Sprintf("%s: teleport to nonexistent room %d",
-					prefix, na.Teleport),
-			})
-		}
-	}
-
-	return issues
-}
-
-// validateStepAction validates the universal effect superset used by on_use,
-// on_look, on_kill, on_enter steps, trigger steps, and exit
-// traversal. It covers the inline NodeAction fields plus spawn_mob/despawn_mob
-// (when mobIDs is non-nil). Returns the list of issues found.
-func validateStepAction(prefix string, step *behavior.StepAction, itemIDs map[string]bool, roomIndex map[int]bool, mobIDs map[string]bool) []Issue {
+// validateStep validates a single Step's effects. mobIDs may be nil when the
+// caller doesn't track mob ids (e.g. local-object on_look); spawn/despawn mob
+// references are then skipped.
+func validateStep(prefix string, step *behavior.Step, itemIDs map[string]bool, roomIndex map[int]bool, mobIDs map[string]bool) []Issue {
 	var issues []Issue
 	if step == nil {
 		return issues
 	}
 
-	for i, msg := range step.Messages {
-		if msg.Delay < 0 {
-			issues = append(issues, Issue{
-				Level: "WARN",
-				Type:  "semantic",
-				Message: fmt.Sprintf("%s: messages[%d].delay is negative (%d)",
-					prefix, i, msg.Delay),
-			})
-		}
+	if step.Wait < 0 {
+		issues = append(issues, Issue{
+			Level: "WARN",
+			Type:  "semantic",
+			Message: fmt.Sprintf("%s: wait is negative (%d)",
+				prefix, step.Wait),
+		})
 	}
 
-	// Validate the embedded NodeAction subset.
 	if step.GiveItem != "" && !itemIDs[step.GiveItem] {
 		issues = append(issues, Issue{
 			Level: "ERROR",
@@ -1283,8 +1219,6 @@ func validateStepAction(prefix string, step *behavior.StepAction, itemIDs map[st
 		}
 	}
 
-	// Spawn mob: validate mob def + despawn_rooms (only if mobIDs populated —
-	// a nil map means the caller doesn't track mob ids, e.g. local objects).
 	if step.SpawnMob != nil && step.SpawnMob.ID != "" {
 		if mobIDs != nil && !mobIDs[step.SpawnMob.ID] {
 			issues = append(issues, Issue{
@@ -1306,7 +1240,6 @@ func validateStepAction(prefix string, step *behavior.StepAction, itemIDs map[st
 		}
 	}
 
-	// Despawn mob.
 	if step.DespawnMob != "" && mobIDs != nil && !mobIDs[step.DespawnMob] {
 		issues = append(issues, Issue{
 			Level: "ERROR",
@@ -1316,26 +1249,148 @@ func validateStepAction(prefix string, step *behavior.StepAction, itemIDs map[st
 		})
 	}
 
+	if step.Condition != nil {
+		issues = append(issues, validateCondition(prefix, step.Condition, roomIndex)...)
+	}
 	return issues
 }
 
-// validateOnLook validates a list of on_look interactions (now a list, not a
-// single NodeAction). For each entry, validates the optional item_id (unused
-// on on_look but permitted), the action's effects, and skips (the entry's
-// condition is a runtime gate, not a static referential ref).
-func validateOnLook(prefix string, list []behavior.Interaction, itemIDs map[string]bool, roomIndex map[int]bool, mobIDs map[string]bool) []Issue {
+// validateCondition checks a Condition tree's Room references (structural room
+// IDs) and recurses into all_of/any_of.
+func validateCondition(prefix string, c *behavior.Condition, roomIndex map[int]bool) []Issue {
+	if c == nil {
+		return nil
+	}
 	var issues []Issue
-	for i, it := range list {
-		entryPrefix := fmt.Sprintf("%s[%d]", prefix, i)
-		if it.Item != "" && !itemIDs[it.Item] {
+	if c.Room != 0 {
+		if _, ok := roomIndex[c.Room]; !ok {
 			issues = append(issues, Issue{
 				Level: "ERROR",
 				Type:  "reference",
-				Message: fmt.Sprintf("%s: item_id %q does not exist",
-					entryPrefix, it.Item),
+				Message: fmt.Sprintf("%s: condition room references nonexistent room %d",
+					prefix, c.Room),
+			})
+		}
+	}
+	for i := range c.AllOf {
+		issues = append(issues, validateCondition(prefix, &c.AllOf[i], roomIndex)...)
+	}
+	for i := range c.AnyOf {
+		issues = append(issues, validateCondition(prefix, &c.AnyOf[i], roomIndex)...)
+	}
+	return issues
+}
+
+// checkPlayerOnlyInCondition warns when a global-scope condition uses
+// player-dependent predicates that cannot be evaluated without a player.
+func checkPlayerOnlyInCondition(prefix string, c *behavior.Condition) []Issue {
+	if c == nil {
+		return nil
+	}
+	var issues []Issue
+	if c.PlayerFlag != "" {
+		issues = append(issues, Issue{
+			Level:   "WARN",
+			Type:    "semantic",
+			Message: fmt.Sprintf("%s: player_flag %q has no effect on global-flag triggers (no player)", prefix, c.PlayerFlag),
+		})
+	}
+	if c.HasItem != "" {
+		issues = append(issues, Issue{
+			Level:   "WARN",
+			Type:    "semantic",
+			Message: fmt.Sprintf("%s: has_item %q has no effect on global-flag triggers (no player)", prefix, c.HasItem),
+		})
+	}
+	if c.MinCredits > 0 {
+		issues = append(issues, Issue{
+			Level:   "WARN",
+			Type:    "semantic",
+			Message: fmt.Sprintf("%s: min_credits has no effect on global-flag triggers (no player)", prefix),
+		})
+	}
+	for i := range c.AllOf {
+		issues = append(issues, checkPlayerOnlyInCondition(prefix, &c.AllOf[i])...)
+	}
+	for i := range c.AnyOf {
+		issues = append(issues, checkPlayerOnlyInCondition(prefix, &c.AnyOf[i])...)
+	}
+	return issues
+}
+
+// validateTriggerBlock validates a list of Trigger entries for one event block.
+// isFlag is true for on_flag_change / on_global_flag_change blocks (which
+// require on_player_flag/on_global_flag and forbid item_id) and false for verb
+// blocks (on_use/on_look/on_kill/on_enter/on_exit/on_traverse), where item_id
+// is checked against itemIDs.
+func validateTriggerBlock(prefix string, block []behavior.Trigger, isFlag bool, itemIDs map[string]bool, roomIndex map[int]bool, mobIDs map[string]bool) []Issue {
+	var issues []Issue
+	for i := range block {
+		t := &block[i]
+		entryPrefix := fmt.Sprintf("%s[%d]", prefix, i)
+		if isFlag {
+			if t.OnPlayerFlag == "" && t.OnGlobalFlag == "" {
+				issues = append(issues, Issue{
+					Level: "WARN",
+					Type:  "semantic",
+					Message: fmt.Sprintf("%s: flag-change trigger has no on_player_flag/on_global_flag — it can never fire",
+						entryPrefix),
+				})
+			}
+			if t.ItemID != "" {
+				issues = append(issues, Issue{
+					Level: "WARN",
+					Type:  "semantic",
+					Message: fmt.Sprintf("%s: item_id is ignored on flag-change triggers",
+						entryPrefix),
+				})
+			}
+		} else {
+			if t.OnPlayerFlag != "" || t.OnGlobalFlag != "" {
+				issues = append(issues, Issue{
+					Level: "WARN",
+					Type:  "semantic",
+					Message: fmt.Sprintf("%s: on_player_flag/on_global_flag are ignored on verb triggers",
+						entryPrefix),
+				})
+			}
+			if t.ItemID != "" && !itemIDs[t.ItemID] {
+				issues = append(issues, Issue{
+					Level: "ERROR",
+					Type:  "reference",
+					Message: fmt.Sprintf("%s: item_id %q does not exist",
+						entryPrefix, t.ItemID),
+				})
+			}
+		}
+		if t.Lock && len(t.Steps) == 0 {
+			issues = append(issues, Issue{
+				Level:   "WARN",
+				Type:    "semantic",
+				Message: fmt.Sprintf("%s: lock is true but the trigger has no steps", entryPrefix),
 			})
 		}
-		issues = append(issues, validateStepAction(entryPrefix, it.Action, itemIDs, roomIndex, mobIDs)...)
+		if t.Condition != nil {
+			issues = append(issues, validateCondition(entryPrefix, t.Condition, roomIndex)...)
+		}
+		if isFlag && t.OnGlobalFlag != "" && t.Condition != nil {
+			issues = append(issues, checkPlayerOnlyInCondition(entryPrefix, t.Condition)...)
+		}
+		for si := range t.Steps {
+			issues = append(issues, validateStep(fmt.Sprintf("%s: step[%d]", entryPrefix, si), &t.Steps[si], itemIDs, roomIndex, mobIDs)...)
+			if isFlag && t.OnGlobalFlag != "" && t.Steps[si].Condition != nil {
+				issues = append(issues, checkPlayerOnlyInCondition(fmt.Sprintf("%s: step[%d]", entryPrefix, si), t.Steps[si].Condition)...)
+			}
+		}
 	}
 	return issues
 }
+
+// validateTalkStep validates a talk node/option Action (a single Step, no mob
+// spawn/despawn expected — mobIDs nil so those are skipped).
+func validateTalkStep(prefix string, step *behavior.Step, itemIDs map[string]bool, roomIndex map[int]bool) []Issue {
+	if step == nil {
+		return nil
+	}
+	return validateStep(prefix, step, itemIDs, roomIndex, nil)
+}
diff --git a/internal/validate/validate.go b/internal/validate/validate.go
index 6558216..234651e 100644
--- a/internal/validate/validate.go
+++ b/internal/validate/validate.go
@@ -77,7 +77,6 @@ func Run(s Source) []Issue {
 	issues = append(issues, validateDropTables(s)...)
 	issues = append(issues, validateCourses(s)...)
 	issues = append(issues, validateRoomTriggers(s)...)
-	issues = append(issues, validateRoomEnterSteps(s)...)
 	issues = append(issues, validateTechs(s)...)
 	issues = append(issues, validateRoomWiring(s)...)
 	issues = append(issues, validateRoomGrid(s)...)