BreakEscape/SCENARIO_JSON_FORMAT_AUDIT.md

# Scenario JSON Format Audit - Complete Report

## Summary

Compared `npc-patrol-lockpick.json` vs `test-npc-patrol.json` and identified **4 major structural issues** that have been **FIXED**.

---

## Issues Identified and Fixed

### 1. ❌ → ✅ Root-Level Properties

| Property | Before | After | Status |
|----------|--------|-------|--------|
| `globalVariables` | Present | Removed | ✅ Fixed |
| `startItemsInInventory` | Present | Removed | ✅ Fixed |
| `endGoal` | Missing | Added | ✅ Fixed |

**Why it matters:** `endGoal` is a standard scenario property used by the game framework.

---

### 2. ❌ → ✅ First NPC Structure (`patrol_with_face`)

**Problem:** Event mappings were inside `behavior` object with trailing comma

**Before:**
```json
"behavior": {
  "facePlayer": true,
  "patrol": { ... }
},                        // ← Trailing comma (syntax error)
"eventMappings": [ ... ]  // ← Incorrectly nested
```

**After:**
```json
"behavior": {
  "facePlayer": true,
  "patrol": {
    "enabled": true,
    "speed": 100,
    "changeDirectionInterval": 4000,
    "bounds": { ... }
  }
},
"los": { ... },
"eventMappings": [ ... ], // ← At NPC root, correct nesting
"_comment": "..."
```

**Changes:**
- ✅ Removed trailing comma
- ✅ Moved `eventMappings` to NPC root
- ✅ Added `enabled` flag to patrol config
- ✅ Reordered properties for clarity

---

### 3. ❌ → ✅ Second NPC Structure (`security_guard`)

**Problem:** `patrol` object was at NPC root instead of inside `behavior`

**Before:**
```json
{
  "los": { ... },
  "patrol": {                 // ← WRONG: At NPC root!
    "route": [ ... ],
    "speed": 40,
    "pauseTime": 10
  },
  "eventMappings": [ ... ]
}
```

**After:**
```json
{
  "behavior": {               // ← CORRECT: Patrol inside behavior
    "patrol": {
      "route": [ ... ],
      "speed": 40,
      "pauseTime": 10
    }
  },
  "los": { ... },
  "eventMappings": [ ... ],
  "_comment": "..."
}
```

**Changes:**
- ✅ Wrapped `patrol` inside new `behavior` object
- ✅ Removed trailing comma
- ✅ Moved `eventMappings` to NPC root
- ✅ Reordered properties for clarity

---

### 4. ❌ → ✅ Property Ordering

**Before:** Mixed and inconsistent ordering

**After:** Standardized ordering for all NPCs:
1. Basic info: `id`, `displayName`, `npcType`
2. Position: `position`
3. Display: `spriteSheet`, `spriteTalk`, `spriteConfig`
4. Story: `storyPath`, `currentKnot`
5. **Behavior:** `behavior` (contains `patrol` and `facePlayer`)
6. Detection: `los`
7. Events: `eventMappings`
8. Documentation: `_comment`

---

## Detailed Format Reference

### Correct NPC Structure (Both NPCs Now Follow)

```json
{
  "id": "patrol_with_face",
  "displayName": "Patrol + Face Player",
  "npcType": "person",
  "position": { "x": 5, "y": 5 },
  "spriteSheet": "hacker",
  "spriteConfig": {
    "idleFrameStart": 20,
    "idleFrameEnd": 23
  },
  "storyPath": "scenarios/ink/security-guard.json",
  "currentKnot": "start",
  
  "behavior": {
    "facePlayer": true,
    "facePlayerDistance": 96,
    "patrol": {
      "enabled": true,
      "speed": 100,
      "changeDirectionInterval": 4000,
      "bounds": {
        "x": 128,
        "y": 128,
        "width": 128,
        "height": 128
      }
    }
  },
  
  "los": {
    "enabled": true,
    "range": 250,
    "angle": 120,
    "visualize": true
  },
  
  "eventMappings": [
    {
      "eventPattern": "lockpick_used_in_view",
      "targetKnot": "on_lockpick_used",
      "conversationMode": "person-chat",
      "cooldown": 0
    }
  ],
  
  "_comment": "Description of NPC behavior"
}
```

---

## Validation Results

### ✅ File: `npc-patrol-lockpick.json`

- **Status:** FIXED AND VALID
- **JSON Syntax:** ✅ Valid
- **Property Structure:** ✅ Correct
- **NPC Format:** ✅ Matches template
- **Ready for Testing:** ✅ Yes

### Files for Reference

| File | Format | Status |
|------|--------|--------|
| `test-npc-patrol.json` | ✅ Correct | Reference |
| `npc-patrol-lockpick.json` | ✅ Fixed | Ready to use |

---

## Key Rules Enforced

### NPC Structure Rules

1. **`patrol` MUST be inside `behavior`**
   ```json
   ✅ npc.behavior.patrol
   ❌ npc.patrol
   ```

2. **No trailing commas after objects**
   ```json
   ✅ { "a": 1 }
   ❌ { "a": 1, }
   ```

3. **`eventMappings` at NPC root**
   ```json
   ✅ npc.eventMappings
   ❌ npc.behavior.eventMappings
   ```

4. **`los` at NPC root**
   ```json
   ✅ npc.los
   ❌ npc.behavior.los
   ```

5. **All properties properly ordered**
   - Basic info first
   - `behavior` contains patrol/interaction logic
   - Detection/events after behavior

---

## Documentation Created

Created 3 comprehensive guides:

1. **`docs/SCENARIO_FORMAT_COMPARISON.md`**
   - Before/after examples
   - Detailed explanation of each fix
   - Template for correct NPC structure

2. **`docs/JSON_SYNTAX_ERRORS_EXPLAINED.md`**
   - Visual error examples
   - Why each error is wrong
   - JSON validation tips
   - Online validators listed

3. **`SCENARIO_FORMAT_FIXES.md`** (this directory)
   - Complete summary of fixes
   - Validation checklist
   - Reference structure

---

## Testing the Fixed Scenario

To verify the fixes work correctly:

```bash
# 1. Validate JSON syntax
python3 -m json.tool scenarios/npc-patrol-lockpick.json

# 2. Load scenario in game
# Open: scenario_select.html
# Select: npc-patrol-lockpick
# Click: Start Scenario

# 3. Test NPC behavior in console
window.enableLOS()  # Should show green cones

# 4. Verify patrol and detection
# - Both NPCs should patrol
# - Green cones should appear
# - Lockpicking should trigger person-chat
```

---

## Before vs After Comparison

### Root Level

```diff
  {
    "scenario_brief": "Test scenario for NPC patrol and lockpick detection",
-   "globalVariables": {
-     "player_caught_lockpicking": false
-   },
+   "endGoal": "Test NPC line-of-sight detection and lockpicking interruption",
    "startRoom": "patrol_corridor",
-   "startItemsInInventory": [],
    
    "player": { ... }
  }
```

### First NPC

```diff
    "storyPath": "scenarios/ink/security-guard.json",
    "currentKnot": "start",
+   "behavior": {
      "facePlayer": true,
      "facePlayerDistance": 96,
      "patrol": {
+       "enabled": true,
        "speed": 100,
        "changeDirectionInterval": 4000,
        "bounds": { ... }
      }
-   },
+   },
    "los": { ... },
    "eventMappings": [ ... ],
+   "_comment": "..."
```

### Second NPC

```diff
    "storyPath": "scenarios/ink/security-guard.json",
    "currentKnot": "start",
+   "behavior": {
      "patrol": {
        "route": [ ... ],
        "speed": 40,
        "pauseTime": 10
      }
-   },
+   },
    "los": { ... },
    "eventMappings": [ ... ],
+   "_comment": "..."
```

---

## Quick Reference Checklist

When creating NPC definitions:

- [ ] `behavior` object created first
- [ ] `patrol` placed inside `behavior`
- [ ] No trailing commas anywhere
- [ ] `los` at NPC root (not in `behavior`)
- [ ] `eventMappings` at NPC root
- [ ] All properties properly quoted
- [ ] Brackets/braces properly matched
- [ ] Properties in standard order
- [ ] Optional `_comment` for documentation

---

## Next Steps

1. ✅ **Format fixed** - JSON is now valid and properly structured
2. ✅ **Documentation created** - Guides for future reference
3. 🔄 **Ready to test** - Scenario can be loaded in game
4. 📚 **Guidelines established** - Format rules documented

The scenario is now in correct format and ready for testing!