Skip to content

Reimplement negative shape cullface 重新实现负形剔除 - #4329

Open
QiuShui1012 wants to merge 6 commits into
Anvil-Dev:dev/1.21/1.6from
QiuShui1012:cull/1.21/1.6
Open

Reimplement negative shape cullface 重新实现负形剔除#4329
QiuShui1012 wants to merge 6 commits into
Anvil-Dev:dev/1.21/1.6from
QiuShui1012:cull/1.21/1.6

Conversation

@QiuShui1012

@QiuShui1012 QiuShui1012 commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator
  • 修改了约束仓角落的超限纹理,使其与其它超限方块同步

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /opt/data/workspace/AnvilCraft && git show FETCH_HEAD:src/main/resources/assets/anvilcraft/models/block/confinement_chamber.json > /tmp/confinement_chamber.json && python3 -c "
import json
with ope...

Reason: script execution via -e/-c flag

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /tmp && curl -sL "https://piston-meta.mojang.com/mc/game/version_manifest_v2.json" -o vm.json && python3 -c "
import json
m=json.load(open('vm.json'))
for v in m['versions']:
    if v['id'] in ('1....

Reason: script execution via -e/-c flag

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /tmp && python3 -c "import json; d=json.load(open('/tmp/confinement_chamber.json')); print('textures:', d.get('textures')); els=d.get('elements',[]); print('num elements:', len(els));
for i,e in en...

Reason: script execution via -e/-c flag

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /opt/data/workspace/AnvilCraft && echo "=== mixins.json 条目对比(common 列表)===" && python3 - <<'EOF'
import json, re
old = open('/tmp/pr4329.diff').read()
# 提取删减列表与新增列表
rm = re.findall(r'^-\s*"([^"]+)"...

Reason: Security scan — [CRITICAL] Mass file deletion in a short window: 4 non-build files were deleted within 20s. A burst of deletions can be destructive (ransomware-like or an accidental recursive wipe).

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /opt/data/workspace/AnvilCraft && git show 10d955b7:src/main/resources/anvilcraft.mixins.json | python3 -m json.tool > /dev/null 2>&1 && echo "mixins.json valid" || echo "mixins.json INVALID"; git ...

Reason: Security scan — [MEDIUM] Schemeless URL in sink context: URL without explicit scheme passed to a command that downloads/executes content; [HIGH] Pipe to interpreter: git | python3: Command pipes local output into interpreter 'python3'. This can execute or process unreviewed piped content; write it to a file and inspect it first when the content may be code.

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

所有关键验证完成。核心结论已确认:edgeQuads 过滤条件只保留该方向边缘四边形,而所有负形家族方块模型(余烬金属、霜冻、超限、约束仓)都含大量非边缘内部面(本体 0.5/15.5、仓壁 0/4/16 等)。以下为审查意见。


代码审查摘要 — PR #4329

操作: edited
范围: 9 个文件(4 新增,0 删除)/ 1198 行 diff
验证: PR open(head 10d955b7e → base dev/1.21/1.6 @ 7dcb6565);已用 base 分支源码交叉引用 INegativeShapeBlock / ITranscendiumBlock / 各方块模型 JSON

🔴 关键

  • NegativeShapeModelEventListener.javaedgeQuads,diff L205-221)— getQuads(state, side) 丢弃所有内部四边形,负形方块本体会整体消失
    渲染时 NegativeShapeBakedModel.getQuads(state, side) 先取 originalModel.getQuads(state, null, ...)(全量四边形),再经 edgeQuads 过滤:if (getEdgeDirection(quad) != side) continue;。而 getEdgeDirection(L242-251)只为恰好位于 0.025 / 15.975 模型边界的四边形返回方向(MIN_MODEL_EDGE/MAX_MODEL_EDGE,容差 0.0001)。
    实测所有负形家族模型都含大量非边缘四边形:

    • ember_metal_block.json / transcendium_block.json 主体立方体面在 0.5 / 15.5|15.5-15.975|=0.475 → 非边缘);
    • confinement_chamber.json 仓壁元素如 [12,0,7]→[16,4,9] 的 x=16 面、[7,0,0]→[9,4,4] 的 y=0/z=0 面(|16-15.975|=0.025 → 非边缘)。
      这些内部四边形在请求任意 side 时全部被 continue 丢弃,导致所有 INegativeShapeBlock 方块(余烬/霜冻金属、负物质、超限方块、超越系列、约束仓)在世界上只剩 0.025/15.975 的轮廓线,实体本体不可见。这不是特性而是渲染级回归——onModelBake 会包裹所有这些方块(diff L75-90 遍历全部 INegativeShapeBlock)。
      修复方向:edgeQuads 中非边缘四边形应按侧放行(本模型无 cullface,vanilla 语义为 unculled 四边形对每个 side 请求都返回),仅对 getEdgeDirection(quad) == side 的边缘四边形做 coverage 裁剪。
  • BlockMixin.java(L680-682)— shouldRenderFace 对负形方块无条件返回 true,放弃面级遮挡判断
    旧实现基于 getFaceOcclusionShape + joinIsNotEmpty(ONLY_FIRST) 真实判断面遮挡;新实现一刀切 cir.setReturnValue(true)。副作用:负形方块与完整实心方块(石头等)邻接时,边界处 0/16 的外壳面与邻居面完全共面 → z-fighting(当前被上一条 bug 掩盖,修复后立即暴露)。且新剔除体系只覆盖 0.025/15.975 轮廓矩形,相邻同族方块共面的 0/16 外壳墙(世界坐标同为 x=16)不在剔除范围内,修复 C1 后会出现双面 z-fight。建议明确"负形方块 vs 普通方块"的遮挡语义并补上面级剔除。

⚠️ 警告

  • BlockStateBaseMixin.java 在 common mixins 列表但引用 client 类 — 该 mixin 的 handler 调用 dev.dubhe.anvilcraft.client.event.NegativeShapeModelEventListener(内部引用 net.minecraft.client.renderer.RenderType 等)。专用服务器无 client 类,skipRendering 注入代码被加载/验证时可能 NoClassDefFoundError。建议移入 client 列表(与两个 compat mixin 一致)。

  • Sodium/Embeddium 兼容 mixin 的面级剔除与 vanilla 四边形级裁剪语义不一致shouldDrawSideshouldSkipFace 为 true 时整面不渲染(内部面一并消失),而 vanilla 路径(修复 C1 后)只裁剪边缘四边形、保留内部面。同一布局在两种渲染器下视觉不同。建议统一为四边形级处理,或明确 shouldSkipFace 的语义边界。

  • EDGE_RECTANGLES 静态 HashMap 的并发读写 — 模型烘焙(主线程 clear()+重填)与 Sodium/Embeddium 异步区块构建(工作线程读)并发。资源重载(F3+T)期间可能读到半填充状态,表现为剔除暂时失效。建议烘焙时构建新 map 整体替换。

  • 纹理"同步"不完整confinement_chamber.json 角落仅改了 texture: "#1",对比 transcendium_block.json/ember_metal_block.json 的 outline 元素:缺少 "light_emission": 15、faces 缺少 "rotation": 180、UV 为 [0,4,4,0] 而非 [0,0,16,16]。与 PR 声称"与其它超限方块同步"不完全一致(角落不会发光、贴图裁切不同)。若有意为之可忽略。

💡 建议

  • withoutEdgeQuads(side==null 路径)getQuads(state, null) 会剥离全部轮廓四边形 → 以实体形式渲染的方块(BlockDisplay、掉落方块)会缺 outline。确认是否预期。
  • shouldSkipFace 的 UV 空间覆盖检查忽略世界坐标偏移:相邻块边缘矩形实际位于不同平面(15.975 vs 16.025),系统按"覆盖"剔除一侧轮廓——这是边界线去重设计,但跨方块类型也会互相剔除(如约束仓旁放超限方块,getBlockType() 同为 ITranscendiumBlock.class),混合摆放时边界线只由一侧渲染。确认预期。
  • anvilcraft.mixins.json 整表重排(169 行纯缩进噪音):已验证无条目丢失(131→137,新增 BlockStateBaseMixin + 2 compat),但建议恢复原缩进以缩小 diff。
  • AnvilCraftMixinPlugin 门控字符串contains(".compat.Emb") 会误匹配未来 Emb* 前缀的其它 mixin,建议改为 endsWith 精确匹配。

🟢 看起来不错

  • mixins.json 条目完整性已验证:0 删除,3 新增,无遗漏。
  • 插件新增 hasSodium/hasEmbeddium 门控顺带修正了既有 SodiumChunkBuilderMeshingTaskMixin/EmbChunkBuilderMeshingTaskMixin 原先无条件应用的问题。
  • rebuildQuad 顶点重写正确:POSITION/UV(元素 0,1,2,4,5)双线性插值、COLOR(元素 3)分通道插值、UV1/UV2/NORMAL(6,7,8)离散取最近角点,避免破坏打包值。
  • FaceRectangle.subtract 四向分割 + addIfVisible 空矩形过滤、sameAs/容差处理正确。
  • compat mixin 的类路径检测(net/caffeinemc/mods/sodium/...org/embeddedt/embeddium/impl/...)与 mixin 目标一致;@EventBusSubscriber 写法与仓库既有 RegisterAdditionalEventListener 约定一致(MOD 总线事件可正常触发)。
  • JSON EOF 缺换行顺带修复。

📋 声称验证表

声称 状态 对应文件
重新实现负形剔除(模型四边形级裁剪 + skipRendering + sodium/embeddium 兼容) ⚠️ NegativeShapeModelEventListener(新)、BlockStateBaseMixin(新)、BlockMixin、Emb/SodiumBlockOcclusionCacheMixin(新)——架构完整但存在 C1 渲染级缺陷
约束仓角落超限纹理与其它超限方块同步 ⚠️ confinement_chamber.json——texture 已换 #1,但缺 light_emission/rotation/UV 对齐

结论: REQUEST_CHANGESedgeQuads 丢弃内部四边形导致所有负形方块本体不可见,属必须修复的渲染回归;修复后需同步确认相邻块 0/16 共面外壳与实心方块邻接的 z-fighting 语义,以及 vanilla/sodium 视觉一致性。

🧪 测试建议

被测目标 推荐测试场景 优先级
NegativeShapeBakedModel.getQuads(state, side) 内部四边形(0.5/15.5、0/16 面)在任意 side 请求下保真返回;边缘四边形按 coverage 裁剪 🔴
FaceRectangle.subtract / intersection 全覆盖、部分覆盖、相切、分离矩形组合;减法后碎片和等于原面积 🟡
shouldSkipFace 同族/异族/非负形邻接 3×3 矩阵;EDGE_RECTANGLES 空(未烘焙)回退 🟡
getEdgeDirection / isOnEdge 0.025/15.975 ± 0.0001 容差边界;非轴对齐四边形 🟢
rebuildQuad 顶点重写 切片后 UV 插值、COLOR 通道、UV1/UV2/NORMAL 保持 🟡
渲染集成 同布局 vanilla vs Sodium vs Embeddium 视觉一致性(对应 W2) 🟡

由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown
Collaborator

💾 Self-improvement review: Patched SKILL.md in skill 'minecraft-mod-pr-review' (1 replacement).

@Gugle2308

Copy link
Copy Markdown
Collaborator

分析完成。已交叉验证 PR head(10d955b7)、基分支与相关模型/Sodium 源码。以下是审查意见。


代码审查摘要 — PR #4329

操作: opened
范围: 9 个文件(7 Java:4 新增 + 3 修改;另含 mixins.json 与模型 JSON)/ 1198 行 diff

实现概览: 旧的 shouldRenderFace + OCCLUSION_CACHE + 3D VoxelShape 剔除逻辑整体删除,改为「矩形覆盖剔除」:

  • BlockStateBase.skipRendering HEAD 注入 + NegativeShapeModelEventListener.shouldSkipFace()(基于烘焙模型在 0.025/15.975 平面上的面矩形做 2D 覆盖判断)
  • 模型烘焙时用 NegativeShapeBakedModel 包装所有 INegativeShapeBlock 模型,逐面只输出"边缘四边形"并按相邻方块覆盖做切片
  • Sodium / Embeddium 的 BlockOcclusionCache.shouldDrawSide 各加一个 compat mixin(行为与 skipRendering 一致)

🔴 关键

  1. BlockStateBaseMixin 注册在公共 mixins 列表,但引用了纯客户端类(anvilcraft.mixins.json:13;BlockStateBaseMixin.java 调用 NegativeShapeModelEventListener.shouldSkipFace,而后者 @EventBusSubscriber(value = Dist.CLIENT) 且 import net.minecraft.client.*)。专用服务器上 BlockStateBase 必然在启动早期加载 → mixin 会被应用、mixin 类会被加载;一旦 JVM 校验/链接期解析到该客户端类(或未来任何服务端路径调用 skipRendering),就是 NoClassDefFoundError 崩溃。同一 PR 里其他新 mixin 都正确放在 client 段,唯独这个漏了。请移入 client 列表。

  2. 包装器把"非边缘四边形"从逐面渲染中剥离,全部退化为 general pass(side == null)渲染——这是本 PR 最大的设计风险:

    • 约 15 个负形方块完全不达标cut_ember/frost_metal_slab/pillar/stairsember/frost_anvil/grindstone/smithing_tabletranscendence_* 等模型的面坐标是 0.5/3.5/7.5/8.5/12.5/15.5,没有任何面在 0.025/15.975collectEdgeRectangles 为空 → shouldSkipFace 恒 false,负形剔除对它们静默失效;同时它们的全部面都被逐面 pass 丢弃,整个模型只靠 general pass 存活。
    • vanilla 与 Sodium/Embeddium 行为分叉:vanilla ModelBlockRenderer.tesselateWithoutAO 的 general pass 无条件渲染(不参与逐面剔除、且丢失逐面方向着色 → ember 等带 shade 的装饰面会变平光);Sodium 1.21.1 的 FRAPI 适配器按 quad facing 逐面剔除(表现更正确)。同一模型在两个渲染器下接缝/着色表现不一致。
    • 0.025/15.975 是硬编码耦合MIN_MODEL_EDGE/MAX_MODEL_EDGE 直接写死在分类逻辑里,未来任何面坐标不同的负形模型都会同时失去"剔除"和"逐面渲染"。

    建议:要么在 bake 时预分类每个 side 的完整四边形列表(非边缘四边形保留在对应 side 且参与剔除),要么明确声明"仅框架类模型受支持"并对不达标模型跳过包装,避免静默退化。

⚠️ 警告

  1. 物品栏图标丢失框架getQuads(state, null, ...)withoutEdgeQuads() 把边缘四边形(outline 框架)从 general pass 里剥掉,而物品渲染恰恰走 side == null → 物品模型只剩装饰面、没有 0.025/15.975 的轮廓,与世界中外观不一致(NegativeShapeModelEventListener.java 189-203 行)。

  2. 性能:每次 getQuads(state, side, ...) 都重新 originalModel.getQuads(state, null, ...) 全量取面 + 逐 quad 调 getEdgeDirection() 分类(6 个方向 × 每方块 × 每次区块重建)——chamber 模型约 60+ 元素/300+ quad,属热路径。可在烘焙时按 side 预分类缓存。

  3. javax.annotation.Nullable:1.21 分支已以 org.jspecify 为主(多处使用),javax.annotation 全库仅 1 处旧用法;新 564 行文件又引入一处,与仓库 AGENTS.md 的 JSpecify 约定冲突。

💡 建议

  1. anvilcraft.mixins.json 整个 mixins 数组从 2 空格重排为 6 空格——89 行纯格式 churn,建议还原为最小 diff(仅新增 3 行)。
  2. 相邻负形方块的框架面会被两侧同时剔除,接缝处留下 0.05 单位的透明发丝缝(0.025/15.975 约定固有),属预期但值得在 PR 描述中注明。
  3. BlockMixincanOcclude() 处对负形方块无条件 setReturnValue(true),意味着负形方块的面永远不被 vanilla 遮挡剔除——旧代码对非负形相邻方块也是同样结果,语义等价,但建议加注释说明。

🟢 看起来不错

  • 矩形剔除数学(FaceRectangle.subtract / intersection / rebuildQuad 双线性插值 UV、逐通道颜色插值、离散值 pick)实现扎实,部分覆盖时切片正确,getCornerVertices/rebuildQuad 的 null 回退安全。
  • AnvilCraftMixinPlugin 新增 hasSodium/hasEmbeddium 门控(.compat.Sodium/.compat.Emb),此前 Sodium/Emb chunk builder mixin 无门控,属净改进;BlockOcclusionCacheshouldDrawSide(selfState, view, pos, facing) 签名与 Sodium 1.21.1 实际源码匹配。
  • 删除了 OCCLUSION_CACHE ThreadLocal 手工操作,实现显著简化。
  • transcendium_block_outline.png(含 mcmeta)已存在于仓库;EDGE_RECTANGLES 静态表在每次模型烘焙时清空重建,生命周期正确。

📋 声称验证表

声称 状态 对应实现
重新实现负形剔除 BlockStateBaseMixin + shouldSkipFace + NegativeShapeBakedModel
Sodium/Embeddium 兼容 Sodium/EmbBlockOcclusionCacheMixin + 插件门控
ConfinementChamberBlock 接入 ITranscendiumBlock ConfinementChamberBlock.java + 模型换 #1 轮廓贴图(8 个 inverted cube,48 处)
框架合并渲染(相邻方块剔除共享面) 仅对 0.025/15.975 面生效(chamber/ember/frost 等全方块模型)

结论: REQUEST_CHANGES — 问题 1(mixin 公共列表引用客户端类)必须修复,一行改动(移入 client 段)即可;问题 2 需要设计决策:目前约半数负形方块(slab/stair/anvil 系列)的剔除静默失效且依赖 general pass,至少应在文档/注释中明确支持范围,或按建议 2 的方案改造为非边缘四边形保留在逐面 pass。

🧪 测试建议

被测目标 推荐测试场景 优先级
NegativeShapeModelEventListener.shouldSkipFace 两个 chamber 相邻(完全覆盖→剔除)、部分覆盖(角对边)、不同类型负形方块相邻 🔴
NegativeShapeBakedModel.getQuads side==null(物品渲染)vs 逐面调用:非边缘四边形是否存活、物品图标是否含框架 🔴
FaceRectangle.subtract 矩形完全包含/部分相交/不相交/退化(0.025 厚度) 🟡
rebuildQuad 插值 含 tint 的四边形切片后颜色/UV 保持、非轴对齐 quad 的 null 回退 🟡
专用服务器启动 无客户端类环境下加载 BlockStateBaseMixin 是否抛 LinkageError 🔴
Sodium/Embeddium 实测 相邻 chamber/ember 块接缝表现、slab/anvil 是否可见且着色正常 🟡

由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown
Collaborator

💾 Self-improvement review: Patched references/baked-model-wrapper-quad-filter-review.md in skill 'minecraft-mod-pr-review' (1 replacement). · Patched SKILL.md in skill 'minecraft-mod-pr-review' (1 replacement).

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /opt/data/workspace/AnvilCraft && git show pr4329-head:src/main/resources/anvilcraft.mixins.json | python3 -c "import json,sys; d=json.load(sys.stdin); print('keys:', list(d.keys())); print('mixins...

Reason: script execution via -e/-c flag

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

Analysis complete. I've verified everything against both the PR branch head and the target branch. Here's the review:


代码审查摘要 — PR #4329

操作: synchronize
范围: 9 个文件(4 新增 Java、3 修改 Java、1 mixins.json、1 模型 JSON)/ 1215 行 diff
分支: cull/1.21/1.6 → dev/1.21/1.6(MC 1.21.1 / NeoForge 21.x)

🔴 关键

  • anvilcraft.mixins.json — 新增的 compat mixins 从未注册,是死代码
    本 PR 新增了 mixin/compat/EmbBlockOcclusionCacheMixin.javaSodiumBlockOcclusionCacheMixin.java,但 mixins 配置中没有任何条目指向它们client 数组只删除了旧的 compat.EmbChunkBuilderMeshingTaskMixin / compat.SodiumChunkBuilderMeshingTaskMixin,没有补入新条目;AnvilCraftMixinPlugin.getMixins() 返回 null,不存在动态注册。已在 PR 分支头验证:grep -n "Emb\|Sodium\|Occlusion" anvilcraft.mixins.json 仅命中 BlockStateBaseMixin
    后果:Embeddium/Sodium 下的负形剔除钩子(新架构对 BlockOcclusionCache.shouldDrawSide 的拦截)完全不会生效,且旧的 MeshingTask 钩子也被移除了——Sodium/Embeddium 环境相比 PR 前反而退化;AnvilCraftMixinPlugin 新增的 hasSodium/hasEmbeddium 判断因无匹配 mixin 名而成为死代码;两个旧 *MeshingTaskMixin 文件也仍留在仓库中(孤儿文件)。
    修复:在 client 数组加入 compat.EmbBlockOcclusionCacheMixincompat.SodiumBlockOcclusionCacheMixin,并删除两个旧的 MeshingTask 文件。

⚠️ 警告

  • NegativeShapeBakedModel.getQuads(state, side) 从不委托 originalModel.getQuads(state, side, …) — 只取 null(无 cullface)列表再按坐标筛边。任何带 cullface 的四边形只会出现在原模型的方向列表中,会被静默丢弃。当前 5 个 INegativeShapeBlock 方块(confinement_chamber、transcendium_block、transcendence_anvil/grindstone/smithing_table)的模型均无 cullface(已逐一验证),所以目前安全;但这是一个无任何强制手段的脆弱不变量——未来任何带 cullface 的负形方块模型会静默缺面。
  • confinement_chamber.jsonlight_emission: 15 在 1.21.1 不会被解析 — 已核对 vanilla 1.21.1 的 BlockElement/BlockModel 反序列化器及 NeoForge 21.x 的 BlockModel/BlockElement/SectionCompiler patch,均无该字段处理;约束仓的实际发光来自方块属性(confinedAnvilon.lightLevel(15).emissiveRendering(),PR 前已有)。模型字段与其它超限方块一致,无害但纯装饰。另外该模型仍缺少其它超限方块模型全部携带的 "format_version": "1.21.11"(ember_metal_block、transcendium_block、transcendence_* 等均有)——若要在新模型格式下真正"同步"(无 format_version 时新格式特性不生效),建议补上。
  • 覆盖面的非边缘四边形仍会渲染 — 无 cullface 的四边形走 null 列表,整面跳过(shouldSkipFace)只影响方向列表的边缘四边形;相邻负形方块之间的中间面存在额外过绘制。且在 Sodium/Embeddium 下(因上述问题 1)整套剔除均不生效。性能影响轻微,但值得知晓。

💡 建议

  • NegativeShapeModelEventListener.javarebuildQuad 切片后的四边形保留原 quad.getDirection(),却被 wrapper 在相反 side 下发射(z=0.025 的四边形声明方向为 SOUTH,却作为 NORTH 面输出),环境光遮蔽方向可能有细微偏差;对 0.025/15.975 的内侧小面几乎不可见,可考虑按 side 修正。
  • 数据生成/测试 — 建议断言所有 INegativeShapeBlock 模型不含 cullface,固化上述不变量。
  • EDGE_RECTANGLES — 静态 HashMap 在 bake 事件清空重建、渲染线程只读,当前安全;若未来出现异步烘焙需注意线程安全。

🟢 看起来不错

  • 新架构三层协同清晰:skipRendering(HEAD 注入) + shouldRenderFace 强制渲染 + ModelData 驱动的四边形切片;旧的 OCCLUSION_CACHE 篡改被移除,vanilla 干扰面更小。
  • 矩形减法/切片/重建数学严谨(subtract 四向切分、交点容差、UV/颜色双线性插值、离散元素按角点取值),异常四边形均有容错回退。
  • 方向映射一致:shouldSkipFacegetModelData 均用 face.getOpposite() 取邻块相对面,坐标空间对齐正确;RenderChunkRegion 越界返回空气,不会在渲染线程触发区块加载。
  • 模型纹理改动与描述一致(角落元素换用 transcendium_block_outline)。

📋 声称验证表

声称 状态 对应文件
重新实现负形剔除 NegativeShapeModelEventListener.java、BlockStateBaseMixin.java、BlockMixin.java
约束仓角落超限纹理与其它超限方块同步 ⚠️ 部分 confinement_chamber.json(纹理已换 #1 + light_emission,但缺 format_version;且为 8 角元素 + UV[0,4,4,0],与其它方块的单 outline 立方体 + UV[0,0,16,16]/rotation 180 结构不同,视觉效果需实际验证)
Sodium/Embeddium 兼容 EmbBlockOcclusionCacheMixin.java、SodiumBlockOcclusionCacheMixin.java — 未在 mixins.json 注册,死代码

结论: REQUEST_CHANGES — 核心剔除算法本身设计合理、数学正确,但新增的 Sodium/Embeddium compat mixins 未注册进 anvilcraft.mixins.json,导致兼容路径完全失效并留下死代码(含插件判断与孤儿文件),需修复后再合入。

🧪 测试建议

被测目标 推荐测试场景 优先级
shouldSkipFace / sliceQuad 相邻负形方块全覆盖/部分覆盖/错位覆盖的剔除结果;与 ITranscendiumBlock 族(transcendium_block 全脸矩形 vs 约束仓角矩形)的混合覆盖 🔴
NegativeShapeBakedModel.getQuads 带 cullface 的模型是否缺面(当前无用例,建议加回归测试);item 渲染(null + 六方向)是否完整 🟡
Embeddium/Sodium 环境 注册 mixins 后两环境下相邻方块面剔除的 in-game 截图对比 🔴

由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown
Collaborator

💾 Self-improvement review: Patched SKILL.md in skill 'minecraft-mod-pr-review' (1 replacement). · Patched references/baked-model-wrapper-quad-filter-review.md in skill 'minecraft-mod-pr-review' (1 replacement).

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

curl -sL "https://linkie.shedaniel.dev/api/source?namespace=mojang&version=1.21.6&class=ModelBakery" 2>/dev/null | head -c 300; echo; echo "=== 备用: maven 上的官方 sources? ==="; curl -sL "https://linkie.s...

Reason: Security scan — [MEDIUM] Lookalike TLD detected: Domain uses '.dev' TLD which can be confused with file extensions; [MEDIUM] Lookalike TLD detected: Domain uses '.dev' TLD which can be confused with file extensions

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

All verification is complete. Here is the review.


代码审查摘要 — PR #4329

操作: synchronize
范围: 20 个文件 (14 Java, 5 模型 JSON, 1 mixin 配置) / 1838 行 diff
分支: cull/1.21/1.6 → dev/1.21/1.6

实现概述

负形剔除被重写为「模型烘焙包装 + 边矩形裁剪」方案:NegativeShapeModelEventListenerModelEvent.ModifyBakingResult 时收集所有 INegativeShapeBlock 方块模型位于 0.025/15.975 边界平面上的四边形("edge rectangles"),用 NegativeShapeBakedModel 包装模型;渲染时根据相邻同类型方块的边矩形对边界四边形做减法裁剪;BlockStateBaseMixinskipRendering + Sodium/Embeddium 兼容 Mixin 负责整面跳过;BlockMixin 简化为负形方块的面永远渲染。模型侧:约束仓/约束铁砧的角落框改用共享 outline 纹理 + 发光,negative_matter_block 移除 6 个面的 cullface。

已交叉验证(目标分支 dev/1.21/1.6 + Sodium 1.21.6 源码):接口层级(ITranscendiumBlock/IEmberBlock/IFrostBlock/INegativeMatterBlock 均继承 INegativeShapeBlock)、ShapeUtil.merge 存在、Sodium/Embeddium 的 shouldDrawSide实例方法(兼容 Mixin 的实例 handler 正确,非 static 不匹配问题)、confined_energy/mass/space_anvilon 通过 parent 链自动继承新增的 outline 纹理。

🔴 关键问题

  1. 三个装饰描边方块会渲染成完全透明(不可见)

    • EmberDecoOutlineBlock / FrostDecoOutlineBlock / TranscendenceDecoOutlineBlock 的模型(ember/frost/transcendence_deco_outline.json)是单个倒置立方体(16→0),6 个面全部带 cullface(指向相反方向)。而 NegativeShapeBakedModel.getQuads 只读取 originalModel.getQuads(state, null, ...)(即未剔除桶);带 cullface 的面在烘焙时被归入各方向桶,包装器侧向与 null 两个通道都拿不到它们 → 三个方块的所有面都不渲染。
    • 佐证:本 PR 特意给 negative_matter_block.json 移除 cullface,正是为了让其面落入 null 桶使包装器可见——三个 deco 模型漏掉了同样的处理。修复:与 negative_matter 一致移除这 6 个 cullface("ambientocclusion": false 下视觉等价),或在包装器侧向通道改为按 side 取原始面 + 兼容两种桶。
  2. BlockStateBaseMixin 注册在通用 mixins 列表却引用 Dist.CLIENT 专用类

    • 该 Mixin 的注入 handler 调用 NegativeShapeModelEventListener.shouldSkipFace,而后者(@EventBusSubscriber(value = Dist.CLIENT))的静态初始化与方法签名引用 net.minecraft.client.renderer.*net.neoforged.neoforge.client.model.data.ModelProperty 等专用服务端 jar 中不存在的类。专用服务器加载 BlockStateBase(极早期的通用类)应用该 Mixin 时,JVM 验证/解析 handler 字节码会触发加载监听器类 → 大概率在启动阶段 NoClassDefFoundError 崩溃(即使惰性解析,skipRendering 一旦被调用也会炸)。
    • 修复:把 BlockStateBaseMixin 移入 anvilcraft.mixins.json"client" 列表(与两个 compat Mixin 同列),该功能本就纯客户端。

⚠️ 警告

  1. EDGE_RECTANGLES 静态 HashMap 存在并发/重载竞态 — 烘焙事件(主线程)中 clear() + 重填,区块重建工作线程在 getModelData/getQuads/shouldSkipFace 中并发读。资源热重载(F3+T、模组重载、着色器重载)期间重建线程可能读到半清空状态 → ConcurrentModificationException 或短暂漏剔除。建议烘焙时构建新 map 并通过 volatile 引用整体发布,或直接使用 ConcurrentHashMap

  2. 包装器对模型约束脆弱:负形方块模型中任何带 cullface 的面都会消失 — 当前实现依赖"所有面都在 null 桶"这一前提。除本次三个 deco 方块外,若未来负形方块复用了 vanilla slab/stair/pillar 等带 cullface 的模型(如 EmberMetalSlabBlock 若引用 vanilla slab 模型),同样会整块不可见。建议在 collectEdgeRectangles/包装时对"找不到任何面"的模型打日志或断言,并在接口文档注明约束。

  3. NeutronIrradiatorBlock@Getter @Setter checkBlockState 是死代码INegativeMatterBlock 并未声明 getCheckBlockState/setCheckBlockState(只有 IEmberBlock 声明),全仓库无任何调用点,生成的访问器无人使用,疑似从 IEmberBlock 模式复制残留。

  4. 剔除只覆盖 0.025/15.975 的框架面confinement_chamber.json 中还有大量恰在 0/16 的壳体条(如 [12,7,0]→[16,9,4]),它们不属于 edge 面,始终经 null 通道渲染;两个相邻约束仓的共享 0/16 面仍会 z-fighting(该问题 PR 前已存在,非回归,但负形剔除未覆盖到它们,若要彻底消除接缝需模型统一到 0.025/15.975)。

💡 建议

  • anvilcraft.mixins.json 整个 mixins 数组从 2 空格重排为 6 空格(90 行无关 churn),建议单独提交或还原,减小 diff 噪音。
  • sliceQuad 每次可将一个面拆成最多 4N 个小面,大片同类型负形方块邻接时注意顶点膨胀;当前规模可接受。
  • getModelData 每方块 6 次 getBlockState 邻接查询,可只查 6 个方向中已有覆盖矩形的方向(微优化)。

🟢 看起来不错

  • 矩形减法/切片数学正确(subtract 四分法、rebuildQuad 双线性插值 UV/颜色、离散属性取角点值,stride 假设与 NeoForge 1.21 一致)。
  • shouldSkipFace 与两个 compat Mixin 的语义(同类型 → 按覆盖判定;非同类 → 强制绘制)与 BlockMixin 的"负形面永远渲染"一致。
  • Mixin 插件按 modid 门控 Sodium/Embeddium 兼容 Mixin 正确;负形方块实现接口与模型改动闭环一致(约束仓/约束铁砧类 ↔ 对应模型)。
  • 负形剔除由旧的遮挡缓存(OCCLUSION_CACHE 影子字段)改为烘焙期纯客户端方案,删除了服务端无用的缓存路径。

📋 声称验证表

声称 状态 对应文件
重新实现负形剔除 NegativeShapeModelEventListener, BlockStateBaseMixin, BlockMixin, Sodium/Emb 兼容 Mixin
约束仓角落超限纹理与其它超限方块同步 confinement_chamber / confined_neutronium_ingot / confined_time_anvilon 模型(energy/mass/space 经 parent 继承)

🧪 测试建议

被测目标 推荐测试场景 优先级
三个 DecoOutline 方块 放置后肉眼/截图验证可见性(预期当前不可见) 🔴
专用服务器 启动 + 加载区块验证不崩溃(BlockStateBaseMixin 移至 client 前) 🔴
NegativeShapeModelEventListener F3+T 热重载期间快速移动镜头(EDGE_RECTANGLES 竞态) 🟡
相邻负形方块 同类型 2×2 邻接时框架面消失、异类型邻接时框架完整、与实体方块邻接时面仍渲染 🟡
方块物品形式 物品栏渲染不丢框架面(null 通道 + 侧向通道合并正确) 🟢

结论: REQUEST_CHANGES — 核心算法设计合理且验证充分,但三个装饰描边方块会整体不可见(问题 1),且通用列表 Mixin 引用客户端类有专用服务器崩溃风险(问题 2),均需修复后再合并。


由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown
Collaborator

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

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /tmp && curl -sL "https://maven.neoforged.net/releases/net/neoforged/neoforge/21.1.100/neoforge-21.1.100-sources.jar" -o nf-sources.jar && ls -la nf-sources.jar && unzip -l nf-sources.jar 2>/dev/nu...

Reason: Security scan — [MEDIUM] Archive extraction to sensitive path: Archive command 'unzip' extracts to a potentially sensitive location

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /opt/data/workspace/AnvilCraft && git grep -rn "skipRendering\|shouldRenderFace" FETCH_HEAD -- 'src/main/java' | grep -v "mixin\|INegativeShapeBlock\|IEmberBlock\|IFrostBlock" | head -10; echo "===...

Reason: script execution via -e/-c flag

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

分析完成。所有关键点已交叉验证:接口层级、模型几何(0.5 inset vs 0.025/15.975)、原版渲染器双通道、mixin 分层约定、资源存在性。以下是审查意见:


代码审查摘要 — PR #4329

操作: synchronize
范围: 15 个文件 (6 Java, 4 新增) / 1701 行 diff

主题: 重新实现负形剔除——废弃原 Block.shouldRenderFace 中的形状剔除 + 模型 cullface 方案,改为「烘焙时收集边缘矩形 → 运行时按邻块矩形覆盖剔除/切片」的新机制。

🔴 关键

  • mixin/BlockStateBaseMixin.java 挂在主 mixins 列表却依赖纯客户端类 — 该 mixin 被注册在 anvilcraft.mixins.json"mixins" 数组(主列表,双端加载),但 import 了 dev.dubhe.anvilcraft.client.event.NegativeShapeModelEventListener(其内部引用 net.minecraft.client.renderer.block.model.BakedQuad/BakedModel/ModelData 等)。仓库既有约定严格:所有含 client import 的 mixin 都在 "client" 数组(AnvilBlockMixin、BlockMixin 等主列表 mixin 均无 client import)。dedicated server 类路径不含 net.minecraft.client.* → 服务器端一旦调用 BlockStateBase.skipRendering 且命中负形分支即 NoClassDefFoundError。虽然原版服务器端目前很少调用 skipRendering(主要渲染路径用),这是潜伏崩溃 + 分层违规。修复:把 BlockStateBaseMixin 移入 "client" 列表(skipRendering 本就是渲染用途,无服务器端语义)。

  • BlockStateBaseMixin 对无边界矩形的方块族绕过原版同方块剔除ember_metal_block/frost_metal_block/transcendium_block/neutron_irradiator 的模型面都在 0.5/15.5(0.5 inset),不在 0.025/15.975 → collectEdgeRectangles 为空 → shouldSkipFace 恒 false。而 mixin 在 HEAD 拦截并 setReturnValue(false)短路了原版 state.is(adjacentState.getBlock()) 的同方块剔除——两个相邻 EmberMetalBlock 的共享面不再剔除(不透明方块内部隐藏面全量渲染,overdraw 翻倍)。且 BlockMixin 现在对全 INegativeShapeBlock 族强制返回 true(含 0.5 inset 方块),叠加后 ember/frost/transcendium 方块的所有面都无条件渲染。修复建议:mixin 中当双方 getEdgeRectangles 均为空时 fall through 到原版逻辑(或至少先保留 state.getBlock() == adjacentState.getBlock() 的同方块检查)。

⚠️ 警告

  • NeutronIrradiatorBlock 新增 @Getter @Setter private BlockState checkBlockState; 是死代码 — 仓库中 getCheckBlockState()/setCheckBlockState() 只被 IEmberBlock 接口使用(破坏粒子 levelEvent(2001...)),辐照器并不实现 IEmberBlock,全 diff 无调用者。疑似从 EmberMetalBlock 模式复制的残留,建议删除或接入实际用途。
  • javax.annotation.Nullable 违反仓库 nullness 规范NegativeShapeModelEventListener 用了 javax.annotation.Nullable,而 AGENTS.md 明确规定只允许 JSpecify(org.jspecify.annotations),仓库其他代码均遵守。建议替换。
  • 不同负形家族交界处无剔除negative_matter_block(全帧面 16×16)vs confinement_chamber(角块矩形)等跨 getBlockType() 家族相邻时,双方边界帧面都渲染(0.05 间距不 z-fight,但形成视觉双线)。旧 cullface 方案两边都剔(产生缺口——正是本 PR 修的 bug),新方案同族内正确、跨族遗留双线。若为设计取舍请确认,否则建议在 shouldSkipFace 中跨族比较。
  • AnvilCraftMixinPlugincontains("Emb") 匹配过宽 — 会命中任何含 "Emb" 的未来 mixin,建议改为精确类名(如 "EmbBlockOcclusionCache")。

💡 建议

  • 边缘检测约定依赖模型面恰在 0.025/15.975 — 本次 neutron_irradiator(面在 0.5/15.9/4.1 等)即因此从未激活剔除(其 Java 碰撞形状倒是 0-16)。建议文档化该约定,或 bake 时对 rectangles.isEmpty() 的方块跳过包装(顺带解决 7 次/方块的无效过滤开销——每次 getQuads 都重新全量取 quads + 分配新列表,对全族 ~20 个方块类生效)。
  • light_emission: 15 只加在 confinement_chamber/confined_time_anvilon 角块,confined_neutronium_ingot 未加 — 确认是否有意(neutronium 不发光的差异)。

🟢 看起来不错

  • 核心机制验证通过:原版 ModelBlockRenderer 确实有 null-direction 通道(AO 路径 renderModelFaceAO、flat 路径 renderModelFaceFlat)——withoutEdgeQuads(内部面走 null 通道)+ edgeQuads(边界面走方向通道)的拆分设计成立,且内部面光照正确。
  • 矩形减法几何分区正确subtract 产出四条边带(左/右/底/顶)无重叠无遗漏;UV 用四角双线性插值,对模型 "rotation": 180 的旋转 UV 依然正确(UV 场线性,与旋转无关)。
  • 同族全覆盖剔除语义正确:negative_matter 帧 vs 帧 → 完全覆盖 → 单线连续(正是修复的旧缺口 bug);部分覆盖 → 不整面剔除 + 按矩形切片,两机制互补。
  • 健壮性处理到位getCornerVertices 对非矩形/退化 quad 返回 null 走保守回退;bake 时幂等(instanceof NegativeShapeBakedModel 防重复包装);EDGE_RECTANGLES 静态表 bake 时一次性构建、不可变拷贝。
  • Sodium/Embeddium 兼容 mixin 有插件门控hasSodium/hasEmbeddium),与既有 EmbChunkBuilderMeshingTaskMixin 门控风格一致;BlockMixin 简化后从 50 行缓存逻辑收敛为 8 行,逻辑更清晰。
  • transcendium_block_outline.png 资源已存在 ✓;ShapeUtil.merge(AABB...) 为既有 API(AccelerationRingBlock 同款用法)✓。

📋 声称验证表

声称 状态 对应文件
重新实现负形剔除 NegativeShapeModelEventListener(564 行新)、BlockStateBaseMixin、BlockMixin 简化、Sodium/EmbBlockOcclusionCacheMixin
约束仓角落超限纹理同步 confinement_chamber.json / confined_neutronium_ingot.json / confined_time_anvilon.json 角块面 #0→#1

🧪 测试建议

被测目标 推荐测试场景 优先级
shouldSkipFace 全覆盖/部分覆盖/无覆盖/跨族相邻四象限;同族不同 state(POWERED) 🔴
NegativeShapeBakedModel.getQuads rotation:180 UV 切片后纹理采样正确性(角块视觉回归) 🔴
服务端启动 无 client 类环境下加载含 BlockStateBaseMixin 的 jar 不崩溃(移入 client 列表后验证) 🔴
getEdgeDirection 0.025/15.975 边界检测对 0.5-inset 模型返回 null(不误判) 🟡

结论: REQUEST_CHANGES — 机制本身设计扎实且正确,但需先修复两个问题:① BlockStateBaseMixin 移入 client 列表(服务器类加载风险 + 分层约定违反);② skipRendering 拦截不应绕过原版同方块剔除(影响全部 ember/frost/transcendium 方块族的隐藏面 overdraw)。其余为建议项。


由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown
Collaborator

💾 Self-improvement review: Patched references/baked-model-wrapper-quad-filter-review.md in skill 'minecraft-mod-pr-review' (1 replacement). · Patched SKILL.md in skill 'minecraft-mod-pr-review' (1 replacement).

@Gugle2308

Copy link
Copy Markdown
Collaborator

分析完成。所有关键路径已验证:模型几何、接口族、mixin 门控、quad 分类机制。现在输出审查意见。

代码审查摘要 — PR #4329

操作: synchronize
范围: 15 个文件 (11 Java, 4 模型 JSON) / 1695 行 diff
主题: 重新实现负形剔除(shape 级 → quad 级切片),并同步约束仓角落纹理

机制理解(先确认我读对了)

新机制把负形方块的渲染拆成两条路径:

  • null passgetQuads(state, null)):body quads(0.5/15.5 平面等非边缘)无条件渲染,withoutEdgeQuads 剔除边缘 quad;
  • per-face passgetQuads(state, side)):只输出 edge quads(0.025/15.975 边界平面),由 shouldRenderFace/skipRendering 门控 + 相邻同族方块的 edge rectangles 做 subtract 切片。

BlockMixin 对负形方块强制 shouldRenderFace=trueBlockStateBaseMixin.skipRendering 做全覆盖剔除,Sodium/Embeddium 各有一份对应 mixin。整体自洽。关键验证结论:

  • 所有负形方块模型(transcendium/ember/frost/overheated/confined_*/infinite_collector/neutron_irradiator)均无 cullface 且使用 0.025/15.975 边缘坐标 → 与机制一致 ✅
  • negative_matter_block.json 移除 cullface 是必要且正确的:带 cullface 的 quad 烘焙进方向列表,而 wrapper 只从 null 列表取 quad,不移除会整个消失 ✅
  • NeutronIrradiatorBlockShapes.orShapeUtil.merge(AABB...) 行为等价(均为 union)✅

⚠️ 警告

  1. EDGE_RECTANGLES 静态 HashMap 并发读写NegativeShapeModelEventListener.java
    写入发生在模型烘焙(资源重载线程),读取发生在区块构建线程(vanilla 多线程 + Sodium/Embeddium 异步线程)。onModelBake 里先 clear() 再重新 put,重载期间并发读会触发 ConcurrentModificationException(客户端崩溃)或读到半填充状态(剔除失效 → 纹理闪烁)。建议:构建新 map 后原子替换 volatile 引用,或 ConcurrentHashMap 且烘焙期间只构建不原地 mutate。

  2. confined_neutronium_ingot.json 角元素缺 light_emission: 15 — 与 PR 目标"与其它超限方块同步"矛盾
    本 PR 给 confinement_chamber.jsonconfined_time_anvilon.json 的 8 个角元素都加了 "light_emission": 15(与 transcendium_block.json 的 cube_outline 一致),但 confined_neutronium_ingot.json 的角元素只改了纹理 #0→#1,没有加 light_emission。结果:锭的角落框架按环境光照渲染(偏暗),其它方块角落全亮 — 视觉不一致。

  3. sliceQuad/rebuildQuad 失败回退为完整 quadrebuildQuad 对顶点不在矩形角的 quad 返回 null,此时整块未切片 quad 被加入(slicedQuad == null ? quad : slicedQuad)→ 被覆盖区域仍渲染。当前模型全部轴对齐,低风险;未来引入非轴对齐模型时需注意。

💡 建议

  • getQuads 每次全量重取+重过滤:wrapper 每次调用都 originalModel.getQuads(state, null, ...) 再遍历过滤。可在 bake 时按方向缓存 edge/non-edge 分类结果,减少 chunk rebuild 时的重复开销(这些是 cutout 透明方块,quad 调用频繁)。
  • writeInterpolatedVertex 冗余插值:position 元素(0,1,2)被插值后又立即被 rebuildQuad 强制覆写(edge/first/second 坐标),0-2 的 interpolateFloat 是纯浪费。
  • Sodium/Embeddium 实测建议:两个 BlockOcclusionCache mixin 的 shouldDrawSide 注入签名需与目标 Sodium/Embeddium 版本匹配(两个 mixin 参数名 pos/selfPos 不一致是小事)。另外建议在有 Sodium 的环境实测——若其 BlockRenderer 与 vanilla 结构不一致(无 null pass),wrapper 的 body quads 会丢失。vanilla 侧已由"这些模型无 cullface 且基线可渲染"反证 null pass 存在,Sodium 侧建议实测确认。

🟢 看起来不错

  • AnvilCraftMixinPlugin 新增 hasSodium/hasEmbeddium 门控,修复了基线问题(原 SodiumChunkBuilderMeshingTaskMixin/EmbChunkBuilderMeshingTaskMixin 无条件应用,无 Sodium/Embeddium 时客户端会因硬引用类加载失败)。
  • 三层剔除逻辑(vanilla skipRendering + shouldRenderFace + Sodium/Emb occlusion)对同族判定和 shouldSkipFace 的使用完全一致。
  • onModelBake 正确处理重复包裹(unwrap 一层 + EDGE_RECTANGLES.clear()),重载幂等。
  • FaceRectangle.subtract 分解(左右上下四条带)正确,sameAs/isEmpty 容差处理严谨。

📋 声称验证表

声称 状态 对应文件
重新实现负形剔除 NegativeShapeModelEventListener (新), BlockMixin 重写, BlockStateBaseMixin (新), Sodium/Emb occlusion mixin (新)
修改约束仓角落超限纹理 confinement_chamber.json 角元素 #0→#1 + light_emission 15
与其它超限方块同步 ⚠️ confined_time_anvilon ✅ / confinement_chamber ✅ / confined_neutronium_ingot 缺 light_emission

结论: COMMENT — 核心机制正确且自洽,无阻塞级问题。建议合并前补上 confined_neutronium_ingot.json 角元素的 light_emission: 15,并考虑 EDGE_RECTANGLES 的并发安全(原子替换 volatile 引用即可,改动很小)。

🧪 测试建议

被测目标 推荐测试场景 优先级
同族相邻角块(chamber 角 + transcendium 墙) 6 个方向各放一组,目视确认共享面无双线/无缺口,角部纹理对齐 🔴
不同族相邻(transcendium + ember) 确认双方框架均正常渲染(不互剔) 🟡
EDGE_RECTANGLES 重载 F3+T 资源重载 + 同时移动视角触发 chunk rebuild,确认无崩溃/闪烁 🟡
模型一致性 扫描全部负形方块模型:无 cullface + 边缘坐标 ∈ {0.025, 15.975}(可用 grep 快速校验) 🟢

由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown
Collaborator

💾 Self-improvement review: Patched references/baked-model-wrapper-quad-filter-review.md in skill 'minecraft-mod-pr-review' (1 replacement). · Patched SKILL.md in skill 'minecraft-mod-pr-review' (1 replacement).

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

curl -sL "https://raw.githubusercontent.com/Anvil-Dev/AnvilCraft/dev/1.21/1.6/src/main/java/dev/dubhe/anvilcraft/block/TranscendiumBlock.java" -o /tmp/TranscendiumBlock.java; cat /tmp/TranscendiumBloc...

Reason: Security scan — [MEDIUM] Dependency manifest change followed by network egress: build.gradle was modified, then a network call to a network destination ran within 60s. A dependency edit that immediately phones out can indicate a poisoned install step.

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /tmp && python3 -c "
import zipfile
z = zipfile.ZipFile('/tmp/userdev.jar')
names = z.namelist()
print(len(names))
for n in names[:30]: print(n)
"

Reason: script execution via -e/-c flag

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

⚠️ Dangerous command requires approval:

cd /opt/data/workspace/AnvilCraft && git show FETCH_HEAD:src/main/resources/assets/anvilcraft/models/block/confined_time_anvilon.json > /tmp/cta.json 2>/dev/null && python3 -c "import json; d=json.loa...

Reason: script execution via -e/-c flag

Reply /approve to execute, /approve session to approve this pattern for the session, /approve always to approve permanently, or /deny to cancel.

@Gugle2308

Copy link
Copy Markdown
Collaborator

The sandbox guard is blocking further local commands, but I now have comprehensive evidence. Let me compile the final review.

Key evidence gathered:

  1. NegativeShapeBakedModel.getQuadsside==nullwithoutEdgeQuads (drops ALL edge quads); side!=nulledgeQuads (drops ALL non-edge quads, keeps only boundary-plane quads of that side)
  2. Model geometry on target branch: confinement_chamber.json 64 elements / 336 of 360 faces non-edge; confined_neutronium_ingot.json 360 of 384 non-edge; neutron_irradiator.json 136 non-edge; infinite_collector.json 49 non-edge; negative_matter_block.json inner cube [0.5,15.5] — all faces non-edge
  3. All 4 newly-tagged blocks + pre-existing TranscendiumBlock/NegativeMatterBlock/Transcendence* blocks get wrapped
  4. ShapeUtil.merge(AABB...) = BooleanOp.OR union — verified equivalent to old Shapes.or
  5. New interfaces exist on target branch ✓

代码审查摘要 — PR #4329

操作: synchronize
范围: 15 个文件 (10 Java, 5 资源模型) / 1711 行 diff
分支: cull/1.21/1.6 → dev/1.21/1.6

变更概览

全新的负形剔除实现,取代旧的 BlockMixin 遮挡缓存 hack:

  • 模型烘焙期包装NegativeShapeModelEventListener.onModelBake 把所有 INegativeShapeBlock 方块的 BakedModel 包成 NegativeShapeBakedModel,收集边界四边形(位于 0.025/15.975 模型坐标平面的 quad)存入静态 EDGE_RECTANGLES
  • 方块级剔除BlockStateBaseMixin.skipRendering 对同类型相邻方块调用 shouldSkipFace(自方边缘矩形被对方完全覆盖则剔除整面)
  • 模型级切片getModelData 收集相邻同类型方块的面覆盖信息,sliceQuad 按覆盖矩形对边界 quad 做减切重建
  • 渲染器兼容:Sodium/Embeddium BlockOcclusionCache mixin + 插件 gating
  • 4 个方块新实现 ITranscendiumBlock/INegativeMatterBlock;约束仓/约束锭/约束时间安维龙角元素改用 transcendium_block_outline 纹理 + light_emission: 15negative_matter_block.json 移除反向 cullface

🔴 关键

1. NegativeShapeBakedModel.getQuads 的 quad 分流逻辑会吞掉大部分模型几何(NegativeShapeModelEventListener.java:247-315)

List<BakedQuad> quads = originalModel.getQuads(state, null, random, modelData, renderType);
if (side == null) return withoutEdgeQuads(quads);   // ← 丢弃全部边界 quad
return edgeQuads(quads, side, ...);                 // ← 只保留"位置在 side 边界平面"的 quad

两条路径返回的是互斥子集

  • side == null(物品栏/GUI):withoutEdgeQuads 删掉所有边界 quad → 发光描边消失
  • side != null(世界内逐面 meshing 的标准契约):edgeQuadsif (getEdgeDirection(quad) != side) continue;所有不在 0.025/15.975 边界平面上的 quad 直接丢弃 → 主体几何全部消失

我核对了目标分支所有受影响方块的模型,非边界面的比例是决定性的:

模型 元素数 非边界面数
confinement_chamber.json 64 336/360
confined_neutronium_ingot.json 68 360/384
neutron_irradiator.json 38 136/165
infinite_collector.json 9 49/54
negative_matter_block.json(内立方体 [0.5,15.5]) 2 6/6 全非边界

世界内逐面查询(vanilla BlockModel.getQuads 的契约是按面方向分组返回 quad,与 quad 在模型中的位置无关)下,约束仓只会渲染 8 个角柱的 24 个边界面,所有连接杆消失;负物质方块只剩发光描边框,主体纹理不可见;中子辐照器/无限收集器同理。物品形态则反过来缺描边。无论渲染器是逐面查询还是 null 单次查询,总有一半几何丢失——正确实现应当是:side == null 返回全部 quad;side != null 返回非边界 quad(按 quad.getDirection() 过滤)+ 该面的边界 quad 切片。这是必须修复的阻塞问题。

2. shouldSkipFace 只覆盖边缘矩形,与非边界面的剔除脱节(NegativeShapeModelEventListener.java:187-200)

即使修复 #1,方块级剔除也只比较边界矩形。同类型相邻方块接触面上的非边界几何(如 0.5 处的横杆面)不在矩形减切范围内,模型级 sliceQuad 也只对边界 quad 生效 → 相邻方块之间的非边界部分无法剔除。需要与 #1 一起重新设计覆盖模型。

⚠️ 警告

3. NeutronIrradiatorBlock 实现 INegativeMatterBlock(NeutronIrradiatorBlock.java:31) — 语义存疑:辐照器与负物质方块被划为同一剔除家族(getBlockType() = INegativeMatterBlock.class),两者相邻时会互相剔除/切片接触面;且其渲染模型并非按 0.025/15.975 边界约定构建,被包装后同样受 #1 影响。若意图是"辐照器中心的负物质显示",建议确认;否则应换用独立类型接口。

4. EDGE_RECTANGLES 静态 HashMap 的线程安全与生命周期 — 在 onModelBake(资源重载线程)写入、chunk 构建工作线程与 skipRendering 中读取,无同步。实践中烘焙先于网格构建、重载会暂停网格化,风险低,但重载窗口期存在读到半更新状态的可能;建议烘焙完成后 Map.copyOf 冻结。

5. BlockMixin 对负形方块无条件 setReturnValue(true)(BlockMixin.java) — 跳过 vanilla 对非遮挡方块的面逻辑,包括朝向普通实心方块时的 getFaceOcclusionShape join 判定。对非遮挡方块而言大体等价,但所有 6 个面都会无条件进入网格化(即使该面 edgeQuads 结果为空),有额外开销。可接受,但值得注意。

6. Sodium/Embeddium mixin 目标方法签名需与目标版本核对shouldDrawSide(BlockState, BlockGetter, BlockPos, Direction) 注入点若与玩家实际安装的 Sodium/Embeddium 版本签名不符会直接崩溃(@Inject 找不到目标 = 硬错误)。建议在 README 或 CI 中声明支持的版本范围。

💡 建议

  • AnvilCraftMixinPluginmixinClassName.contains("Emb") 同时门控了既有的 EmbChunkBuilderMeshingTaskMixin(此前无门控、无条件应用)——这其实是修复,但属于行为变化,确认是预期即可。另注意 "Emb" 子串过于宽泛,未来任何含 "Emb" 的 mixin 都会被门控。
  • collectEdgeRectangles 使用 RandomSource.create(0L):对依赖随机种子的变体模型可能收集到与真实渲染不一致的矩形;当前这些静态模型无影响。
  • 角落元素 UV [0,4,4,0] + 新纹理 在REI渲染方块(初步测试) #1:是 16x16 outline 纹理的 4x4 子区域,而 transcendium_block.json 的描边立方用全幅 [0,0,16,16]——确认视觉上与其他超限方块一致。
  • 3 参 getQuads(state, side, random) 重载ModelData.EMPTY 委托,走该路径的调用方拿不到切片(边界 quad 原样返回);如无必要可保持一致。
  • mixins.json 整体重新缩进:格式噪音,可单独提交以便 diff 聚焦。

🟢 看起来不错

  • 新架构(方块级 skipRendering + 模型级切片 + 渲染器兼容层)比旧的 Object2ByteLinkedOpenHashMap 缓存 hack 清晰得多,方向正确
  • ShapeUtil.merge(AABB...) 重构经 AnvilLib 源码验证(BooleanOp.OR 并集),与旧 Shapes.or 行为等价 ✓
  • 4 个模型 + NeutronIrradiatorBlock 补齐了 EOF 换行符
  • negative_matter_block 移除反向 cullface 后,描边显示不再依赖"对侧实心"这一脆弱条件
  • 新接口 ITranscendiumBlock/INegativeMatterBlock 在目标分支存在且类型层级自洽 ✓

📋 声称验证表

声称 状态 对应文件
重新实现负形剔除 ⚠️ NegativeShapeModelEventListener, BlockStateBaseMixin, BlockMixin(核心机制在,但 quad 分流存在致命缺陷)
约束仓角落超限纹理与其它超限方块同步 confinement_chamber/confined_neutronium_ingot/confined_time_anvilon.json(#0→#1 + light_emission 15)
Sodium/Embeddium 兼容 SodiumBlockOcclusionCacheMixin, EmbBlockOcclusionCacheMixin + 插件门控

🧪 测试建议

被测目标 推荐测试场景 优先级
NegativeShapeBakedModel.getQuads 逐面查询时返回的 quad 集合是否等于原模型该面的全部 quad(非边界不丢失);null 查询是否含全部 quad 🔴
方块相邻剔除 同类型相邻(完全覆盖/部分覆盖/无覆盖三态)的面剔除与切片结果截图对比 🔴
物品形态 vs 世界形态 物品栏渲染(side=null)与世界中渲染的外观一致性 🟡
资源重载 重载后 EDGE_RECTANGLES 重建、包装不叠加(无双重 wrapper) 🟡
Sodium/Embeddium 实机 安装后 shouldDrawSide 注入不崩溃、剔除行为一致 🟡

结论: REQUEST_CHANGES — 核心机制思路可行,但 NegativeShapeBakedModel.getQuads 的 quad 分流使所有被包装方块的世界内渲染丢失主体几何(约束仓只剩角框、负物质方块只剩描边),必须修复后实机验证再合入。


由 Hermes Agent 审查

@Gugle2308

Copy link
Copy Markdown
Collaborator

💾 Self-improvement review: Patched SKILL.md in skill 'minecraft-mod-pr-review' (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.

2 participants