Skip to content

Fix 1.21.11 predicate previews 修复方块预览属性组合 - #144

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

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

Conversation

@WhereisFff

Copy link
Copy Markdown
Contributor

@Gugle2308

Copy link
Copy Markdown

已完成 diff 下载与跨文件/跨分支核对(base 源码取自 dev/1.21.11,diff 行号与上下文完全吻合,+11/−3、1 文件,无截断/无 EOF 缺失)。以下是审查意见。


代码审查摘要 — PR #144

操作: opened(state=open、mergeable=true,未合并)
范围: 1 个文件 / 35 行 diff(+11 / −3),无新增/删除文件
文件: module.util/.../util/predicate/BlockStatePredicate.java(两处 hunk)
目标分支: dev/1.21.11 ← WhereisFff:codex/fix-5206-predicate-render-states-1.21.11

🔍 根因定位(与 #5206 症状对齐,已交叉验证)

hunk 1 — 属性组合(constructStatesForRender,base 244–250 行)
旧代码把每个 PropertyMatcher 各自独立地作用在 defaultBlockState() 上再并入集合:

for (PropertyMatcher matcher : matchers)
    states.addAll(matcher.applyToState(definition, blockHolder.value().defaultBlockState()));

只要某个 matcher 的属性值恰好等于默认值,它就会把"原样默认状态"塞进预览集合。以 giant_anvil_1.json(AnvilCraft dev/1.21/1.6 已迁移为 lib 格式)里楼梯符号为例:{facing:south, half:bottom, waterlogged:false} —— half/waterlogged 两个 matcher 作用在默认楼梯(facing=north)上即产出 facing=north 的整块默认状态,与 facing=south 的正确状态一起进入预览列表。消费者再按索引/轮播取用(RecipeUtil.getDisplayIndex、JEI getDisplayedState 的 .filter(state -> state.is(block)).findFirst()、BlockTagUtil 的 millis/1000 % size),于是出现 "时而正确、时而全部朝北" 的周期性错乱——与 issue 描述完全吻合。
新代码改为"从默认状态出发逐 matcher 求笛卡尔积 + allMatch 终筛",预览集合与 testWithoutEntity 的 OR(组)/AND(组内) 语义严格一致。✅ 正确修法。

hunk 2 — 范围匹配(RangedMatcher.applyToState,base 619–625 行)

.map(minValue -> value.value().compareTo(minValue) < 0)   // 保留了 ≤ min 的“区间外”值
.map(maxValue -> value.value().compareTo(maxValue) > 0)   // 保留了 ≥ max 的“区间外”值

旧写法保留的是区间之外的值(且与 min/max 语义相反)。同 record 的 RangedMatcher.match(base 599–614 行)用的是 >= min / <= max,即"运行时判定"与"预览枚举"两个方向原本是反的。修正为 >= 0 / <= 0 后二者自洽。✅

🔴 关键问题

  • 无。变更小、语义闭环、可编译(PropertyMatcher.match(StateDefinition, StateHolder) 在 base 520 行已存在,diff 中新增调用点无 API 缺失风险;List<BlockState> 类型推断、states::add 丢弃 boolean 返回值均合法)。

⚠️ 警告

💡 建议(非阻塞)

  • 预览列表顺序仍不确定: Set<BlockState> states = new HashSet<>() + List.copyOf(states)(本次未改动)依赖 BlockState 的身份哈希,同一份配方在不同 JVM/启动进程间顺序会变。状态成员现在都合法了,影响降为"取哪个合法状态",但 AnvilCollisionCraftRecipe.getResult()(getStatesCache().getFirst().getBlock().asItem())、MineralFountainRecipe、AgeratumUtil.renderBlock 及 JEI 轮播都按索引/首元素取值,仍可能出现"两次启动结果物品/朝向不同"。建议顺手换成 LinkedHashSet(保持构造顺序)让预览稳定可复现——这是与 #5206"时有时无"直觉最贴近的残留风险点。
  • 笛卡尔积开销: 组内 k 个 matcher 各 n 值时为 Πn 的中间列表(每方块每组各算一次),对"tag 谓词 × 多属性"(如 #minecraft:stairs × facing/half/waterlogged/shape)会放大,但结果被 statesCache 缓存且仅用于渲染,可接受;如需优化可在每步后提前 filter(allMatch) 剪枝而非只在末尾筛。
  • 标题建议: 当前标题只提"属性组合",没体现范围过滤修复,且 gh 本机未认证(token invalid,未执行 gh pr edit)。建议改为:
    Fix predicate preview state generation (property combination + range filter) 修复方块预览状态生成(属性组合与范围过滤)

🟢 看起来不错

  • 预览集合与运行时判定语义(testWithoutEntity、PropertyMatcher.match、RangedMatcher.match)三处对齐,这是本 PR 最有价值的部分。
  • 边界语义自洽:min/max 无法解析为合法属性值时 applyToState 返回空 → 该组不再产出预览,与 match 返回 false 一致(旧代码里其他 matcher 仍会"漏"进去)。
  • 组内 matcher 指向方块不存在的属性时,PropertyMatcher.applyToState 返回空列表,新代码不会再像旧代码那样混入不匹配状态。
  • 空组("properties": [{}])现在会产出该方块默认状态,与"空组匹配一切"的运行时语义一致(旧代码此时什么都不渲染)。
  • 变更规模克制(+11/−3)、无 TODO/调试语句/凭据;hunk 行号偏移(619→627,净 +8)与 hunk 1 的净增行数吻合,diff 内部自洽。

📋 声称验证表

声称 状态 依据
修复预览状态的属性组合 ✅ hunk 1:笛卡尔积 + allMatch 终筛
修复预览状态的范围匹配 ✅ hunk 2:>= min / <= max,与 RangedMatcher.match 一致
fixed Anvil-Dev/AnvilCraft#5206 ⚠️ 根因对齐(楼梯"全朝北"= 默认状态被并集污染),但仅覆盖 1.21.4/1.21.8/1.21.11/26.1 四条 lib 线;#5206 的 1.21.1 复现线(anvillib-neoforge-1.21.1)未见对应 PR
描述"1.21.11"限定 ✅ base 为 dev/1.21.11,标题与分支一致

🧪 测试建议

被测目标 推荐测试场景 优先级
BlockStatePredicate.constructStatesForRender() 组内多 matcher 且部分属性值等于默认值(楼梯 {facing:south,half:bottom,waterlogged:false})→ 断言结果不含 facing=north 默认状态;结果全部满足 testWithoutEntity 🔴
同上 组内两条同属性 matcher 互斥(facing=south + facing=east)→ 结果为空(组不可满足) 🟡
同上 多组(OR)分别命中 → 结果为各组合集合并集、且无重复 🟡
RangedMatcher.applyToState() min=A,max=C → 仅返回 A/B/C(含端点);min 为非法字符串 → 空列表(与 match 一致) 🔴
getStatesCache() 同一实例重复调用返回同一不可变列表(缓存生效,distinct 去重正确) 🟢
tag 谓词(this.blocks 多元素) 各组独立求值、不串味;组内属性在该方块上不存在时该组对该方块不产出状态 🟡

结论: APPROVE — 两处 hunk 都精确命中根因(预览集合污染 + 范围判定反向),且与同类的 match 实现语义对齐,可直接合并。合并前请确认 ⚠️ 项:dev/1.21.1 是否需要同款 backport(#5206 的原始复现线)。


由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown

💾 Self-improvement review: Patched SKILL.md in skill 'blockstate-predicate-review' (1 replacement).

@Gugle2308

Copy link
Copy Markdown

🌿 Roseau API Breaking Change Report

Module Status Breaking Changes
codec ⚪ Skipped —
collision ⚪ Skipped —
cube ⚪ Skipped —
config ⚪ Skipped —
integration ⚪ Skipped —
moveable-entity-block ⚪ Skipped —
network ⚪ Skipped —
rendering ⚪ Skipped —
space-select ⚪ Skipped —
font ⚪ Skipped —
util ⚪ Skipped —
explosion ⚪ Skipped —
rpc ⚪ Skipped —
math ⚪ Skipped —
multiblock ⚪ Skipped —
recipe ⚪ Skipped —
registrum ⚪ Skipped —
sync ⚪ Skipped —
wheel ⚪ Skipped —
main ⚪ Skipped —

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

@WhereisFff WhereisFff closed this Oct 6, 2026
@Gugle2308

Copy link
Copy Markdown

PR #144 状态已确认为 closed(未合并),按 webhook 约定跳过审查,未发布审查内容。

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