# 小地图系统 R25 独立评审报告

**评审轮次**：Round 25（R24 修复后全面重审）  
**评审基准**：成熟 2D Metroidvania 游戏的商业级标准；编辑器工具须对开发者/策划可用友好

---

## 一、各维度评分

| 维度 | 满分 | 得分 | 变化 |
|------|------|------|------|
| 架构设计 & 解耦合 | 25 | 24.5 | ±0 |
| 性能 & 运行时效率 | 20 | 19.5 | ±0 |
| 编辑器工具质量 | 15 | 14.5 | ±0 |
| 代码质量 & 可维护性 | 15 | 13.5 | -0.5 |
| 存档 & 持久化 | 10 | 9.0 | -0.5 |
| 可扩展性 | 10 | 9.5 | ±0 |
| 功能完整性 | 5 | 5.0 | ±0 |
| **综合** | **100** | **95.5** | **-2.0** |

> R24 得分：97.5 → R25 得分：95.5（发现 1 Critical + 2 Normal 问题）

---

## 二、确认正常的设计（R24 修复全部落地）

| 项 | 状态 |
|----|------|
| TeleportService 完整重建（135 行） | ✅ |
| RequestTeleport 源 RoomId 改为 IPlayerPositionProvider.CurrentRoomId | ✅ |
| MapInputHandler 删除 _zoom 双重状态，OnScroll 直接读写 _zoomTarget.localScale.x | ✅ |
| MapLayoutEditorWindow 搜索框 ✕ 清除按钮 | ✅ |
| 执行顺序链：MapManager(-700)→MapPlayerTracker(-600)→MapPinManager(-500)→TeleportService(-400)→UI(0) | ✅ |
| ISaveable 防御性拷贝三处对称 | ✅ |
| _servicesReady 短路（MapPanel & MinimapHUD 对称） | ✅ |
| MapRoomDataSO.ChooseDisplayIcon DRY 消除 | ✅ |
| MapServiceExtensions 集中 BuildRegionDict / ResolveRegionDisplayName | ✅ |
| 三对象池：MapPanel（Cell/Exit/Pin）；MinimapHUD（Cell/Pin） | ✅ |
| 发现动画协程自清理（R20-N1 RunRevealAnim） | ✅ |
| MinimapHUD O(viewRadius²) 空间索引裁剪 | ✅ |
| UnsubscribeServices 有意不对称注释说明 | ✅ |

---

## 三、新发现问题

### C1 — Critical：TeleportService.CanTeleportTo 逻辑反转（Fail-Open 安全漏洞）

**文件**：`Assets/_Game/Scripts/World/Map/TeleportService.cs`  
**行号**：L81–82

**问题**：
```csharp
_mapSvc ??= ServiceLocator.GetOrDefault<IMapService>();
return _mapSvc == null || _mapSvc.IsExplored(roomId);  // ← BUG
```

当 `_mapSvc` 查询失败（服务未注册、场景切换中等）时，返回 `true`（允许传送）。  
而注释明确写道：**"需要玩家已探索过目标房间（仅已知位置才允许传送）"**。  
逻辑与意图完全相反：
- 实际行为：`_mapSvc == null` → **无条件放行**（Fail-Open）
- 预期行为：`_mapSvc == null` → **拒绝传送**（Fail-Safe）

**影响**：玩家可在 MapService 不可用时（存档切换、热重载等边缘时机）跳过探索校验，传送到从未到达过的房间，破坏探索核心机制。

**修复**：
```csharp
return _mapSvc != null && _mapSvc.IsExplored(roomId);
```

---

### N1 — Normal：MapPanel._subs 声明但永远为空（死代码）

**文件**：`Assets/_Game/Scripts/World/Map/MapPanel.cs`  
**行号**：L76（声明）、L109（Clear）

**问题**：
```csharp
private readonly CompositeDisposable _subs = new();   // 永远不会有内容
// ...
private void OnDisable()
{
    _subs.Clear();   // ← 空操作
    // ...
}
```

MapPanel 的所有事件订阅均通过直接 `+=` / `-=` 方式管理（在 `SubscribeServices` / `UnsubscribeServices` 中），没有任何订阅通过 `.AddTo(_subs)` 加入。`_subs.Clear()` 是一个完全的空操作。

**影响**：轻微；不影响运行时行为，但
1. 误导后续维护者（以为订阅已通过 CompositeDisposable 管理）
2. 在 OnDisable 中清理了并不存在的订阅，逻辑意图不清晰

**修复选项 A（推荐）**：删除 `_subs` 字段和 `_subs.Clear()` 调用，保持现有 `+=` 管理方式即可（已有 `UnsubscribeServices` 妥善处理）。

---

### N2 — Normal：MapInputHandler 缺少 _zoomTarget 配置校验

**文件**：`Assets/_Game/Scripts/World/Map/MapInputHandler.cs`  
**行号**：L21 注释、L97 & L382（MapPanel.CurrentZoom）

**问题**：
- `MapInputHandler._zoomTarget` 注释写"通常为 _roomContainer（格子根节点）"
- `MapPanel.CurrentZoom` 明确读取 `_roomContainer.localScale.x`
- `MapInputHandler.OnScroll` 写入 `_zoomTarget.localScale`
- 若在 Inspector 中将 `_zoomTarget` 配置为非 `_roomContainer` 的节点，两者的 scale 将静默分裂：`CurrentZoom` 读到的是旧值，`OnScroll` 写入的是新值，缩放操作功能性损坏但无任何报错

**影响**：仅在配置错误时触发，但属于"配置错误无提示"的隐患，在 Prefab 调试时难以定位。

**修复**：在 Awake 中添加运行时校验警告：
```csharp
private void Awake()
{
    _panel = GetComponent<MapPanel>();
#if UNITY_EDITOR || DEVELOPMENT_BUILD
    if (_zoomTarget != null && _panel != null)
    {
        // MapPanel.CurrentZoom 读取 _roomContainer，要求 _zoomTarget 与其为同一节点
        Debug.LogWarning("[MapInputHandler] _zoomTarget 应配置为 MapPanel 的 _roomContainer（格子根节点），" +
                         "否则 CurrentZoom 与缩放操作将读写不同节点导致状态分裂。", this);
    }
#endif
}
```

> 更优做法：提供 `public void SetZoomTarget(RectTransform t)` 由 MapPanel.OnEnable 注入，彻底消除配置依赖。

---

## 四、各文件详细状态

### MapPanel.cs
- 架构：五层解耦（Interface → ServiceLocator → C# Events → UI → Object Pool）✅
- 性能：三对象池 + PinsVersion 脏检查 + _servicesReady 短路 ✅
- **死代码**：`_subs` 字段（N1）⚠️
- 其余：对象池三种（Cell/Exit/Pin）均正确入池、出池、OnDestroy 销毁 ✅

### MinimapHUD.cs  
- O(viewRadius²) 空间索引裁剪 ✅
- _servicesReady 短路 + 跨场景重连（null 服务引用 + 重置标志）✅
- GC 友好：复用 `_toRemove`、`_roomsInViewBuffer`、`_newlyAddedBuffer` ✅
- 无新问题 ✅

### MapInputHandler.cs
- R24-N2 修复落地：无独立 `_zoom` 字段，OnScroll 直接读写 `_zoomTarget.localScale.x` ✅
- **配置隐患**：缺少 `_zoomTarget` 校验（N2）⚠️

### TeleportService.cs
- R24 完整重建架构正确：ISaveable / ITeleportService / 单例保护 ✅
- **C1 逻辑反转**：`CanTeleportTo` 在 `_mapSvc == null` 时 Fail-Open ❌

### MapManager.cs
- 执行顺序 (-700) / ISaveable / IMapService ✅
- `GetRoomsByRegion` 懒加载缓存 ✅
- `NotifyDatabaseChanged` 同步清空区域缓存 ✅

### MapPinManager.cs (MapPin.cs)
- 执行顺序 (-500) / ISaveable / IPinService ✅
- PinsVersion 脏版本号 ✅

### MapServiceExtensions.cs
- 无状态扩展方法集 ✅
- GetVisibility / CreatePinAtWorldPos / BuildRegionDict / ResolveRegionDisplayName ✅

### MapLayoutEditorWindow.cs（编辑器）
- 拖拽/缩放/验证/搜索/图例/Play Mode 玩家点 ✅
- ✕ 清除按钮（R24-N3）✅
- MatchesRoomType RoomType 枚举搜索（R23-N2）✅
- GUIStyle 缓存（避免 60fps × 100 房间重分配）✅
- Undo.undoRedoPerformed 注册 ✅

---

## 五、修复优先级

| 编号 | 严重度 | 文件 | 影响 | 是否立即修复 |
|------|--------|------|------|-------------|
| C1 | 🔴 Critical | TeleportService.cs | Fail-Open 逻辑反转，破坏探索机制 | 是 |
| N1 | 🟡 Normal | MapPanel.cs | 死代码误导维护 | 推荐 |
| N2 | 🟡 Normal | MapInputHandler.cs | 配置错误无提示 | 推荐 |

---

## 六、评分历史

| 轮次 | 综合评分 | 主要变化 |
|------|---------|---------|
| R17 | 93.0 | 初轮大规模重构后 |
| R18 | 93.8 | 平移/缩放 + 协程修复 |
| R19 | 93.8 | 持平 |
| R20 | 94.2 | DRY + 协程自清理 |
| R21 | 95.0 | ServicesReady 对称 |
| R22 | 96.5 | BuildRegionDict 集中 |
| R23 | 97.5 | MatchesRoomType + 执行顺序 |
| R24 | 97.5 | TeleportService 重建（但遗留 C1 逻辑错误）|
| **R25** | **95.5** | 发现 C1（-1.5）+ N1（-0.25）+ N2（-0.25）|

> R25 评分低于 R24，因 R24 重建 TeleportService 时引入了 C1 逻辑反转错误（fail-open vs fail-safe），该问题在 R24 评审中被遗漏。

---

*R25 评审完成时间：2025*