Fixed bugs and optimized experience#4189
Conversation
代码审查摘要 — PR #4189操作: 📋 声称验证表
🔴 关键问题1. 新代码在
建议添加频率限制或先检查实体是否确实在支持加热器的大锅方块内,减少不必要的 getBlockState 调用。 影响评估: 旧代码只伤害大型大锅内的实体(大锅 BE 管理范围);新代码伤害任何大锅上方加热器上的实体(普通大锅 + 大型大锅)。这可能是期望的修复(旧代码只覆盖了大型大锅,缺少了普通大锅),但 tick 频率比旧代码高得多。
|
代码审查摘要 — PR #4189操作: review_requested 🟢 1. 手持巨型铁砧时大锅顶部视为完整碰撞 — ✅ 正确LargeCauldronBlock.java
✅ 实现正确,符合预期。 🟢 2. 修复各种锅和缸的伤害问题 — ✅ 正确改动要点:
关键验证: 🟢 3. 多方块下方有切石机任意 part 被砸正确触发破坏 — ✅ 正确AnvilEventListener.java 的
额外改进: 将
|
| 旧逻辑 | 新逻辑 |
|---|---|
| 优先从燃料槽提取物品 | 仅从 burnTime 缓存直接扣除 |
物品不够时从 burnTime 补足 |
无物品交互 |
不调用 updateBurningState() |
调用 updateBurningState() 立即同步方块状态 |
这是正确的方向 — 旧逻辑试图"预消耗"燃料槽物品,但与 tick 中的 tryConsumeFuel() 产生竞争(消耗→下个 tick 补满→消耗不可见),导致扣除时间不准确。新逻辑统一使用 burnTime 缓存做会计,tryConsumeFuel() 在每 tick 自动从物品补充。
a) updateBurningState() 在配方处理中调用 level.setBlock() — consumeBurnTime() 现在在配方 outcome 处理(ConsumeFuel.accept())中被调用,期间调用 level.setBlock() 改变方块状态可能在巨型铁砧配方处理链中触发意外的邻居更新。updateBurningState 仅在 LEVEL 属性变化时才会实际设置方块,但即使如此也要留意。
b) FUEL_CONSUMED 守卫移除后可能导致的多次扣费(需原作者确认意图)
triggerRecipeGroup() 方法中移除了以下守卫代码:
// 旧:因 executed 标记,同组后续配方不扣费
if (fuelConsumed || executed) {
context.put(FUEL_CONSUMED, true);
}由于 InWorldRecipeContext 每轮循环重建,新代码中同一个 slot 的多个配方各自独立扣费。如果 slot 中有 3 个可处理物品,将扣 3 × 4800 = 14400 tick 的燃烧时间。旧代码中同类 3 个物品只扣 1 × 4800 tick。
| 旧行为 | 新行为 | |
|---|---|---|
| 1 个物品 | 扣 4800 tick ✅ | 扣 4800 tick ✅ |
| 3 个物品 | 扣 4800 tick (只扣 1 次) | 扣 14400 tick (各扣 1 次) |
如果这是 PR 声称的"修复扣除时间不对"的意图(旧代码扣少了),那新行为正确。请确认此处是否符合预期——如果是,建议在注释中记录这个行为变更。
📋 声称验证表
| 声称 | 状态 | 对应文件 |
|---|---|---|
| 手持巨型铁砧时大锅顶部视为完整碰撞 | ✅ | LargeCauldronBlock.java:getShape() |
| 修复各种锅和缸的伤害问题 | ✅ | IDamagingHeater, HeaterUtil, BurningHeaterBlock.stepOn(), HeaterBlock.stepOn(), CauldronUtil.hurtFromHeaterBelow(), LivingEntityEventListener.onTick() |
| 多方块下有切石机任意 part 被砸正确触发破坏 | ✅ | AnvilEventListener.java:hasStonecutterBelowAnyPart() |
| 修复大锅燃烧加热器扣除时间不对问题 | ✅ (见注) | BurningHeaterBlockEntity.consumeBurnTime(), LargeCauldronBlockEntity.java 多处重构 |
💡 建议
- 注释记录
executed守卫移除意图 — 在triggerRecipeGroup中,如果多物品多扣费是故意行为,建议添加注释说明"同组每物品独立扣费"以避免未来误解。 super.stepOn()调用顺序 — 两个 Heater 的stepOn()都在调用HeaterUtil.hurtEntity()后调用super.stepOn(),确保父类行为(如绊线钩、压力板等)仍正常触发。✅- EOF 修复 —
SuperHeatingRecipe.java缺少的末尾换行已被修复 ✅
结论: COMMENT
PR 正确实现了声称的四个修复目标。代码结构清晰——抽取了 ICauldronGeometry、IDamagingHeater 接口和 HeaterUtil、CauldronUtil 工具类,减少了重复代码。主要的关注点是 triggerRecipeGroup 守卫移除可能导致的燃烧时间扣费变化,建议原作者确认是否为有意行为。无阻塞性问题。
由 Hermes Agent 审查
|
Now I have a thorough understanding. Let me compile the review. 代码审查摘要 — PR #4189操作: edited 变更概览
🔴 关键问题暂无 — 逻辑变更经过验证且正确。
|
| 声称 | 状态 | 对应文件 |
|---|---|---|
| 巨型铁砧时大锅顶部完整碰撞 | ✅ | LargeCauldronBlock.java — getShape() 中 part.getOffsetY() == 2 + 持有巨型铁砧 |
| 修复各种锅和缸的伤害问题 #4186 | ✅ | LivingEntityEventListener.java, CauldronUtil.java, HeaterUtil.java, LargeCauldronBlockEntity.java, HeaterBlock.java, BurningHeaterBlock.java |
| 多方块下石切机任意 part 被砸触发破坏 | ✅ | AnvilEventListener.java — hasStonecutterBelowAnyPart() 遍历所有 part |
| 大锅燃烧加热器扣除时间不对 #4187 | ✅ | BurningHeaterBlockEntity.java — consumeBurnTime 简化 + 增加 updateBurningState() |
结论: COMMENT — 代码质量好,重构和修复的逻辑正确。仅有一条建议核实 LargeCauldronBlock 是否在 BlockTags.CAULDRONS tag 中,以及对 BurningHeater 在大型锅下的伤害行为做最终确认。整体可以合并。
由 Hermes Agent 审查
Reason: Security scan — [HIGH] Pipe to interpreter: curl | python3: Command pipes output from 'curl' directly to interpreter 'python3'. Downloaded content will be executed without inspection. Reply |
|
好的,我已经仔细分析了完整的 diff。以下是我的审查意见。 代码审查摘要 — PR #4189作者: WhereisFff
🔴 关键问题1.
|
| 声称 | 状态 | 对应文件 |
|---|---|---|
| 手持巨型铁砧时大锅顶部视为完整碰撞 (fix #4186) | ✅ | LargeCauldronBlock.java |
| 修复各种锅和缸的伤害问题 (fix #4186) | 见关键问题 #1 (Heater POWERED 语义) | |
| 多方块下有切石机任意 part 被砸正确触发破坏 | ✅ | AnvilEventListener.java |
| 修复大锅燃烧加热器扣除时间不对问题 (fix #4187) | ✅ | BurningHeaterBlockEntity.java |
结论: REQUEST_CHANGES
需要修复:
- 🔴 关键 在REI渲染方块(初步测试) #1: 确认
HeaterBlock.isActive()对POWERED的检查是否是有意的行为变更 - 🔴 关键 [修复] 铁砧砸物品后客户端物品数量不变 #2: 巨型铁砧处理大锅配方后
TimeWarpPlayerBehavior意外触发问题 - 🔴 关键 升级fabric loom #3:
consumeBurnTime移除物品消耗后,需确认燃料消耗路径完整性(ConsumeFuel配方结果是否仍正确调用它) ⚠️ 警告 [Bug] Amythest tools have unappropriate attribute values #5: 巨型铁砧碰撞箱应覆盖整个顶层,而不仅是中心块⚠️ 警告 Fix attribute values of amethyst tools #6:hurtFromHeaterBelow每次实体 tick 的性能影响
次要建议(可选):
CacheableBERenderingPipeline.blockRemoved()vsupdate()的方法差异确认HeaterUtil.hurtEntity中isSteppingCarefully()的兼容性确认
由 Hermes Agent 审查 — PR #4189
|
审查完成。以下是对 PR #4189 的详细分析: 代码审查摘要 — PR #4189操作: edited 概览:5 个 Bugfix / 优化项
🔴 关键问题未发现关键问题。所有 5 项修复逻辑自洽,坐标算术和经验证无回归。
|
| 声称 | 状态 | 说明 |
|---|---|---|
| 手持巨型铁砧时大锅顶部视为完整碰撞 | ✅ | getShape() 中 part.getOffsetY()==2 && isHoldingItem(GIANT_ANVIL) → Shapes.block() |
| 修复各种锅和缸的伤害 (#4186) | ✅ | ICauldronGeometry + IDamagingHeater 接口 + CauldronUtil.hurtFromHeaterBelow() 统一处理 |
| 多方块下面有切石机任意 part 被砸触发破坏 | ✅ | hasStonecutterBelowAnyPart() 遍历所有 part |
| 大锅燃烧加热器扣除时间不对 (#4187) | ✅ | consumeBurnTime 简化为直接扣缓存,移除燃料槽消耗逻辑 |
| 环形光路 bug (#4152) | ✅ | onCancelingIrradiation 添加 contains(source) 守卫 + setRemoved() 客户端早期返回 |
结论:APPROVE(无阻塞性问题)
所有 5 项 Bugfix 逻辑正确且彼此不冲突。两个警告(行为流程变更、加热器熄灭闪烁)属于已知的可接受范围,不影响合并决策。
由 Hermes Agent 审查
Reason: script execution via -e/-c flag Reply |
代码审查摘要 — PR #4189操作: synchronize 📋 声称验证表
🔴 关键问题1. LargeCauldronBlock 未注册
|
Uh oh!
There was an error while loading. Please reload this page.