Skip to content

Refactor recipes / 重构配方结算并支持催化剂与来源筛选(26.1) - #137

Merged
PigeonNian merged 1 commit into
dev/26.1from
fff/26.1-recipe
Oct 3, 2026
Merged

PigeonNian merged 1 commit into
dev/26.1from
fff/26.1-recipe

Conversation

@WhereisFff

Copy link
Copy Markdown
Contributor

No description provided.

- 统一物品实体与容器槽位缓存,修复重复结算、回滚和溢出产物丢失
- 记录实际原料消费并支持原位催化剂预留、批次组件传递和来源过滤
- 明确提交阶段与完成事件,延后世界副作用并优化匹配及索引生命周期
- 增加配方守恒、来源隔离、序列化和异常路径游戏测试
@Gugle2308

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ✅ Compatible 0
collision ✅ Compatible 0
cube ✅ 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
math ✅ Compatible 0
multiblock ✅ Compatible 0
recipe 🔴 BC detected 1
registrum ✅ Compatible 0
sync ✅ Compatible 0
wheel ✅ Compatible 0
main ✅ Compatible 0

🔴 recipe — 1 breaking change(s)

dev/anvilcraft/lib/v2/recipe/predicate/item/HasItemBase.java:174

dev.anvilcraft.lib.v2.recipe.predicate.item.HasItemIngredient$Type
dev.anvilcraft.lib.v2.recipe.predicate.item.HasItemBase$AbstractType.codec()
METHOD_RETURN_TYPE_CHANGED_INCOMPATIBLE
✓ binary-compatible
✗ source-breaking

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

@Gugle2308

Copy link
Copy Markdown

All checks are done. Here is my review.


代码审查摘要 — PR #137

操作: opened(data-status="pullOpened",未合并,正常审查)
范围: 27 个文件(21 个改动 Java + 6 个新增 Java + 1 个 NBT),+1416 / −174;module.recipe +680,module.test +736
核对方式: 除 diff 外,我 fetch 了 rvbase261(dev/26.1 = 8a65beaf)与 rvhead137(PR head = 45278f95)的真实树,对 HasItemBase / HasItemIngredient / ItemCache / ICacheInputOutputImpl / AbstractCacheElement / ShapelessMatcher / InWorldRecipeContext 逐方法对照读取,并交叉核对了下游消费方 AnvilCraft(review-base)。


✅ 我实际复算过的部分(结论:核心重构逻辑正确)

  1. 催化剂(consume=false)机制自洽:匹配阶段 HasItemIngredient.snapshot() 无条件 shrink()(占位,防止同一物品被两个谓词重复使用),accept 阶段 restoreConsumedItems() 归还 + clearConsumedItems() 取走 receipt。AbstractCacheElement 用 shrinkSnapshots(IdentityHashMap) + releasedReservations 双重防重复归还,rollbackShrink 命中已归还时 return 0 不二次归还 —— 归还/回滚/提交三条路径不会互相叠加。recipe_catalyst_reserves_without_consuming 与 checkReturnedCatalyst(shared/separate) 覆盖了共享槽与独立槽两种接线。
  2. ShapelessMatcher.checkItemCapacity 新快速路径是安全的:IMPOSSIBLE 只用了三个必要条件(单谓词需求 > 其候选元素总量 / 总需求 > 并集容量 / 同候选集合组的需求 > 该集合容量);LINEAR 仅在候选集合两两不相交时启用 —— 此时每个谓词的 test 只读自身候选元素残量,与顺序无关,因此与逐排列回溯等价;其余一律回落 BACKTRACK(原算法,且 next.remove(i) 修掉了 base 里"同一实例出现两次会一起删掉"的瑕疵)。我构造了候选集合重复合并(demands.merge 同键)、部分重叠两类边界,未找到误判 IMPOSSIBLE 或误判 LINEAR 的反例。
  3. inRange/matchesRange 由"并集包围盒"改为"逐个已扫描范围"是正确修复:base 用 this.range.contains(pos,range)(所有扫描范围的并集包围盒)判定"已扫描",请求落在两个不相交已扫描区域之间的空隙时会被误判为已覆盖而漏扫;现在逐范围 anyMatch 修掉了(recipe_disjoint_scans_discover_gap 覆盖)。
  4. 共享物理槽去重链路完整:handlerElement()(ItemCacheSlotProvider 视图链 + 环检测 + handlerElements/containerHandlers/entityElements 三个 IdentityHashMap 缓存)+ endCache() 的 synchronizedElements identity 去重 + dirty 标志 ⇒ 同一槽位只 sync 一次、未变化的槽位不写回。这修掉了 base 里输入/输出两条路径各 sync 一遍导致的双写与重复回调(recipe_output_callback_runs_once、recipe_queries_track_replaced_items 覆盖)。
  5. AbstractCacheElement.grow 的 clamp 是实质修复:base int grownCount = grownSimulateCount - simulateCount 在 getCapacity(stack) < simulateCount(替换成容量更小的物品时可达)会得到负 grownCount,随后 simulate.shrink(负数) 会让模拟栈反向膨胀,同时遗留一个 CacheOperation(0) 挂在 growSimulateStack 上使 rollbackGrow 配对错位;现在 clamp 到 [0, …] 且 grownCount == 0 直接提前返回,两处都干净了。
  6. SaveComponentToTag 的 null 分支是修复:base 在组件缺失时把 null 直接喂给 codec().encodeStart(ops, object)(SAVED_ENTITY 这类无默认值的组件必然命中),会抛异常并沿 accept → assemble 直接漏到 anvil 事件里;现在移除 tag 并返回,下游 ApplyTagToComponent 对 null tag 正是"原样返回",两边配对一致。
  7. 幂等契约成立:context.accept() 二次调用不重放 acceptor(recipe_commit_is_idempotent),ItemCache.endCache() 二次调用 early-return,element.sync() 靠 dirty 保证重复调用无副作用。
  8. 与下游 AnvilCraft 的兼容性:4 个执行点(AnvilEventListener:119-129、LargeCauldronBlockEntity:832、ProceduralProcessStepManager:129/189)都是"新建 context → 一次 trigger → 一次 accept",与新的单次执行契约(assertPlanning 在 commit 后拒绝继续使用)兼容,recipeMatches 前置的 assertPlanning() 不会打到既有路径。
  9. 新增的二进制资产有效:recipe_empty.nbt gzip 解压后是合法 structure(size/blocks 空表/entities 空表/palette 仅 minecraft:air)。

⚠️ 需要注意

  1. 这套新增的 gametest(736 行)在 CI 里永远不会被编译或执行。 证据:.github/modules.json(head 与 base 完全一致)没有 test 条目,pull_request.yml 的 build-l0/l1/l2 矩阵由它生成,因此不会构建 module.test;git grep recipeTests rvhead137 -- .github 为空,没有任何 workflow 传 -PrecipeTests;module.main/build.gradle 也不依赖 module.test。也就是说 PR 里 20 个回归用例 + 新的 recipeGameTestServer run 目前只靠本地手动执行,编译错误或断言失败都不会被 CI 拦下。建议把 test 加入 modules.json(或在 main job 里补 :anvillib-test-neoforge-26.1:compileJava + -PrecipeTests 跑 gametest)—— 否则这些"回归测试"的收益接近零。
  2. -PrecipeTests 开关没有文档。 它同时是 gametest run 的开关和 AnvilCraft runtimeOnly 依赖的开关,建议在 README/模块说明里写清 ./gradlew :anvillib-test-neoforge-26.1:runRecipeGameTestServer -PrecipeTests 的用法(PR 未改动任何文档)。
  3. 两处"宽松 → fail-fast"的加载期收紧属破坏性变更,建议在 PR 描述里点明:
    • HasItemIngredient 构造器新增 item.count() <= 0 → IllegalArgumentException,但 ItemIngredientPredicate.CODEC 仍是 Codec.INT.optionalFieldOf("count", 1)(允许 0/负数)。旧数据包写 "count": 0 时语义是"恒真占位谓词"(testCount(0) = count >= 0),现在会在解析期抛异常。
    • max_efficiency 由 Codec.INT 收紧为 Codec.intRange(1, Integer.MAX_VALUE),且 InWorldRecipe 构造器 maxEfficiency < 1 抛异常;而 InWorldRecipeBuilder.maxEfficiency(int) 无校验,下游传 0 会在 build() 时炸。
      方向上都合理,但请确认是有意的,并考虑写进 PR 描述/CHANGELOG。
  4. SaveComponentToTag 语义从"首个匹配元素生效(computeIfAbsent)"变为"最后处理的堆栈生效(putTag),且组件缺失会删除已有 tag"。 单物品 / maxEfficiency(1) 的既有配方(AnvilCraft 三条 time_warp)行为不变;但当一个谓词消耗多个物品(count ≥ 2)时,结果组件取决于 receipt 的处理顺序,而元素集合顺序来自 level.getEntities / 方块遍历,不稳定;此外一个"无该组件"的堆栈会清掉同批其它堆栈刚写入的值。base 在这种混用下是直接抛异常,所以这仍是净改善 —— 只是残留一个顺序依赖。建议显式定义"首个非空优先",或在 javadoc 写明"最后消耗的物品决定组件值"。
  5. ICacheInput.supportsConsumptionReceipts() 的能力判断放在 wrapper 上,而"能否归还"的能力其实在元素上。 ICacheInputOutputImpl 无条件返回 true,但 ICacheElement.consume() 的默认实现把 restoration 设成抛 UnsupportedOperationException("This cache element cannot restore reservations")。当前库内元素全是 AbstractCacheElement(已覆写 consume),所以不会触发;但只要将来有第三方 ICacheElement 进入 ItemCache.inputs,catalyst 就会在 accept 阶段炸,且报错来自更内层、难以定位。建议 supportsConsumptionReceipts() 按元素计算(或至少把该契约写进 javadoc)。
  6. ICacheInputOutputImpl.rollbackShrink() 的返回值在"预留已被归还"时会变成 0(AbstractCacheElement.rollbackShrink 命中 releasedReservations 时 return 0),而契约注释仍是"回滚的数量"。库内调用方都忽略返回值,属 API 语义漂移;建议文档化或让元素侧记录原始 amount。
  7. receipt 是"按位置 FIFO"而非"绑定到谓词/操作":getConsumedItems()/clearConsumedItems()/restoreConsumedItems() 都是 peek/poll 最早那份。正常路径上 accept 顺序 == push 顺序,因此一一对应;但若两个相等谓词(同 item 谓词 + 同 range + 同 sourceFilter,例如同一实例同时出现在 conflicting 与 nonConflicting)共用同一个 ICacheInput 且 consume 标志不同,restoreConsumedItems() 归还的会是先入队的那份预留、可能落在另一个元素/槽位上。由于两边物品组件相同,宏观结果一致,我判定为潜在而非现实缺陷;若以后要支持"同谓词 + 不同 consume + 跨容器",建议把 receipt 绑到 commit 动作(返回令牌)而不是靠位置。
  8. ItemEntityCacheElement.sync() 的 if (!this.dirty) return; 意味着范围内未被本 PR 改动的物品实体不再被 setAge(0)(base 每次 endCache 都会重置其 despawn 计时)。如果存在依赖"配方执行期间范围内物品不消失"的行为,请确认这是有意的。

💡 建议

  • InWorldRecipeCommittedEvent 只在 InWorldRecipeManager.trigger 的循环里经 afterCommit 注册;AnvilCraft 里手工驱动配方的地方(如 LargeCauldronBlockEntity.triggerOneRecipe 直接 for 循环 matches/assemble/accept)不会发出该事件。建议抽一个"执行一次配方并排队 committed 事件"的公共方法,避免下游各写一份而漏发。
  • 该事件的 javadoc 已坦白"context 的 tag cache 不是该 batch 的快照"(afterCommit 直到整轮执行结束才统一运行,tag cache 已被后续 batch 覆盖,batchId 是唯一区分)——但事件名 + batchId 仍容易被误当成"该批次组件/产物快照"。建议事件里直接带上该 batch 的 tag 快照,或给出明确的替代获取方式。
  • ItemCacheEvent.SelectOutput 只在缓存 miss 时发布一次(同 key+range 复用),因此"依据当前容器状态"的取舍会在整个执行期内粘住;若这是设计意图,建议在 javadoc 里写明"每次查询仅触发一次且结果被缓存"。
  • ICacheElement.getStack() 的默认实现用 apply() + 元素总 count 拼装(多身份实现会取最后一个堆栈并把总数量塞进去;AbstractCacheElement 已正确覆写为 simulate)。建议标注"仅适用于单身份元素"或改为抽象方法。
  • context.assertPlanning() 引入了单次执行契约(commit 后 matches/putAcceptor 都抛 IllegalStateException)。现有 4 个下游调用点兼容 ✔,建议在 InWorldRecipeContext javadoc 写明生命周期(fresh → planning → accept → completed/invalid)。
  • ICacheInputOutputImpl.clearStack() 不再清 spawnSimulateStack / 元素栈(由 prepareSync + element.sync 各自负责)。我核对过所有调用点,当前无泄漏路径;但 clearStack() 仍是 public API,建议在 javadoc 说明其语义已在本次重构中收窄。

🟢 看起来不错

  • 事务化收尾是本 PR 的核心价值:matches/assemble 都带 rollbackAfterFailure + invalidate()(聚合 suppressed 异常、pop 增加"必须是栈顶"断言并用 try/finally 保证出栈),accept() 把 acceptor 与 afterCommit 副作用分离、资源按 (phase, key) 稳定排序提交、失败即 invalidate 且拒绝重试 —— 相比 base 的"边匹配边改状态、异常直接漏出去"安全得多。失败三路径都有 gametest 覆盖。
  • ProduceExplosion 从"匹配期立即爆炸"改为 ctx.afterCommit(...),修掉了"爆炸先于物品结算"的真实顺序问题。
  • 一批防御性/确定性改善:List.copyOf 不可变化、OutputKey.copyWithCount(1) 归一化、LinkedHashSet/LinkedHashMap 稳定顺序、endCache 生成物品合并加 break、spawnOperation.stack().copyWithCount(1) 不再就地改 setCount、事件 getStack() 返回副本、ResourceEventListener 的 initManager 加 null 守卫避免每次单人同步重建整个 manager + 剪枝计划。
  • 大量 JSpecify @Nullable / 新测试包 @NullMarked 迁移,与仓库既有 nullness 规则一致。
  • 测试设计有针对性:DrainingHandler 验证共享槽单次 sync、IdentityKey 验证 computeByIdentity 不与 equals 混淆、queriesTrackReplacedItems 验证缓存查询不误计替换后的物品。

📋 声称验证表

PR 声称 状态 证据
重构配方结算 ✅ InWorldRecipe.matches/assemble + InWorldRecipeContext.accept/invalidate/assertPlanning + commitPhasesAreStable/failedCommitSuppressesEffects
支持催化剂 ✅ HasItemIngredient.consume(CODEC + STREAM_CODEC 往返)、ItemConsumption、getConsumedItems/restoreConsumedItems/clearConsumedItems、ItemCacheSlotProvider 共享槽处理
支持来源筛选 ✅ ItemCache.getInput/getOutput(..., sourceFilter) 重载、ICacheElement.getSource/getSourceSlot、HasItemBase.getSourceFilter(库内暂无覆写者,属对外新增 API)
新增回归测试 ⚠️ 20 个 gametest 已实现且覆盖失败路径,但 CI 不构建 module.test、也无任何 job 传 -PrecipeTests → 实际不会被运行
面向 26.1 线 ✅ fff/26.1-recipe → dev/26.1,gradle.properties minecraft_version_range=[26.1,26.2)

结论: COMMENT — 核心重构的等价性与失败安全性经我逐方法复算未发现会破坏现有调用方的缺陷(并修掉了 base 的负 grownCount、跨空隙漏扫、重复 sync、组件缺失抛异常等真实问题);合并前主要诉求是 ⚠️1(把新增测试接入 CI) 与 ⚠️3(在描述里点明 count/max_efficiency 的加载期收紧),其余为文档化与 API 契约明确化建议。

说明:gh auth status 显示 /opt/data/home/.config/gh/hosts.yml 的 token 已失效(Active account Gugle2308,token invalid),因此本次未能用 gh pr edit 直接改标题。当前标题 Refactor recipes 偏笼统,建议改为:
Refactor in-world recipe settlement with catalysts and source filtering 重构配方结算并支持催化剂与来源筛选

🧪 测试建议

被测目标 推荐场景 优先级
ShapelessMatcher.checkItemCapacity 谓词数 ≥ 6、候选集合部分重叠(迫使 BACKTRACK)与完全不相交(LINEAR)两条路径的结果一致性对拍:同一物品组合下"新实现 vs 逐排列回溯"必须给出相同 matches 🔴
AbstractCacheElement.grow/rollbackGrow getCapacity(stack) < simulateCount(换成小容量物品)、grownCount == 0 时不产生悬挂操作、连续 grow/rollback 后 simulate 与快照严格对消 🔴
HasItemIngredient 序列化 count = 0 / 负数 JSON 与 max_efficiency = 0 的加载行为(确认是"拒绝加载"而不是服务端崩溃),以及旧数据包升级路径 🟡
SaveComponentToTag 一次谓词消耗 2+ 个"部分带组件"的堆栈,断言结果组件确定(并固化"最后生效/首个生效"的语义) 🟡
ItemCache.getOutput + SelectOutput 同一 key+range 复用(事件只发一次)、事件取消后回落生成目的地、两次 endCache() 不重复生成 🟡
InWorldRecipeContext 生命周期 commit 后再次 matches/putAcceptor/afterCommit 抛 IllegalStateException;invalidate() 后 accept() 抛异常 🟢
ItemEntityCacheElement.sync 未被改动的范围内物品实体不再重置 age 的行为是否符合预期(despawn 计时) 🟢

由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown

💾 Self-improvement review: Skill 'recipe-settlement-refactor-review' created. · Patched SKILL.md in skill 'recipe-settlement-refactor-review' (1 replacement).

@PigeonNian
PigeonNian merged commit 432b60d into dev/26.1 Oct 3, 2026
45 checks passed
@Gugle2308

Copy link
Copy Markdown

收到的是 closed 事件,按约束 1(closed/labeled → stop)处理,不发布任何审查评论。

PR #137(fff/26.1-recipe → dev/26.1,Refactor recipes / 重构配方结算并支持催化剂与来源筛选)已被关闭,无审查输出。

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