Fix bug where DynamicMultiblockManager attempts to load unloaded blocks 修复DynamicMultiblockManager会尝试加载未加载的区块的问题

[QiuShui1012]

No description provided.

[Gugle2308]

代码审查摘要

PR #73 · Anvil-Dev/AnvilLib · fix/26.1 → dev/26.1 · 3 files · 73 lines

结论：请求更改（1 个关键问题）

---

🔴 关键

DynamicMultiblockManager.java:403 — 首次调用 buildSnapshot() 时 old 可能为 null 导致 NPE

问题位置： DynamicMultiblockManager.buildSnapshot() 第 403 行

MultiblockCheckSnapshot old = state.getSnapshot();  // 首次调用时 = null
// ...
if (!level.isLoaded(pos)) {
    entries.put(pos, old.entries().get(pos));  // ❌ old == null → NPE
    continue;
}

根因分析：

• MultiblockState.snapshot 字段默认值为 null（无显式初始化）
• checkMultiblockFormed() 的调用顺序是：先 buildSnapshot() → 再 setSnapshot()
• 因此第一次对某个 MultiblockState 调用 buildSnapshot() 时，state.getSnapshot() 返回 null
• 如果此时恰好有多方块位置位于未加载的区块中，old.entries().get(pos) 会抛出 NPE，导致服务端崩溃

触发条件（race condition）：

• 当服务器仍在启动/加载区块的早期阶段
• 当多方块跨越已加载和未加载区域的边界
• 多方块刚形成后立即触发检查

建议修复：

MultiblockCheckSnapshot old = state.getSnapshot();
// ...
if (!level.isLoaded(pos)) {
    // 如果 old 为 null（首次检查），跳过该位置（本次不校验）
    if (old == null) {
        continue;
    }
    entries.put(pos, old.entries().get(pos));
    continue;
}

或者更安全的做法是如果 old == null 则直接跳过本次检查（return 一个标记为 pending 的 snapshot），因为首次检查时所有位置理论上都应在已加载区块中——未加载意味着时机不对。

---

⚠️ 警告

1. MultiblockCheckSnapshot.Entry.blockState 从非空变为 @Nullable — 需交叉验证所有调用点

位置： MultiblockCheckSnapshot.java:47

Entry 是 public record，这意味着 entry.blockState() 的返回类型从非空变为可空。这是源码级 API 变更。需要确认整个项目中所有直接访问 entry.blockState() 的地方都能处理 null 值。

建议用以下命令检查：

grep -rn "\.blockState()" module.multiblock/src/main/java/

如果其他代码直接解引用返回值（如 entry.blockState().getBlock()），会引发 NPE。

2. 缺少 @Nullable 导入验证

新增的 @Nullable 注解（第 47 行）需要对应的 import 语句。请确认文件中已导入正确的包（org.jspecify.annotations.Nullable 或 org.jetbrains.annotations.Nullable）。

---

💡 建议

1. 考虑在 MultiblockState 构造器中初始化 snapshot

为避免 null 状态的隐式依赖，建议在构造器中初始化 snapshot 为一个空的 snapshot：

public MultiblockState(BlockPos controllerPos, ResourceKey<MultiblockDefinition> definitionKey) {
    this(controllerPos, definitionKey, false);
    this.snapshot = new MultiblockCheckSnapshot(Map.of());  // 初始化空快照
}

这样 buildSnapshot() 中的 old 永不为 null，简化了 null 处理逻辑。

2. 添加 @Nullable 到 Entry 的 blockState 参数 javadoc

记录的参数文档应说明：当所在区块未加载时 blockState 为 null，此时 test() 返回 true（假定结构完整）。这有助于未来的维护者理解设计意图。

---

✅ 看起来不错

• 整体思路正确：用旧 snapshot 中的条目填充未加载区块的位置，避免强制加载区块，符合预期行为
• Entry.test() 提取：将条件判断封装到 record 内部方法中，比在循环中靠外层逻辑判断更清晰、更可维护
• 线程安全 OK：setSnapshot() 在添加到 pendingChecks 之前发生，通过 LinkedBlockingQueue 建立了 happens-before 关系
• Entry.test()短路逻辑合理：blockState == null → return true（未加载则假定结构完整），避免校验过程加载区块

---

总结

等级	数量	关键项
🔴 关键	1	首次调用时 old 为 null 导致 NPE
⚠️ 警告	2	record 字段变为 @Nullable 影响外部调用点；缺少 import 验证
💡 建议	2	构造器预初始化 snapshot；记录可空性 javadoc
✅ 不错	3	核心思路、封装提取、线程安全

建议修复 NPE 后再合并。

[Gugle2308]

审查完成。以下是分析结果：

---

代码审查摘要

PR #73 — 修复 DynamicMultiblockManager 尝试加载未加载区块的问题。
作者： QiuShui1012
分支： fix/26.1 → dev/26.1
结论： ✅ 批准（无阻塞性问题）

---

变更概述

文件	状态	行数
DynamicMultiblockManager.java	修改	+11 / -1
MultiblockCheckSnapshot.java	修改	+11 / -5
MultiblockState.java	修改	+4 / -0

核心思路： 在构建多方块快照时，若目标位置所在区块未加载，改为复用上一轮的快照条目（或创建空条目跳过），而非调用 level.getBlockState(pos)——从而避免强迫加载已卸载的区块。

---

✅ 正确性

1. 未加载区块守卫路径完整：!level.isLoaded(pos) → 尝试取 old.entries().get(pos) → 若仍为 null 则创建 (null, null, predicate) fallback 条目 → continue 跳过后续逻辑。三种情况都覆盖了。

2. 初始状态安全：MultiblockState 两参构造器将 snapshot 初始化为 new MultiblockCheckSnapshot(this.controllerPos, Map.of())。首次 buildSnapshot() 时 old.entries().get(pos) 返回 null，fallback 路径正确生效。

3. Entry.test() 正确处理 null blockState：新增的 Entry.test() 方法在 blockState == null 时返回 true（无法检查的位置视为通过检查），否则委托给 predicate.testOffThread()。这符合"对未加载区域不做判定"的设计意图。

4. Snapshot.test() 委托至 Entry.test()：原调用 entry.predicate().testOffThread(entry.blockState(), entry.entityNbt()) 改为 entry.test()，让 Entry 自行决定如何测试（包括 null 情况）。良好的职责下沉。

5. controllerPosLong → controllerPos（BlockPos）：伴随此变更，MultiblockCheckSnapshot 内部现在直接存储 BlockPos 而非 long 编码。调用方 buildSnapshot() 也同步更新为 state.getControllerPos()。一致性良好。

---

⚠️ 提示（非阻塞）

线程安全——现有约定延续：buildSnapshot() 在工作线程（ExecutorService）上调用，其中 state.getSnapshot() 读取和 mstate.setSnapshot(snapshot) 写入共享的 MultiblockState.snapshot 字段。该字段通过 Lombok @Setter 暴露，即裸字段赋值，无 volatile/synchronized 保障。

但这不是本 PR 新引入的问题——整个类的多区块检查已使用 ExecutorService 处理所有状态，线程模型一致。本 PR 追加上下文是合乎情理的。如果后续出现竞态可见性问题（极少发生，因为 snapshot 是幂等的——旧数据优于没有数据），可考虑将 snapshot 字段标记为 volatile。

---

✅ 看起来不错

• 作用域纯净：仅修改核心逻辑，无多余格式化/翻译/无关变更
• 边界完整：未加载区块、首次构建、后续构建三种路径均覆盖
• 接口向后兼容：MultiblockCheckSnapshot 的 record 字段名变更（controllerPosLong → controllerPos）是该类的内部 API，不影响使用者
• Entry 的 blockState 标记为 @Nullable：正确标注了语义变化

---

总结

此 PR 精准地修复了目标问题：通过快照复用机制避免在多区块形成检测中强行加载已经卸载的区块。改动范围有限，逻辑清晰，边界情况处理完整。批准。

[Gugle2308]

🌿 Roseau API Breaking Change Report

Module	Status	Breaking Changes
codec	✅ Compatible	0
collision	✅ Compatible	0
config	✅ Compatible	0
integration	✅ Compatible	0
moveable-entity-block	✅ Compatible	0
network	✅ Compatible	0
rendering	✅ Compatible	0
space-select	✅ Compatible	0
font	✅ Compatible	0
util	✅ Compatible	0
explosion	✅ Compatible	0
rpc	✅ Compatible	0
multiblock	✅ Compatible	0
recipe	✅ Compatible	0
registrum	✅ Compatible	0
sync	✅ Compatible	0
wheel	✅ Compatible	0
main	✅ Compatible	0

Full CSVs: see the Artifacts section of this workflow run.

[Gugle2308]

🌿 Roseau API Breaking Change Report

Module	Status	Breaking Changes
codec	✅ Compatible	0
collision	✅ Compatible	0
config	✅ Compatible	0
integration	✅ Compatible	0
moveable-entity-block	✅ Compatible	0
network	✅ Compatible	0
rendering	✅ Compatible	0
space-select	✅ Compatible	0
font	✅ Compatible	0
util	✅ Compatible	0
explosion	✅ Compatible	0
rpc	✅ Compatible	0
multiblock	🔴 BC detected	1
recipe	✅ Compatible	0
registrum	✅ Compatible	0
sync	✅ Compatible	0
wheel	✅ Compatible	0
main	✅ Compatible	0

🔴 multiblock — 1 breaking change(s)

dev/anvilcraft/lib/v2/multiblock/dynamic/MultiblockCheckSnapshot.java:20

dev.anvilcraft.lib.v2.multiblock.dynamic.MultiblockCheckSnapshot
dev.anvilcraft.lib.v2.multiblock.dynamic.MultiblockCheckSnapshot.<init>(long,java.util.Map<net.minecraft.core.BlockPos,dev.anvilcraft.lib.v2.multiblock.dynamic.MultiblockCheckSnapshot$Entry>)
EXECUTABLE_REMOVED
✗ binary-breaking
✗ source-breaking

Full CSVs: see the Artifacts section of this workflow run.

[Gugle2308]

停止审查

操作状态为 closed。根据代码审查技能规定：

"closed" / "labeled" → 停止，不发评论

此 PR 已关闭，无需发布审查。