Skip to content

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

Merged
PigeonNian merged 1 commit into
dev/1.21.4from
codex/1.21.4-recipe
Oct 3, 2026
Merged

PigeonNian merged 1 commit into
dev/1.21.4from
codex/1.21.4-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 ⚪ Skipped —
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:173

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

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

@Gugle2308

Copy link
Copy Markdown

代码审查摘要 — PR #141

操作: opened(state=open、merged=false,未合并,照常审查)
范围: 29 个文件(26 Java / 10 新增 / 0 删除 / 1 二进制 nbt)/ +1684 −177 / head 52d5fc4
diff 完整性校验: 29 个 diff --git 头 ↔ API changed_files=29;^+ 行 1712 − 28 个 +++ 头 − 1 个二进制条目 = 1684 ↔ API additions=1684 ✅ 未截断;无 ghost 文件;无 TODO/FIXME/printStackTrace。
PR 描述: body: null(无描述、无 checklist)→ 下文按标题核对,并据此提出补齐要求。

变更地图

主题 关键点
结算阶段模型 InWorldRecipe.matches 只做预订(不再 clearStack)、assemble 开 batch/末尾提交、失败回滚到初始栈深 + invalidate();InWorldRecipeContext 的 acceptors 改为 (phase,key) 确定性排序、beginBatch/afterCommit/assertPlanning/computeByIdentity、pop 加栈顶断言
催化剂 HasItemIngredient.consume(boolean) + Builder + JSON optionalFieldOf("consume", true) + STREAM_CODEC 追加 bool
消费凭据 ItemConsumption(新)、ICacheInput.supportsConsumptionReceipts/getConsumedItems/clearConsumedItems/restoreConsumedItems/availableElements、consumedOperations
来源筛选 getInput/getOutput 4 参重载 + InputKey/OutputKey(filter 进缓存键)+ ItemCacheSlotProvider(视图→物理 (handler,slot) 归一)
输出选择 ItemCacheEvent.SelectOutput(可取消)+ 取消后回退新生成 ItemEntity
扫描/一致性 inRange→scannedRanges(修复断层漏扫)、dirty 标志、identity 去重、严格 extract/insertChecked
匹配加速 ShapelessMatcher.checkItemCapacity(IMPOSSIBLE/LINEAR/BACKTRACK)+ backtrack 改索引删除(修同实例重复谓词)
测试 module.test 新增 21 个 game test + recipeTests gradle run

🔴 合并前需澄清

  1. 新测试没有任何可发现的执行路径 —— 两重门:(a) RecipeRegressionTests:79 需系统属性 anvillib.recipeTests=true,只有新增的 recipeTests run 设置(module.test/build.gradle:309-316,需 -PrecipeTests);(b) .github/modules.json 无 test 模块 → CI 矩阵(level_0..2 + main)不构建 anvillib-test-neoforge-1.21.4,.github 内 grep recipeTests 零命中。README:361 已声明该模块是开发/测试模块、"不进聚合产物",所以不进 CI 符合既有约定(portSmoke 亦同)——但 PR 描述为空,未说明怎么跑、跑没跑过。请补:./gradlew :anvillib-test-neoforge-1.21.4:runRecipeGameTestServer -PrecipeTests 的实际通过输出(或把该 run 接入某个 workflow)。这次是把结算语义重写,21 个 test 是唯一可核证的依据,纯 review 无法确认。

⚠️ 警告

  1. 严格校验的异常会穿出事件总线(库使用者的崩服风险):ItemHandlerCacheElement.sync() 的新 insertChecked/extractChecked 在 handler 不完全遵守 simulate 契约时抛 IllegalStateException(NeoForge 允许 insertItem 返回余量,PR 自带的 dishonest fixture 正是这种)。commit 路径是 InWorldRecipeContext.accept() ← ItemEntityEventListener:23,事件处理器无 try/catch,异常会经 NeoForge 派发上抛 → 崩服/崩溃报告。旧行为是静默部分写入,现在是硬异常;请明确策略(在 trigger/事件边界 catch+log 并放弃本次 commit,更符合库的定位,或在文档里写清"misbehaving IItemHandler 会崩游戏"是可接受代价)。
  2. "先真提取、后真插入"可留下半提交状态:ItemHandlerCacheElement:114-121,槽位非空且物品类型变化时,真实 extractItem(118) 在真实 insertItem(120) 之前;插入被拒时槽位已被清空且异常上抛,而缓存已 committed=true(不可重试)。建议把插入可行性校验做成可回滚顺序(先校验/试插,再提取),至少在异常里保留槽位原状信息。
  3. 跨仓库(AnvilCraft)需同步三处:
    • HasDiffItems 用 mixin accessor 自行复刻了 ICacheInputOutputImpl#shrink(直接 element.shrink(1) + shrinkSimulateStack.push(new InputOutputOperation(...))),不写新的 consumedOperations 凭据。因其视图独享,当前不会串味,但它从此拿不到消费凭据语义;建议改走公开 API,并考虑补一个"按元素各减一"的公开入口,别再依赖 accessor。
    • 同文件仍用旧的 InWorldRecipeData.of(AnvilLibRecipe.of("item_cache_input/%s".formatted(this.hashCode()))) 缓存视图,而 HasDiffItems 是 record → 值相等实例共用视图(本 PR 已在库侧用 computeByIdentity 修掉同类问题),建议同步迁移。
    • LargeCauldronBlockEntity:1419 的 FLUID_RECIPE_ACCEPTOR 走 2 参 putAcceptor → 新实现把它归入 phase 200(InWorldRecipeContext:200-204 只识别 ITEM=0/BLOCK=100),流体提交从"hash 序"变成"item→block→fluid 之后"。语义可接受(commitFluidRecipes 只写流体),但属静默顺序变更;顺序重要时请改用新 3 参重载显式指定。
  4. ItemCacheSlotProvider 只有契约、仓库内无实现者:请说明面向谁(直觉是 AnvilCraft 的多方块/视图 handler)。同时注意语义——经 provider 归一到物理 handler 后,isItemValid/transfer 均以物理 handler 为准,视图自身的限制(只读、隐藏槽位、输出专用)会被绕过;需在接口注释写明"实现该接口即表示视图不是权限边界"。

💡 建议

  1. grow 不再压入 0 数量操作(AbstractCacheElement:125 早返回):内部调用方安全(ICacheInputOutputImpl.grow 用 remaining.getCount() < previousCount 判断是否入 op),但外部直接使用 ICacheElement.grow/rollbackGrow 的调用方在"什么都没吃下"时会弹出别的操作并错误 shrink simulate。要么保持压 0 操作,要么把该契约写进 ICacheElement 注释。
  2. 性能/状态:ItemCache.grow(331-332) 丢掉了旧的联合包围盒快速路径,改为 scannedRanges 线性扫描,且 scannedRanges/elementRanges 只增不减(每个新查询/元素一条)。正确性修复值得,但建议保留 O(1) 早退 if (this.range.contains(pos, range)) return;(联合盒包含查询是安全超集判据)。另外 range 字段现在只写不读(仅当"扫过没有"标志),可换布尔。
  3. ItemHandlerCacheElement 未同步 equals/hashCode 策略:ItemEntityCacheElement 已改 onlyExplicitlyIncluded(identity),handler 元素仍是 @EqualsAndHashCode(callSuper=false) 且含 iItemHandler.hashCode()。若某 handler 覆写了内容型 equals/hashCode,两个不同物理槽的元素会在视图 LinkedHashSet 里被去重掉一个 → 该槽静默不同步;去重已由 handlerElements(IdentityHashMap) 负责,建议这里也改 identity 或去掉 equals。
  4. ShapelessMatcher.checkItemCapacity:getClass() != HasItemIngredient.class(:59) 使 mod 侧子类永远走 BACKTRACK(保守、不产生假阴性);Set<ICacheElement> 作 HashMap 键依赖 SetFromMap(IdentityHashMap) 的相等语义,不相等时同样只是退化。建议加注释说明这些分支只影响性能——现在需要读者自行推演。
  5. API/健壮性小项:ItemCache.handlerElement 无空值防护(getItemCacheSlot 返回 null 或解析到 null handler 都会在深处 NPE);ICacheElement.consume 默认实现把失败推迟到回滚/催化剂时刻(自定义元素必须自行实现,建议写进注释);TestItemHandler.java 缺文件末尾换行(全仓唯一 No newline,顺手修);HasItemIngredient 现对 count<=0 抛异常、max_efficiency 走 intRange(1,…)——我已核对 AnvilCraft 的生成数据无 "count": 0 / "max_efficiency": 0,但其他数据包可能存在(旧行为下 count:0 是"恒真且不消耗"),建议在 PR 里显式标注这条向后不兼容。
  6. jspecify 方向正确(AnvilCraft 侧 AGENTS.md 指定其为唯一 nullness 注解),但同模块仍有 6 个文件用 javax.annotation.Nullable,且 compileOnly("org.jspecify:jspecify:1.0.0") 硬编码版本未进 gradle/libs.versions.toml。建议版本进 catalog,并说明本次是否只做"新代码用 jspecify"。

🟢 看起来不错

  • matches 成功不再丢弃回滚能力(预订可回滚到 assemble 提交),failedAssemblyRestoresReservations/matcherExceptionRestoresReservations 正好压住该不变量;rollbackAfterFailure 先回滚再 invalidate() 且保留 suppressed 链——比旧实现严谨得多。
  • pop 的栈顶一致性断言、accept() 幂等(completed)、资源失败不执行 afterCommit 副作用(commitPhasesAreStable/failedCommitSuppressesEffects)、putAcceptor 确定性排序。
  • ProduceExplosion 改 afterCommit 延迟爆炸,修掉"爆炸先于物品结算"的时序问题。
  • 扫描断层修复(disjointScansDiscoverGap)与物理槽归一(共享槽不再重复计数/重复同步)是真实缺陷修复。
  • 输出路径副作用收敛良好:SelectOutput 可取消、entity.isRemoved()/空物品守卫、identity 去重、dirty 标志、spawn 合并修掉 stack.setCount(1) 改写记录栈与首个匹配未 break。
  • JSON consume optional 默认 true,legacy 数据 + JSON/网络往返均有测试。
  • 测试断言贴近实现不变量(组件跟随被消耗物品、催化剂守恒、来源筛选计数与消耗同源)。

📋 标题声称核对(无描述,按标题)

声称 状态 证据
重构配方结算 ✅ InWorldRecipe / InWorldRecipeContext / ItemCache / ICacheInputOutputImpl / AbstractCacheElement
支持催化剂 ✅ HasItemIngredient.consume + codec/STREAM_CODEC + Builder;tests catalyst_reserves_without_consuming、shared/separate_handler_catalyst
来源筛选 ✅ 4 参 getInput/getOutput + InputKey/OutputKey;tests source_filter_accounts_exactly、output_filters_separate_keys

结论: REQUEST_CHANGES — 核心实现审查通过、未发现逻辑缺陷;需先办两件:(1) 给出新测试的可执行路径与通过证据(PR 描述为空);(2) 明确"严格校验抛异常"在库边界的处理策略(ItemEntityEventListener 无捕获 ⇒ misbehaving IItemHandler 会崩服)。其余为跨仓库跟进与非阻塞建议。

🧪 测试建议

被测目标 场景 优先级
ItemHandlerCacheElement.sync() 类型变化 + 非 IItemHandlerModifiable:真实插入被拒时槽位不应被清空(当前会) 🔴
InWorldRecipe.matches 匹配成功但未调用 assemble:断言残留预订/栈的契约行为 🟡
ItemCache.grow 大量互不包含的查询:scannedRanges 线性扫描的性能回归基准 🟡
ItemCacheSlotProvider 循环映射、返回 null、视图限制被绕过 🟡
HasItemIngredient count=0 / max_efficiency=0 的旧数据包加载错误信息清晰可读 🟢

标题建议(未自动修改)

gh auth status 显示 token 无效(Gugle2308 账户 token invalid),未执行 gh pr edit,仅给出建议(英文部分比"Refactor recipes"更具体):
Refactor recipe settlement; add catalyst (non-consuming) ingredients and source-filtered cache queries 重构配方结算并支持催化剂与来源筛选(1.21.4)


由 Hermes Agent 审查

@PigeonNian
PigeonNian merged commit 6138b0d into dev/1.21.4 Oct 3, 2026
40 checks passed
@Gugle2308

Copy link
Copy Markdown

PR #141 的操作类型为 closed(已关闭),按审查流程约定:closed/labeled 事件直接停止,不发布评论。

因此本次不做 diff 拉取与审查输出,也不会在 PR 上留下任何评论。若该 PR 后续被重新打开(reopened / ready_for_review),再触发一次即可正常审查。

@Gugle2308

Copy link
Copy Markdown

💾 Self-improvement review: Patched SKILL.md in skill 'offline-pr-evidence-verification' (1 replacement).

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