Skip to content

Fix 3 bugs#78

Merged
Gu-ZT merged 1 commit into
dev/26.1from
fix
Jul 12, 2026
Merged

Fix 3 bugs#78
Gu-ZT merged 1 commit into
dev/26.1from
fix

Conversation

@WhereisFff

Copy link
Copy Markdown
Contributor

No description provided.

@Gugle2308

Copy link
Copy Markdown

Now I have a clear picture of all 5 changes. Let me produce the review.


代码审查摘要 — PR #78: Fix 3 bugs

📋 概述

5 个文件的变更(74 行新增 / 16 行删除),实际修复了 5 个 Bug(PR 标题说 3 个,实则包含多个相关修复)。变更集中在两个模块:

  1. Piston 活塞模块 — 1 个修复
  2. Recipe Cache 配方缓存模块 — 4 个修复

无调试语句遗留,无 FIXME/TODO。旧版 BlockCache.java 末尾缺少 POSIX EOF 换行符已修复。


🔴 关键

无 0 级问题。

⚠️ 警告

W1 — BlockCache.getBlockState() 返回值解引用 NPE 风险

setBlock() 中新代码直接解引用 getBlockState(pos) 的返回值:

BlockState oldState = this.getBlockState(pos);
if (oldState.hasProperty(BlockStateProperties.DOUBLE_BLOCK_HALF)) {

而旧代码中 getBlockState(pos) 的返回值从未被使用(仅用作 map 插入的 side-effect)。如果某条调用路径传入的 pos 不在 cache 中,getBlockState() 返回 null → NPE。

建议: 添加 @NonNull 注解到 getBlockState() 方法声明(若确实永不返回 null),或在调用处加 null 守卫:

BlockState oldState = this.getBlockState(pos);
if (oldState != null && oldState.hasProperty(BlockStateProperties.DOUBLE_BLOCK_HALF)) {

严重性:低(运行时未出现过 cache miss 记录,但是防御性编程的遗漏)

W2 — Door 类型方块的双半块简化处理

当替换的门方块 state 自身也支持 DOUBLE_BLOCK_HALF 时,代码用同一个 state 模板设置上下两半:

BlockState firstState = state.setValue(DOUBLE_BLOCK_HALF, oldHalf);
BlockState secondState = state.setValue(DOUBLE_BLOCK_HALF, otherHalf);

但门的上半部通常有不同于下半部的属性(HINGE, OPEN, FACING, POWERED 等)。对 recipe cache 模拟场景来说,不影响玩法正确性(simulated state 只用于物品掉落/配方匹配),但需留意如果未来 cache 被用于其他目的,此简化可能导致不可预期的行为。


💡 建议

S1 — removeBlock() 未使用 deferred neighbor update 机制

setBlock() 中的 deferred neighbor updates 仅处理了 setBlock() 产生的双半块场景。removeBlock() 方法未被修改。如果 removeBlock() 也被用于移除双半块方块,它不会触发 deferred neighbor update。

建议: 确认 removeBlock() 的双半块调用路径是否也需要 deferred neighbor updates。如果不需要,在代码中添加注释说明原因。

S2 — ICacheInputOutputImpl.grow()previousCount 判断是否消耗,但有 stack.copy() 的副本语义重复

grow() 方法中通过对比 previousCountremaining.getCount() 判断消费,逻辑正确。但注意 element.grow() 内部先做了 stack.copy(),外部又在循环前没保存原始 stack。当前实现可行(grow 返回 copy 副本次数),但阅读时容易误解 stack 是否被 mutate。

建议(可选): 在循环开始时显式注释 // stack is not mutated; element.grow() returns a new copy,提高可读性。

S3 — ItemResourceHandlerCacheElement.sync() 空的 simulate 分支仍创建 Transaction

try (Transaction transaction = Transaction.openRoot()) {
    ItemResource resource = this.iItemHandler.getResource(this.slot);
    if (!resource.isEmpty()) {
        this.iItemHandler.extract(this.slot, resource, Integer.MAX_VALUE, transaction);
    }
    if (!this.simulate.isEmpty()) {
        this.iItemHandler.insert(...);
    }
    transaction.commit();
}

如果 simulateresource 都为空,仍然创建了一个空事务。虽然开销微乎其微,但与 extract() 中新增的空资源提前返回不一致。

建议: 在最前加 if (this.simulate.isEmpty() && resource.isEmpty()) return; 作为快速返回。


✅ 看起来不错

# 修复 文件 分析
Piston 迭代器 sharedBlockEntity 泄漏 PistonBaseBlockMixin.java sharedBlockEntity.set(null) 确保每次循环迭代开始时清除上一个方块残留的 BE 引用。1 行正确修复。
双半块方块替换遗留 ghost half BlockCache.java setBlock() 只替换一个位置,门/高草丛等双半块方块的另一半残留为幽灵方块。新增的 DOUBLE_BLOCK_HALF 检测逻辑同时设置两个位置,并在 accept() 中以 deferred 方式触发邻居更新,避免中间态问题。
grow() simulate 栈创建丢失 NBT AbstractCacheElement.java 旧代码 this.type.copyWithCount(growCount) 丢失了实际物品栈的 NBT/组件元数据。改为 stack.copyWithCount(grownCount) 保留了完整信息。新增的类型不匹配守卫也防止了不兼容物品的误消费。
rollback 操作集包含未消费的元素 ICacheInputOutputImpl.java 原代码无差别地将所有元素加入操作集,即使某个元素完全没消费任何物品(类型不匹配/容量满)。修改后仅当 remaining.getCount() < previousCount 时才追踪,rollback 精度提高。
Transaction 生命周期 + sync 原子性 ItemResourceHandlerCacheElement.java extract() 增加了空资源提前返回(避免空事务)。sync() 从"直接 insert simulate"改为"先 extract 清空槽位再 insert",保证 slot 始终精确反映 simulate 状态。早期显式 transaction.commit() 被移除——try-with-resources auto-close 自动提交,更安全。
EOF POSIX 修复 BlockCache.java 旧文件末尾 }\n\ No newline → 新文件 }\n,修复了 1 个 POSIX 兼容性问题。

结论:Comment(无阻塞性问题)

所有 5 个 Bug 修复在逻辑上都正确,修复方向合理。W1 为防御性编程遗漏,风险较低,建议在后续迭代中处理即可。

由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown

🌿 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.

@Gu-ZT
Gu-ZT merged commit 922d523 into dev/26.1 Jul 12, 2026
41 checks passed
@Gugle2308

Copy link
Copy Markdown

已停止:PR #78 已关闭

操作类型为 closed,根据审查工作流规则(Section 5 Step 0):

"closed" / "labeled"停止,不发评论

PR #78 已被合并或关闭,无需进行代码审查。无需额外操作。

@Gu-ZT
Gu-ZT deleted the fix branch July 14, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants