Skip to content

Fix 1.21.4 predicate previews 修复方块预览属性组合 - #142

Closed
WhereisFff wants to merge 1 commit into
Anvil-Dev:dev/1.21.4from
WhereisFff:codex/fix-5206-predicate-render-states-1.21.4
Closed

WhereisFff wants to merge 1 commit into
Anvil-Dev:dev/1.21.4from
WhereisFff:codex/fix-5206-predicate-render-states-1.21.4

Conversation

@WhereisFff

Copy link
Copy Markdown
Contributor

@Gugle2308

Copy link
Copy Markdown

代码审查摘要 — PR #142

操作: opened(PR 状态:open,mergeable ✅,未合并)
仓库: Anvil-Dev/AnvilLib · base dev/1.21.4 (6138b0d) ← head codex/fix-5206-predicate-render-states-1.21.4 (1d5d6df)
范围: 1 个文件(1 Java,0 新增/0 删除)/ 35 行 diff(+11 / -3)
Diff 完整性: ✅ 与 API 对账一致(changed_files=1, additions=11, deletions=3),无截断、无 ghost 文件、无 EOF 缺失。

🔴 关键问题

无。未发现会引入崩溃或语义倒退的问题。

⚠️ 警告

  • BlockStatePredicate.constructStatesForRender()(L210+)— 输出顺序不确定(既有问题,但与本 PR 目标直接相关)
    states 是 HashSet<BlockState>,最后 List.copyOf(states)。BlockState 不覆写 hashCode,集合迭代顺序取决于身份哈希 → 每次 JVM 启动顺序可能不同。本 PR 正是为了让预览列出「真正匹配的属性组合」,而 JEI 侧(如 SqueezingCategory、BlockCrushCategory)会整表渲染,顺序抖动会让预览图标顺序在不同启动间变化。既然在动这块代码,建议改用 LinkedHashSet 固定顺序。非阻塞。

  • 下游 .getFirst() 空值守卫需复核(跨仓库)
    该 PR 改变了 getStatesCache() 的空/非空分布:范围谓词从「恒为空」变为「[min,max] 全部命中」;而互斥组合(如同一属性 name=a AND name=b)会变为空。我在 AnvilCraft 1.21.1 线核对到 MineralFountainRecipe.assemble() 直接 this.needBlock.getStatesCache().getFirst()(无 isEmpty 守卫),AnvilCollisionCraftRecipe.getResult() 有守卫。请确认 1.21.4 线对应消费者是否都做了空判断——本 PR 未触碰这些文件,但会改变它们的取值。

💡 建议

  • 方法契约与实际用法冲突:constructStatesForRender() 的 Javadoc 写明「此方法不应用于除渲染外的任何用法!」,而 MineralFountainRecipe.assemble()、AnvilCollisionCraftRecipe.getResult() 用它决定配方结果物品。建议要么拆一个非渲染访问器,要么更新注释。PR 后结果会更准确,但契约仍被违反。
  • .distinct() 冗余:候选集已通过 matcher.applyToState 产出(setValue 返回规范化实例),且最终 states::add 进 HashSet 再次去重,中间 .distinct() 只增加开销,可删。
  • 中间候选集增长:链式 flatMap 的中间规模为各 matcher 取值数之积(3 个 16 值属性 → 4096 个中间态),结果集本身正确且有 statesCache 缓存,但可在每步 matcher 之后立即做一次命中过滤提前剪枝,降低一次性分配峰值。
  • 补测试:当前仓库无任何测试引用 constructStatesForRender/RangedMatcher,建议补一组边界用例(见文末)。

🟢 看起来不错

  • 范围过滤器反转修复正确:原代码两个 filter 以 AND 串联,判据为 value < min 且 value > max —— 只给 min 会返回低于 min 的值,只给 max 会返回高于 max 的值,两者都给则恒为空。改为 >= min / <= max 后返回 [min, max],与 RangedMatcher.match()(t >= min、t <= max)语义一致。
  • 属性组合语义修复正确:原实现把每个 matcher 各自独立作用于 defaultBlockState,多属性谓词永远产不出「A=x 且 B=y」的组合态;新实现按 matcher 链式 applyToState(对应 testWithoutEntity 的 list 内 AND)并以 allMatch(matcher.match) 收尾,保证输出状态必然满足该谓词。
  • 泛型推断可编译:getStateDefinition() 返回 StateDefinition<Block, BlockState>,candidates 为 List<BlockState>,推断无误;applyToState/match 对缺属性的方块均有 hasProperty/property != null 处理。

📋 声称验证表

声称 状态 对应实现
修复预览状态的属性组合 ✅ constructStatesForRender 链式 applyToState + allMatch(match) 收尾
修复范围匹配 ✅ RangedMatcher.applyToState >=/<= 反转修正
fixed Anvil-Dev/AnvilCraft#5206 ✅ 库侧修复(AnvilLib 提供 BlockStatePredicate,AnvilCraft 消费)

🧪 测试建议

被测目标 推荐测试场景 优先级
constructStatesForRender() 单精确匹配 / 单范围 min-only / max-only / 双范围 / 精确+范围组合 / 多 matcher-list(OR) / 互斥组合→空 🔴
RangedMatcher.applyToState min==max 边界、min>max(应空)、min/max 字符串非法(回退 List.of(state) 后被过滤) 🔴
applyToState ↔ match 一致性 断言 applyToState 的输出状态全部 matcher.match()==true 🟡
多方块 HolderSet 部分方块缺该属性时整条 matcher-list 被正确剪枝 🟡

结论: APPROVE(可合并) — 两处修复都指向根因、语义自洽,diff 极小且完整。上述均为非阻塞项,建议至少确认下游 .getFirst() 守卫并补一个 constructStatesForRender 用例。

标题说明:gh auth status 显示 token 失效(hosts.yml is invalid),按流程未执行 gh pr edit。当前标题 Fix 1.21.4 predicate previews 修复方块预览属性组合 已满足「英文 + 中文」格式,可保留;如需更精确可改为 Fix 1.21.4 predicate preview state combination and range matching 修复方块预览属性组合与范围匹配。


由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown

💾 Self-improvement review: Skill 'blockstate-predicate-review' created.

@Gugle2308

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ⚪ Skipped —
collision ⚪ Skipped —
cube ⚪ Skipped —
config ⚪ Skipped —
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 ✅ 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

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ✅ Compatible 0
collision ⚪ Skipped —
cube ⚪ Skipped —
config ⚪ Skipped —
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 ✅ 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

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ✅ Compatible 0
collision ⚪ Skipped —
cube ✅ Compatible 0
config ⚪ Skipped —
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 ✅ 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

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ✅ Compatible 0
collision ✅ Compatible 0
cube ✅ Compatible 0
config ⚪ Skipped —
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 ✅ 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

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 ✅ Compatible 0
registrum ✅ Compatible 0
sync ✅ Compatible 0
wheel ✅ Compatible 0
main ✅ Compatible 0

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

@WhereisFff
WhereisFff marked this pull request as ready for review October 4, 2026 14:38
@Gugle2308

Copy link
Copy Markdown

代码审查摘要 — AnvilLib PR #142

操作: ready_for_review(PR 当前 state=open、merged=false、mergeable=true,1 commit)
范围: 1 个文件(1 Java,+11 / −3)/ 35 行 diff

diff 完整性对账 ✅ — patch-diff.githubusercontent.com 与 API Accept: application/vnd.github.v3.diff 两条路径 md5 一致(fdf6719dbadeb9050247e5abb69965a4),行数 35/35,与 API 元数据 changed_files=1, additions=11, deletions=3 吻合 ⇒ 无静默截断、无 ghost。
hunk 自洽 ✅ — hunk1 净 +8 ⇒ hunk2 由 @@ -613,9 … 变为 @@ -621,9 …,偏移正确。
base 交叉核对 ✅ — dev/1.21.4 全文(raw.githubusercontent.com 直取,685 行)逐上下文行吻合;PropertyMatcher.match(StateDefinition,S) 在 base 已存在(L514),新增调用可编译。


🟢 修复正确性(两处 hunk 都命中根因)

hunk 2 — 模式 1:RangedMatcher.applyToState filter 判据方向反转

-.map(minValue -> value.value().compareTo(minValue) < 0)   // 只保留「低于下界」
+.map(minValue -> value.value().compareTo(minValue) >= 0)
-.map(maxValue -> value.value().compareTo(maxValue) > 0)   // 只保留「高于上界」
+.map(maxValue -> value.value().compareTo(maxValue) <= 0)

两个 filter 是 AND 串联,旧写法必然:只给 min → 返回低于下界的值;只给 max → 高于上界;两者都给 → 交集恒为空。而同一 record 的 match() 本来就是对的(base L593-608:t < min → false、t <= max)⇒ 生成与判定语义相反,正是模式 1 的教科书形态。修复后两侧方向一致 ✅。

Python 逐行转写对照(真实 matcher 组合驱动,脚本 /opt/data/workspace/pr142_predicate_sim.py):

场景 OLD NEW
RangedMatcher(level, min=1, max=2) [] [1, 2] ✅
混合组 {facing:west(exact), level:[1,3](ranged)} [(west,0)](不满足 match) [(west,1),(west,2),(west,3)] ✅

hunk 1 — 模式 2:constructStatesForRender 逐 matcher 独立作用于 defaultState → 组合态缺失
旧代码每个 matcher 都从 defaultBlockState() 出发、互不叠加,A=x AND B=y 永远产不出真正的组合态;且任何「取值恰好等于默认值」的 matcher 都会把整块 defaultBlockState() 塞进预览集。新代码改为链式 flatMap(内层 AND)+ 收尾 allMatch(matcher::match) 过滤 ⇒ 输出保证满足 testWithoutEntity 语义闭环 ✅。

对照实测(giant_anvil_1.json 的楼梯符号 {facing:south, half:bottom, waterlogged:false}):

OLD: 2 states = [facing=north(污染,默认态), facing=south(正确)]
NEW: 1 state  = [facing=south]   ✅

这条链与 issue #5206 症状一「楼梯朝向周期性错误(时而正确、时而全部朝北)」逐字吻合:half / waterlogged 的值恰等于默认值 ⇒ 旧代码并入一条 facing=north 默认楼梯;「周期性」来自消费端按墙钟秒轮播(util/LevelLike.java 的 setBlockStateWithAlpha/getBlockState 用 (int)(System.currentTimeMillis()/1000) % blocks.size() 取,空表 → Blocks.AIR)⇒ size=2 时「时对时错」,模式 1 全反转时该位置方块直接消失。

修后不变量:constructStatesForRender() 每个状态都满足 testWithoutEntity();ValueMatcher 实现全集仅有 ExactMatcher + RangedMatcher(已 grep "implements ValueMatcher" 核实),两个都覆盖,无遗漏 matcher。


⚠️ 警告 / 确认项

  1. fixed #5206 只覆盖症状一,症状二未闭合 — 本次按技能要求跑了决定性判据:giant_anvil_1.json 的 anvil/方块谓词只有 exact matcher,无任何 min/max 字段(dev/1.21/1.6 与 dev/26.1/1.6 两线均为 "blocks"×9 / "properties"×7,min/max 计数 = 0)。⇒ 模式 1 的「范围谓词恒空」不会作用于这条配方,AnvilCollisionCraftRecipe.getResult() 的 isEmpty → 回退普通铁砧 路径不被触发 ⇒ 「产物的巨型铁砧时有时无」不由本 PR 解释。建议把该条写成「⚠️ 部分修复:需在含此修复的构建上复测症状二」,不要在 PR 里打 ✅ 关闭 #5206。
  2. backport 覆盖:dev/1.21.1 线无对应 PR — 同类 PR 序列已开着 #142=dev/1.21.4 / #143=dev/1.21.8 / #144=dev/1.21.11 / #145=dev/26.1(pulls?state=open 实测,四条同题同名)。而 issue #5206 复现线是 AnvilCraft dev/1.21/1.6,其 libs.versions.toml 依赖 anvillib-neoforge-1.21.1 = AnvilLib dev/1.21.1(该线无修复 PR),该分支 raw 全文仍命中 value.value().compareTo(minValue) < 0。措辞按确认项:若 1.21.1 线仍在维护请补同款 backport,否则 #5206 在报告版本上继续复现(不判 🔴、不断言会误关 issue——自动关闭只在合入默认分支时发生)。
  3. 下游 getStatesCache().getFirst() 无 isEmpty 守卫 — 本修复会改变缓存分布(范围谓词从「恒空」→「[min,max] 全命中」;MineralFountainRecipe.assemble 类调用无守卫)。AnvilLib dev/1.21.4 对应的 AnvilCraft 消费线未公开(当前只有 dev/1.21/1.6、dev/26.1/1.6),无法逐条核到本线消费者,请作者确认目标线消费端;并注意该类 Javadoc 写「仅限渲染」而 getResult()/assemble() 实际取用,语义与注释冲突(模式 4,既有问题)。

💡 建议(非阻塞)

  1. states 为 HashSet + List.copyOf:BlockState 全继承链未覆写 equals/hashCode(dtc → dtb$a → dte → Object,身份语义)⇒ 列表顺序随身份哈希变化,跨 JVM 启动不稳定。整表渲染/getFirst() 消费者会受影响 ⇒ 建议 LinkedHashSet(既有问题,非本 PR 引入)。
  2. 中间 .distinct() 与最终 Set 去重冗余,可删。
  3. ExactMatcher.applyToState 在「属性存在但值无法解析」时返回 List.of(state) 原样放行(L558-564),只靠收尾 allMatch 才剔除;建议改成 List.of(),与 PropertyMatcher.applyToState(property == null → List.of())统一,让「applyToState 输出必满足 match()」在局部成立、收尾过滤退化为纯防御。现状行为正确,仅建议。
  4. 空 AND 组行为变化:properties: [{}] 会解码成 List.of(List.of()),旧实现该组不产出状态,新实现折叠 + allMatch(空流)=true ⇒ 产出 defaultBlockState();而 testWithoutEntity 中空内层组是「空真」。属行为变化,建议确认是否有意为之(非缺陷)。
  5. 链式中间规模 = 各 matcher 取值数之积(exact matcher 塌缩为 1,比旧实现更省;只有同组多个 ranged 属性才会增长,且有 statesCache),仅提示。

🟢 看起来不错

  • 两处 hunk 精准命中模式 1 + 模式 2,与 match()/testWithoutEntity() 语义闭环,单文件小改、无副作用扩散。
  • 无新增 TODO/FIXME/调试语句/硬编码凭据;无 EOF 缺换行。

📋 声称验证表

声称 状态 对应实现
修复预览状态的属性组合 ✅ hunk1 链式 flatMap + allMatch(match) 收尾过滤
修复范围匹配 ✅ hunk2 >=min / <=max,与 RangedMatcher.match 对齐
fixed AnvilCraft#5206 ⚠️ 部分 症状一(楼梯朝向周期性)✅;症状二(产物巨型铁砧时有时无)不由本路径解释(配方的 anvil 谓词无 min/max),需复测

🧪 测试建议

仓库内无任何测试引用 constructStatesForRender / RangedMatcher。建议覆盖(纯函数抽取后单测,或 GameTest):

被测目标 推荐场景 优先级
RangedMatcher.applyToState min-only / max-only / min==max / min>max(应空) / 非法值字符串 🔴
RangedMatcher.applyToState state 缺该属性 → List.of(state),收尾过滤剔除 🔴
constructStatesForRender 精确+范围混合组、多 matcher-list(OR)、互斥组合(同名属性两个 exact 不同值)应空 🔴
constructStatesForRender 断言:输出每个 state 都满足 testWithoutEntity() 🔴
constructStatesForRender 空 AND 组 [{}]、多方块 HolderSet 缺属性剪枝 🟡

结论: APPROVE — 两处修复均正确、可合并;症状二与 dev/1.21.1 backport 两点写为确认项,其余为非阻塞建议。

标题说明:gh auth status 显示 token 失效(The token in /opt/data/home/.config/gh/hosts.yml is invalid,本会话按纪律跳过 gh pr edit,未导出任何凭据)。原标题 Fix 1.21.4 predicate previews 修复方块预览属性组合 已满足「英文 + 中文」格式,可保留;若想更精确,建议:Fix BlockStatePredicate render-state combination and ranged matching 修复方块状态谓词预览的属性组合与范围匹配。


由 Hermes Agent 审查

@WhereisFff WhereisFff closed this Oct 6, 2026
@Gugle2308

Copy link
Copy Markdown

PR #142 状态为 closed,按 webhook 规则(closed/labeled → 停止)不进行审查、不发布任何评论。已停止,无操作。

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.

2 participants