Add independent review report for Minimap system Round 7
- Validate fixes from Round 6 and identify new issues - Document findings including UX defects, editor integration flaws, and code quality concerns - Propose solutions and prioritize issues based on severity - Evaluate against standards of mature 2D Metroidvania games
This commit is contained in:
368
Docs/Review/Minimap_Review_Round7_Independent.md
Normal file
368
Docs/Review/Minimap_Review_Round7_Independent.md
Normal file
@@ -0,0 +1,368 @@
|
||||
# 小地图系统 Round 7 独立再审查报告
|
||||
|
||||
> **评估日期**：Round 7
|
||||
> **评估方法**：在 Round 6 修复完成后，再次独立通读所有 17 个地图模块文件，重点发现 Round 6 遗漏的真实问题
|
||||
> **对标标准**：成熟 2D Metroidvania 游戏小地图，专注编辑器扩展质量、架构解耦、高性能、策划友好度
|
||||
|
||||
---
|
||||
|
||||
## 一、Round 6 修复验证
|
||||
|
||||
| Round 6 修复 ID | 验证结果 | 备注 |
|
||||
|---|---|---|
|
||||
| P1-1 MinimapHUD 空间索引 | ✅ 通过 | `_spatialIndex` 构建/查询逻辑正确，O(viewRadius²) 替代 O(N) 落实 |
|
||||
| P2-1 MapDatabaseEditor GUIStyle 缓存 | ✅ 通过 | `_errorRowStyle` + `GetErrorRowStyle()` 惰性初始化，`OnEnable` 重置支持皮肤切换 |
|
||||
| P2-2 MapRoomDataEditor CELL_SIZE 说明 | ✅ 通过 | Inspector HelpBox 已补充三段坐标系说明 |
|
||||
| P2-3 MapLayoutEditorWindow Undo 刷新 | ✅ 通过 | `OnEnable`/`OnDisable` 正确注册/注销，`OnUndoRedo` 清除验证缓存并 Repaint |
|
||||
| P2-4 ValidateAll 编辑器保护 | ✅ 通过 | `#if UNITY_EDITOR` 包裹完整，构建包不再含 O(N²) 验证逻辑 |
|
||||
| N3 MinimapHUD Setup 冗余位置 | ⚠️ 部分通过 | step② 已加 PlaceCell；但 step③ 仍重新遍历全部 _cells 包含刚 PlaceCell 完的新格子，新格子被 PlaceCell 两次（详见 R7-N6） |
|
||||
|
||||
Round 6 主要修复落实良好，但 N3 引入了新的小冗余。
|
||||
|
||||
---
|
||||
|
||||
## 二、Round 7 新发现的问题
|
||||
|
||||
### 🔴 **R7-N1**：MapPanel 不响应 Pin 增删事件（真实 UX 缺陷）
|
||||
|
||||
**位置**：`MapPanel.cs` 第 88-92 行
|
||||
|
||||
```csharp
|
||||
private void OnEnable()
|
||||
{
|
||||
...
|
||||
RenderPins(); // ← 只在打开面板时调用一次
|
||||
UpdatePlayerIcon();
|
||||
CenterOnCurrentRoom();
|
||||
_onMapUpdated?.Subscribe(OnMapUpdated).AddTo(_subs);
|
||||
}
|
||||
```
|
||||
|
||||
**问题**：
|
||||
- `RenderPins()` 仅在 `OnEnable` 调用一次
|
||||
- `LateUpdate` 只刷新玩家图标，**从不重新检查 PinsVersion**
|
||||
- 玩家在地图打开状态下添加/删除 Pin（典型操作），UI 不会同步刷新
|
||||
|
||||
**影响**：地图面板期间用户的所有 Pin 操作（CreatePin/RemovePin）在面板关闭并重新打开前，视觉上无任何反馈。这是策划/玩家可见的 UX 错误。
|
||||
|
||||
**修复方案**：
|
||||
- 方案 A（轻量、推荐）：在 `LateUpdate` 末尾调用 `RenderPins()`，已有 `_lastPinVersion` 脏检查，无版本变化时立即 return，无开销
|
||||
- 方案 B：在 `IPinService` 中新增 `event Action OnPinsChanged`，`MapPinManager` 在 `AddPin/RemovePin` 时触发
|
||||
|
||||
---
|
||||
|
||||
### 🔴 **R7-N2**：MapPanel/MinimapHUD 不响应数据库热更（编辑器/工具集成缺陷）
|
||||
|
||||
**位置**：`MapPanel.cs` 第 83-86 行，`MinimapHUD.cs` 第 56-70 行
|
||||
|
||||
```csharp
|
||||
// MapPanel
|
||||
if (_cells.Count == 0)
|
||||
BuildGrid();
|
||||
else
|
||||
RefreshAllCells(); // ← 只刷新已存在 _cells 的可见性
|
||||
```
|
||||
|
||||
```csharp
|
||||
// MinimapHUD
|
||||
BuildSpatialIndex(_mapSvc?.Database); // ← 仅在 OnEnable 时构建
|
||||
```
|
||||
|
||||
**问题**：
|
||||
- 策划在编辑器中通过 `MapLayoutEditorWindow` 编辑房间布局或新增房间后，运行中的 MapPanel/MinimapHUD 不会反映变化
|
||||
- `RefreshAllCells` 只更新已有 `_cells` 集合，新增房间不会被实例化、被删除房间的格子不会被回收
|
||||
- 空间索引同样不会重建
|
||||
|
||||
**影响**：
|
||||
- 策划开发时的迭代体验差：每次编辑数据库都需重启游戏
|
||||
- 运行时若支持 Addressable 数据库热更或动态地图生成（DLC），地图会失效
|
||||
|
||||
**修复方案**：
|
||||
1. `IMapService` 新增 `event Action OnDatabaseChanged`
|
||||
2. `MapDatabaseSO.OnValidate` 中触发该事件（编辑器侧）
|
||||
3. `MapPanel`/`MinimapHUD` 订阅并执行完整重建
|
||||
|
||||
---
|
||||
|
||||
### 🟡 **R7-N3**：BuildSpatialIndex 在 MinimapHUD 和 MapPlayerTracker 中重复实现
|
||||
|
||||
**位置**：`MinimapHUD.cs` 第 97-108 行 vs `MapPlayerTracker.cs` 第 67-81 行
|
||||
|
||||
两处构建几乎相同的 `Dictionary<Vector2Int, string>` 索引（玩家追踪和小地图查询）。每个组件都独立构建一次相同数据，浪费内存 + 数据不一致风险（房间数据变化时一方更新另一方未更新）。
|
||||
|
||||
**修复方案**：将索引下沉为 `MapDatabaseSO` 的内置查询：
|
||||
|
||||
```csharp
|
||||
// MapDatabaseSO 中
|
||||
private Dictionary<Vector2Int, string> _cellToRoom;
|
||||
public string GetRoomIdAtCell(Vector2Int cell) { ... }
|
||||
```
|
||||
|
||||
或封装为 `MapServiceExtensions.GetRoomIdAtCell(this IMapService svc, Vector2Int cell)` 集中实现。
|
||||
|
||||
---
|
||||
|
||||
### 🟡 **R7-N4**：MapPinManager.CreatePin 缺少输入校验
|
||||
|
||||
**位置**：`MapPin.cs` 第 61-74 行
|
||||
|
||||
```csharp
|
||||
public MapPin CreatePin(string roomId, float normX, float normY,
|
||||
PinType type = PinType.Marker, string note = "")
|
||||
{
|
||||
var pin = new MapPin {
|
||||
RoomId = roomId, // 不验证非空、不验证 roomId 是否存在
|
||||
NormalizedPosX = normX, // 不 Clamp01
|
||||
NormalizedPosY = normY,
|
||||
...
|
||||
};
|
||||
AddPin(pin);
|
||||
return pin;
|
||||
}
|
||||
```
|
||||
|
||||
**问题**：
|
||||
- `roomId` 可空字符串/无效 ID，导致 `MapPanel.RenderPins` 中 `TryGetValue` 失败但 Pin 仍存在于存档
|
||||
- `normX/normY` 可为负数或大于 1，Pin 显示在房间格子外
|
||||
- 错误数据持久化到存档，后续清理困难
|
||||
|
||||
**修复方案**：参数校验 + `Mathf.Clamp01` + RoomId 存在性检查（可选警告）。
|
||||
|
||||
---
|
||||
|
||||
### 🟡 **R7-N5**：MapManager.OnRoomEntered 区域变化日志缺失
|
||||
|
||||
**位置**：`MapManager.cs` 第 70-82 行
|
||||
|
||||
```csharp
|
||||
private void OnRoomEntered(string roomId)
|
||||
{
|
||||
bool changed = _exploredRooms.Add(roomId);
|
||||
if (changed) _onMapUpdated?.Raise(roomId);
|
||||
|
||||
var regionId = _database?.GetRoom(roomId)?.RegionId;
|
||||
if (!string.IsNullOrEmpty(regionId) && regionId != _currentRegionId)
|
||||
{
|
||||
_currentRegionId = regionId;
|
||||
_onRegionChanged?.Raise(regionId);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**问题**：
|
||||
- 首次启动游戏（无存档加载）时，玩家首次进入某区域，`_currentRegionId` 由 null 变成 `regionId`，会触发一次 `EVT_RegionChanged`
|
||||
- 但 **加载存档后**，`OnLoad` 只恢复 `_exploredRooms`/`_mappedRooms`，**不恢复 `_currentRegionId`**
|
||||
- 玩家加载存档进入游戏时，第一次进入房间会再次触发 EVT_RegionChanged，导致「读档后第一次进房显示区域名渐显」UX 异常
|
||||
|
||||
**修复方案**：`OnLoad` 同步恢复 `_currentRegionId`（可基于玩家当前位置或最近进入的房间推导）。
|
||||
|
||||
---
|
||||
|
||||
### 🟡 **R7-N6**：MinimapHUD step③ 对新格子的二次 PlaceCell
|
||||
|
||||
**位置**：`MinimapHUD.cs` 第 175-194 行
|
||||
|
||||
```csharp
|
||||
foreach (var roomId in _roomsInViewBuffer)
|
||||
{
|
||||
...
|
||||
PlaceCell(cell, room); // ← step② 中刚调用
|
||||
_cells[roomId] = cell;
|
||||
}
|
||||
|
||||
// ③ 重定位所有格子
|
||||
foreach (var (id, cell) in _cells)
|
||||
{
|
||||
...
|
||||
PlaceCell(cell, r); // ← 新格子在此被再次 PlaceCell
|
||||
}
|
||||
```
|
||||
|
||||
**问题**：刚在 step② 中通过 PlaceCell 设置过位置的新格子，在 step③ 中又被 PlaceCell 一遍。虽然结果幂等，但浪费写入。
|
||||
|
||||
**修复方案**：step② 不调用 PlaceCell（信任 step③ 统一处理），或 step③ 跳过本帧新增的格子。后者更清晰：
|
||||
|
||||
```csharp
|
||||
var newlyAdded = new HashSet<string>();
|
||||
// step②
|
||||
foreach (var roomId in _roomsInViewBuffer) {
|
||||
if (_cells.ContainsKey(roomId)) continue;
|
||||
...
|
||||
PlaceCell(cell, room);
|
||||
newlyAdded.Add(roomId);
|
||||
_cells[roomId] = cell;
|
||||
}
|
||||
// step③ 重定位剩余存量格子
|
||||
foreach (var (id, cell) in _cells) {
|
||||
if (newlyAdded.Contains(id)) continue;
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 🟢 **R7-N7**：MapInputHandler 仍使用旧版 Input System（Round 6 N4 未修复）
|
||||
|
||||
**位置**：`MapInputHandler.cs` 第 42-43 行
|
||||
|
||||
```csharp
|
||||
float h = Input.GetAxisRaw("Horizontal");
|
||||
float v = Input.GetAxisRaw("Vertical");
|
||||
```
|
||||
|
||||
新 Input System 项目此代码失效，已在 Round 6 标注但未修复，本轮再次确认。
|
||||
|
||||
---
|
||||
|
||||
### 🟢 **R7-N8**：MapPlayerTracker._worldUnitsPerCell 字段无范围保护
|
||||
|
||||
**位置**：`MapPlayerTracker.cs` 第 25 行
|
||||
|
||||
```csharp
|
||||
[SerializeField] private float _worldUnitsPerCell = 18f;
|
||||
```
|
||||
|
||||
若策划误填 0 或负值，会导致：
|
||||
- `WorldToCell` 除以 0 产生 `Infinity`，进而 `Mathf.FloorToInt` 异常
|
||||
- 归一化位置计算 `worldSize.x` 为 0，被 `Mathf.Max(1f, ...)` 兜底但语义错误
|
||||
|
||||
**修复方案**：`[Min(0.01f)]` 或 `OnValidate` 保护。
|
||||
|
||||
---
|
||||
|
||||
### 🟢 **R7-N9**：MapPin.cs 文件名与类名不一致（历史遗留）
|
||||
|
||||
`MapPin.cs` 内部包含 `MapPinManager`（主类）+ `PinSpriteEntry`。VS Code/Rider 全局搜索 `MapPinManager` 文件名找不到，新人查找代码体验差。
|
||||
|
||||
**修复方案**：因 Unity .meta GUID 绑定限制，安全做法是新建 `MapPinManager.cs` 仅含一个 `// see MapPin.cs` 注释引导（不安全的做法是改 .meta 但风险高）；或保持现状并在 README 注明。
|
||||
|
||||
---
|
||||
|
||||
### 🟢 **R7-N10**：MapDatabaseSO._index OnDisable 清理的潜在 NRE
|
||||
|
||||
**位置**：`MapRoomDataSO.cs` 第 92 行
|
||||
|
||||
```csharp
|
||||
private void OnDisable() => _index = null;
|
||||
```
|
||||
|
||||
ScriptableObject 在域重载（Domain Reload）/编辑器停止播放时执行 OnDisable。若同时有运行中的协程或异步任务持有 SO 引用并即将调用 `GetRoom`，会触发索引重建（线性 LINQ），首帧停止时性能尖峰 + 短暂 NRE 风险。
|
||||
|
||||
**修复方案**：清理改为 `lock` 包裹的重建逻辑，或保留缓存让 GC 自然回收。当前实现风险低但非完全无害。
|
||||
|
||||
---
|
||||
|
||||
## 三、本轮独立评分
|
||||
|
||||
| 维度 | Round 6 | Round 7 独立 | 差值 | 说明 |
|
||||
|---|---|---|---|---|
|
||||
| 架构解耦 | 9.0 | **8.5** | -0.5 | R7-N3（索引重复实现）扣 0.5 |
|
||||
| 性能 | 8.0→8.5 | **8.5** | 维持 | P1-1 已修复，新发现的 N6 冗余写入影响微小 |
|
||||
| 编辑器扩展 | 8.5→9.0 | **9.0** | 维持 | P2 修复落实 |
|
||||
| 数据设计 | 8.0 | **7.5** | -0.5 | R7-N4（无输入校验）+ R7-N5（OnLoad 不恢复 region）扣分 |
|
||||
| 功能完整性 | 8.0 | **7.5** | -0.5 | R7-N1（Pin 不响应增删）是真实功能缺陷 |
|
||||
| 代码质量 | 8.5 | **8.5** | 0 | 整体仍达标 |
|
||||
| 可扩展性 | 8.0 | **7.5** | -0.5 | R7-N2（无热更事件）限制工具化扩展 |
|
||||
| 策划友好度 | 8.0 | **7.5** | -0.5 | R7-N2 影响策划迭代体验 |
|
||||
|
||||
### 加权总分
|
||||
|
||||
```
|
||||
架构解耦 ████████░░ 8.5/10
|
||||
性能 ████████░░ 8.5/10
|
||||
编辑器扩展 █████████░ 9.0/10
|
||||
数据设计 ███████░░░ 7.5/10
|
||||
功能完整性 ███████░░░ 7.5/10 ← R7-N1 Pin 响应缺陷
|
||||
代码质量 ████████░░ 8.5/10
|
||||
可扩展性 ███████░░░ 7.5/10 ← R7-N2 无热更
|
||||
策划友好度 ███████░░░ 7.5/10
|
||||
─────────────────────────────
|
||||
加权总分 82/100
|
||||
```
|
||||
|
||||
**Round 6 报告预估修复后 87 分，本轮独立审查实际 82 分**——差距源于 Round 6 评估视野未覆盖 N1/N2/N3 这三类"运行时-编辑器"协同问题。
|
||||
|
||||
---
|
||||
|
||||
## 四、严重程度排序与修复优先级
|
||||
|
||||
### P0（立即修复，影响真实 UX/功能）
|
||||
|
||||
| ID | 问题 | 工时估计 |
|
||||
|---|---|---|
|
||||
| R7-N1 | MapPanel 不响应 Pin 增删 | 小（LateUpdate 加 RenderPins 调用） |
|
||||
| R7-N5 | OnLoad 不恢复 _currentRegionId | 小（OnLoad 推导一行） |
|
||||
|
||||
### P1（高：影响工具化与扩展）
|
||||
|
||||
| ID | 问题 | 工时估计 |
|
||||
|---|---|---|
|
||||
| R7-N2 | MapPanel/MinimapHUD 不响应数据库热更 | 中（IMapService 新增事件 + 订阅） |
|
||||
| R7-N3 | BuildSpatialIndex 重复实现 | 中（下沉到 MapDatabaseSO 或扩展方法） |
|
||||
| R7-N4 | CreatePin 缺输入校验 | 小（参数 Clamp + 警告） |
|
||||
|
||||
### P2（中：代码质量与健壮性）
|
||||
|
||||
| ID | 问题 | 工时估计 |
|
||||
|---|---|---|
|
||||
| R7-N6 | step③ 对新格子二次 PlaceCell | 小（newlyAdded 集合跳过） |
|
||||
| R7-N7 | MapInputHandler 旧版 Input | 中（替换为 IInputService） |
|
||||
| R7-N8 | _worldUnitsPerCell 无范围保护 | 极小（`[Min]`） |
|
||||
|
||||
### P3（低：历史遗留 / 边缘场景）
|
||||
|
||||
| ID | 问题 |
|
||||
|---|---|
|
||||
| R7-N9 | MapPin.cs 文件名与类名不一致 |
|
||||
| R7-N10 | _index OnDisable 清理潜在 NRE |
|
||||
|
||||
---
|
||||
|
||||
## 五、对标空洞骑士 / 丝之歌的差距分析
|
||||
|
||||
| 能力 | 对标标准 | 当前状态 | 差距 |
|
||||
|---|---|---|---|
|
||||
| **三级可见性（Unknown/Explored/Mapped）** | ✓ | ✓ 完整 | 无 |
|
||||
| **MapFragment 购买揭示** | ✓ | ✓ 接口已就位 | 无 |
|
||||
| **自定义 Pin 标记** | ✓ 多种类型 | ✓ 但**地图打开时不响应增删** | **N1** |
|
||||
| **区域名渐显** | ✓ | ✓ 但读档后会异常触发 | **N5** |
|
||||
| **存档点显示完整地图** | ✓ | ✓ via SetMapped API | 无 |
|
||||
| **房间非矩形轮廓** | ✓ | ✓ RoomOutlineTex 字段 | 无 |
|
||||
| **小地图视野跟随玩家** | ✓ | ✓ 含空间索引优化 | 无 |
|
||||
| **图标平滑跟随（非格子跳跃）** | ✓ | ✓ Round 4 已修复 | 无 |
|
||||
| **运行时缩放/平移** | ✓ 滚轮+键盘+手势 | ⚠️ 仅滚轮+键盘 | 无手势/手柄 |
|
||||
| **探索进度统计 UI** | ✓ | ❌ API 完整但无 UI 显示 | 中等 |
|
||||
| **区域配色主题** | ✓ 各区域有专属配色 | ❌ 编辑器随机配色，运行时无 | 大（需 RegionSO） |
|
||||
| **数据库热更** | — | ❌ 不响应 | **N2** |
|
||||
|
||||
---
|
||||
|
||||
## 六、总评
|
||||
|
||||
### 当前最终得分：**82/100**
|
||||
|
||||
**优势**：
|
||||
- 完整的接口/ServiceLocator 解耦体系
|
||||
- 编辑器三工具（SO Inspector + Custom Editor + 全局布局窗口）功能扎实
|
||||
- 热路径性能已优化（脏检查 + 缓存 + 空间索引）
|
||||
- 数据驱动设计支持策划独立迭代
|
||||
|
||||
**真实差距（按修复成本排序）**：
|
||||
1. **N1 Pin 不响应增删** — 1 行修复，是当前最高 ROI 的修复点
|
||||
2. **N5 OnLoad 不恢复 region** — 1 行修复，避免读档体验异常
|
||||
3. **N4 CreatePin 无校验** — 3 行修复，避免脏数据进入存档
|
||||
4. **N6 step③ 二次 PlaceCell** — 5 行修复，性能小优化
|
||||
5. **N3 索引重复实现** — 重构，DRY 改善
|
||||
6. **N2 数据库热更事件** — 中等改动，但显著提升策划工作流
|
||||
|
||||
**仍未弥补的功能缺口**（Round 5/6 已标注，仍在）：
|
||||
- 探索进度 UI（API 有，UI 无）
|
||||
- RegionSO（区域配色/名称集中配置）
|
||||
- MapDesignSpec.md 策划文档
|
||||
- Pinch 缩放、手柄输入
|
||||
|
||||
如果完成 P0+P1（N1/N5/N2/N3/N4），评分预期回到 **88-90 分**；再补 RegionSO + 进度 UI 可达 **93+**。
|
||||
|
||||
---
|
||||
|
||||
*Round 7 旨在矫正 Round 6 在「运行时-编辑器协同」「事件响应完整性」两个视角的盲区。本轮重点发现的 N1/N2/N5 是 Round 1-6 全部漏检的真实功能/UX 缺陷，建议立即修复。*
|
||||
Reference in New Issue
Block a user