Files
zeling_v2/Docs/Review/Minimap_Review_Round22_Independent.md
T
joywayer f74d7f1877 Add independent review reports for Minimap system (Rounds 8, 9, and 26)
- Round 8 report highlights improvements in architecture, editor usability, and data robustness, with a total score of 80/100.
- Round 9 report focuses on editor extension capabilities, identifying issues with room data indexing and layout editing, resulting in a score of 76/100.
- Round 26 report evaluates the system against commercial standards, noting new issues and confirming previous fixes, with a score of 95.8/100.
2026-05-25 23:15:12 +08:00

235 lines
9.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 小地图系统独立评审 Round 22
**评审时间**:R22(基于 R21 全部修复已落地的代码基线)
**评审范围**`Assets/_Game/Scripts/World/Map/` 全部 19 个运行时文件 + 4 个编辑器扩展文件
**评分基准**:专业 2D Metroidvania 编辑器扩展标准(架构解耦 / 高性能 / 可扩展 / 开发者友好)
---
## R21 修复确认
| 编号 | 内容 | 状态 |
|------|------|------|
| R21-N1 | `MinimapHUD._servicesReady` + `LateUpdate` 重试(对齐 MapPanel | ✅ 确认(L66, L197-203 |
| R21-N1 | `MapPlayerTracker [DefaultExecutionOrder(-600)]` | ✅ 确认(L17 |
| R21-N2 | `MapPanel` 集合字段补全 `readonly` | ✅ 确认(L58-63 |
| R21-N3 | `MinimapHUD.UnsubscribeServices` 重置 `_servicesReady` | ✅ 确认(L158 |
---
## 各维度评分
### 1. 架构设计(Architecture 19.0 / 20
**亮点**
- ServiceLocator + 4 接口(IMapService / IPinService / IPlayerPositionProvider / ITeleportService)零耦合
- 事件驱动(C# Action)无轮询,OnDatabaseChanged / OnExplorationChanged / OnRoomMapped 语义分明
- ISaveable 三处防御性拷贝对称(MapManager / MapPinManager / TeleportService
- MapRoomCellUI 双视图复用,无重复 Prefab
- `ChooseDisplayIcon` 唯一入口(MapRoomDataSO),MapPanel / MinimapHUD 无漂移
- `MapServiceExtensions` 无状态扩展方法,消费方零重复查询逻辑
**新发现问题 N1(中等)**`RegionNameEntry` 字典解析逻辑在两个组件重复
`RegionNameDisplay``MapProgressDisplay` 各自独立实现了完全相同的模式:
```csharp
// RegionNameDisplay
private Dictionary<string, RegionNameEntry> _regionDict;
private void BuildRegionDict() { ... }
private string ResolveDisplayName(string regionId) { ... }
// MapProgressDisplay
private Dictionary<string, RegionNameEntry> _regionDict;
private void BuildRegionDict() { ... }
private string ResolveRegionDisplayName(string regionId) { ... }
```
两者代码几乎逐行相同(包含 LocKey 优先、DisplayName 次之、RegionId 回退三段逻辑),仅方法名不同。
修复建议:在 `MapServiceExtensions.cs` 中补充静态扩展方法 `BuildRegionDict / ResolveDisplayName`
两个组件调用同一实现,消除 DRY 违反。
**扣分:−1.0**
---
### 2. 性能(Performance 19.5 / 20
**亮点**
- MapPanel 全部 `readonly` 集合池 × 6Cell / Pin / Exit);MinimapHUD × 4
- `_servicesReady` 短路(MapPanel R12-N7MinimapHUD R21-N1),消除每帧 ServiceLocator 查询
- O(viewRadius²) 空间索引,大地图下 MinimapHUD.RefreshView 比 O(AllRooms) 显著降低开销
- 复用缓冲区:`_toRemove` / `_roomsInViewBuffer` / `_newlyAddedBuffer`(预分配容量)
- PinsVersion 脏检查 + 玩家位置脏检查消除无效写入
- `_regionCache` 懒加载(MapManager),`_regionDict` 字典化(RegionNameDisplay / MapProgressDisplay
**说明(信息级)**`MapProgressDisplay.Refresh()` 区域进度遍历为 O(rooms_in_region)
但仅在 `OnExplorationChanged` 或区域切换时触发(非每帧),当前规模无性能风险。
**扣分:−0.5**`MapProgressDisplay.Refresh` 区域遍历未缓存已探索计数,极大地图时有轻微冗余)
---
### 3. 代码质量(Code Quality 19.0 / 20
**亮点**
- 全部集合字段已补齐 `readonly`MapPanel R21-N2 修复)
- `CurrentZoom` 属性(R19-N1)消除双份状态
- `RunRevealAnim` 自清理协程(R20-N1
- `HasCustomExitPos` 语义布尔替代哨兵
- `[Obsolete]` + `[HideInInspector]` 废弃字段注解完整
- `DrawLine` try/finally 保证 GUI.matrix 恢复(R11-N12
- `MapPin.cs` 文件头注释标明"文件名历史遗留,请搜索类名 MapPinManager"
**新发现问题 N2(轻微)**:编辑器 `DrawRoomBadge` 注释引用已过时
```csharp
// MapLayoutEditorWindow.cs L488
// 优先级与运行时 MapPanel.ChooseIcon 对齐:Save > Boss > Shop > Teleport
```
R20-N2 已将运行时图标选取逻辑迁移至 `MapRoomDataSO.ChooseDisplayIcon`
`MapPanel.ChooseIcon` 已变为单行委托。注释应更新为:
```csharp
// 优先级与运行时 MapRoomDataSO.ChooseDisplayIcon 对齐:Save > Boss > Shop > Teleport
```
**扣分:−1.0**(N1 DRY 违反产生的代码质量问题)
---
### 4. 编辑器扩展(Editor Tools 14.5 / 15
**亮点**
- `MapLayoutEditorWindow`:缩放/平移/拖拽/搜索/区域着色/验证/Undo/热改完整
- `_cachedZoomForStyle` 脏检查 + `_noResultStyle` 首次初始化缓存
- `DrawExitLines` 字段级去重 `_drawnExitPairs`OnGUI 零分配
- `SetDatabase` 公共 API,避免 MapDatabaseEditor 反射访问私有字段
- `MapRoomAutoRegister` AssetPostprocessor 自动注册工作流完整
**扣分:−0.5**DrawRoomBadge 注释引用过时,对策划人员阅读代码时产生误导)
---
### 5. 功能完整性(Feature Completeness 15.0 / 15
**亮点**(所有功能均已实现且正确)
- 三级可见性(Unknown / Explored / Mapped+ 雾效覆盖层
- 图标优先级唯一入口(Override > SavePoint > Boss > Shop > Teleport
- Pin 系统(持久化、类型化、视野内渲染)
- 出口连接线 + Fallback 位置(R13-N1 HasCustomExitPos
- 传送系统(TeleportService:解锁 / 验证 / 请求 / 完成回调)
- 区域检测 + 区域名本地化显示(RegionNameDisplay + MapProgressDisplay
- 存档/读档(ISaveable 三处,防御性拷贝对称)
- 房间发现动画(RevealAnim 自清理,R20-N1
- 全屏地图 + 角落 HUD 双视图;小地图视野档位切换(CycleZoom)
**扣分:0**
---
### 6. 输入系统(Input System 9.5 / 10
**亮点**
- 全部使用 InputSystemInputReaderSO
- `MapInputHandler`Navigate / MapCenter / OnScroll 完整
- `MinimapInputHandler`CycleMinimapZoom 路由
- OnEnable/OnDisable 对称订阅/取消
**信息级**`MapInputHandler._zoom``OnScroll` 中作为本地累加器,与 `_panel.CurrentZoom` 读取路径不同但结果一致(OnScroll 写 → CurrentZoom 读,无环),正确配置下无实际风险。
**扣分:−0.5**(轻微职责重叠,不影响正确性)
---
## 综合评分
| 维度 | 满分 | 得分 | 较 R21 |
|------|------|------|--------|
| 架构设计 | 20 | 19.0 | +0.5 |
| 性能 | 20 | 19.5 | ±0 |
| 代码质量 | 20 | 19.0 | +0.5 |
| 编辑器扩展 | 15 | 14.5 | ±0 |
| 功能完整性 | 15 | 15.0 | +0.5 |
| 输入系统 | 10 | 9.5 | ±0 |
| **合计** | **100** | **96.5** | **+1.5** |
---
## 问题清单与修复建议
### N1 — RegionNameEntry 字典解析逻辑重复(中优先级)
**根因**`RegionNameDisplay``MapProgressDisplay` 独立实现了相同的 `BuildRegionDict` + `Resolve` 模式。
**修复方案**:在 `MapServiceExtensions.cs` 追加静态扩展/工具方法:
```csharp
/// <summary>
/// 将 RegionNameEntry 数组构建为 O(1) 查询字典。
/// RegionNameDisplay / MapProgressDisplay 共享此实现,消除重复。
/// </summary>
public static Dictionary<string, RegionNameEntry> BuildRegionDict(RegionNameEntry[] entries)
{
var dict = new Dictionary<string, RegionNameEntry>();
if (entries == null) return dict;
foreach (var e in entries)
if (!string.IsNullOrEmpty(e.RegionId))
dict[e.RegionId] = e;
return dict;
}
/// <summary>从字典解析 regionId 的玩家可读显示名;字典为 null 时直接回退到 regionId。</summary>
public static string ResolveRegionDisplayName(
Dictionary<string, RegionNameEntry> dict, string regionId)
{
if (dict != null && dict.TryGetValue(regionId, out var e))
return e.GetDisplayName();
return regionId;
}
```
两个组件改为:
```csharp
private void BuildRegionDict()
=> _regionDict = MapServiceExtensions.BuildRegionDict(_regionNames);
private string ResolveDisplayName(string regionId)
=> MapServiceExtensions.ResolveRegionDisplayName(_regionDict, regionId);
```
---
### N2 — DrawRoomBadge 注释引用过时(低优先级)
**修复**`MapLayoutEditorWindow.cs` L488 注释更新:
```csharp
// 优先级与运行时 MapRoomDataSO.ChooseDisplayIcon 对齐:Save > Boss > Shop > Teleport
```
---
### N3 — MapPinManager 缺少 [DefaultExecutionOrder](信息级)
`MapPinManager.Awake` 注册 `IPinService`,若晚于 UI 的 `SubscribeServices` 调用,
`_pinService` 将在首帧为 null(由 `_servicesReady` 重试机制兜底)。
两 UI 的 `_servicesReady` 短路已覆盖此场景,但显式标注执行顺序更具防御性:
```csharp
[DefaultExecutionOrder(-500)] // 晚于 MapPlayerTracker(-600),早于默认 0
public class MapPinManager : MonoBehaviour, ISaveable, IPinService
```
---
## 评分历史
| 轮次 | 评分 | 关键改动 |
|------|------|----------|
| R17 | 93.0 | — |
| R18 | 93.8 | MinimapHUD 废弃 _onMapUpdated 订阅 |
| R19 | 93.8 | CurrentZoom 属性;_revealCoroutines 防泄漏 |
| R20 | 94.2 | RunRevealAnim 自清理;ChooseDisplayIcon 集中 |
| R21 | 95.0 | MinimapHUD _servicesReadyMapPlayerTracker 执行顺序;readonly 补全 |
| **R22** | **96.5** | R21 修复确认;识别 N1RegionNameEntry DRYN2(注释过时)N3MapPinManager 执行顺序) |