Repository navigation
Conversation
- drawTiledFrames 依赖 Vars.renderer.fluidFrames(仅客户端可用) - 客户端 Vars.renderer 不为 null,不影响正常渲染 - null 时跳过液体动画绘制,避免 NPE
…延迟注册、PlayerLeave 清理声源、URL 首帧坐标与扩展名感知缓存、MusicBar 菜单守卫、内置曲目 key 去重
…5s(误按帧计)、远程声源自然播完清理防voices累积、左右声像pan定位、坐标上报30s→2s、关闭播放器广播stop、清理孤儿.part暂存文件
…置路由瞬态;清理marginLeft死代码与allRegions累积;方块搜索栏放大镜图标与38px控件对齐
- seek 夹取到轨道末尾前 0.5s,杜绝拖到末尾触发自动切曲 - seek 后 1s 静默守卫:流式(URL/外部)声源跳转瞬间不被误判播完致跳歌 - 音量/音高/倍速三面板合为一行,滑杆支持外部值回写同步 - 按钮文字禁止换行(倍速/专辑/曲名/全部曲目),悬浮条倍速按钮加宽至56px - 悬浮条进度条独立一行加高留白,圆钮不再遮挡上方曲名 - 弹窗曲目信息列加宽右对齐,时长未知显示 --:-- 占位符保列对齐 - 顶部「现在播放」面板实时刷新;停止播放后进度复位
…、专辑筛选下导入自动归入、导入界面优化
|
/robin |
🏹 Robin✅ Review done. I flagged 12 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
本次 diff 主要为 UI 重构(UniversalJunctionDialog 拖拽预览与整框换位)、新增启动时弹出的 Changelog、Switch 切换逻辑调整(由可配置菜单改为点按切换)、RollGenerator/PowerSource 电网遍历优化,以及 DualPurposeStorager 液面渲染改为动态绘制。改动整体风格较谨慎,已有空指针守卫与早退逻辑,但基于仅有 diff 的限制,未发现确定的崩溃、多人不同步或数据丢失问题。
Robin — the Robin Hood of code review. Free for every PR.
| group = BlockGroup.logic; | ||
| // 配置同步开关自身状态 fE(联网经 Call.tileConfig 全端一致);front 目标由 updateTile 同帧应用 | ||
| config(Boolean.class, (building, value) -> ((SwitchBuild) building).fE = value); | ||
| config(Boolean.class, (building, enabled) -> { |
There was a problem hiding this comment.
Robin —
移除了 configurable = true 且用 tapped() 替代 configTapped(),同时 buildConfiguration() 仍保留切换逻辑。此行为变更会影响方块是否自动弹出配置菜单、与原有「按钮式配置」预期的交互方式以及外部逻辑(如蓝图/设置)对配置入口的感知。
Recommendation: 确认 tapped() 是否符合预期的点击切换语义(点按即切换且不弹配置面板)。若仍需保留配置菜单入口,需明确 configurable 取值;否则在变更说明或注释中记录交互变更,避免误导使用者。
| // 配置同步开关自身状态 fE(联网经 Call.tileConfig 全端一致);front 目标由 updateTile 同帧应用 | ||
| config(Boolean.class, (building, value) -> ((SwitchBuild) building).fE = value); | ||
| config(Boolean.class, (building, enabled) -> { | ||
| Building front = building.front(); |
There was a problem hiding this comment.
Robin —
配置回调 config(Boolean, ...) 直接将 front.enabled = enabled 写入前方建筑,并仅在 building instanceof SwitchBuild 时才同步 sb.fE。由于回调目标是 SwitchBuild 本身,instanceof 恒为 true;但该写法将「状态设置」强行作用于 front(),要求前方必须是同队且非 SwitchBuild(已做校验)。
Recommendation: 保留现有同队与 !(front instanceof SwitchBuild) 校验,并确保仅在服务器端执行(Mindustry 配置回调在服务器处理)且不与 updateTile() 的「fE 跟随 front.enabled」方向产生循环竞争。当前 updateTile() 将 fE 同步为 front.enabled,配置回调也写入 front 再设 fE,语义一致但需注意外部逻辑直接改动 front 时的时序。
| * 继承 StorageBlock,放置于核心旁可与核心连接并为核心扩容(同原版仓库)。 | ||
| * 同时通过 hasLiquids + dumpLiquid 提供液体存储与导管抽取能力。 | ||
| * 支持 bottomRegion/liquidRegion/topRegion 三层贴图,呈现与原版流体储罐一致的流动+颜色特效。 | ||
| * 支持 bottomRegion/topRegion 两层静态贴图,中间液面用原版 drawTiledFrames 动态绘制(呈现与原版流体储罐一致的流动+颜色特效)。 |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: high
移除了 liquidRegion 字段及加载回退逻辑,改用 drawTiledFrames 动态绘制液面。类注释已更新,但仓库若存在其他引用(旧资源、工具、或预期存在 -liquid 的校验)在 diff 外不可见。
Recommendation: 确认 liquidRegion 未在本类以外使用,且对应贴图(name + "-liquid")不再被依赖。若旧存档/蓝图或其他 mod 逻辑无依赖则可接受。
| if (liquids.currentAmount() > LIQUID_THRESHOLD) { | ||
| // 中间液体层改用原版 drawTiledFrames 动态绘制,不再读取 liquidRegion(-liquid 贴图可移除); | ||
| // 依赖 Vars.renderer.fluidFrames(仅客户端可用),正常渲染期 renderer 已就绪,此处空判为防御性写法。 | ||
| if (liquids.currentAmount() > LIQUID_THRESHOLD && Vars.renderer != null) { |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: medium
新增 if (liquids.currentAmount() > LIQUID_THRESHOLD && Vars.renderer != null) 守卫。Vars.renderer 仅在客户端渲染期可用,空判是防御性的;但 draw() 在渲染线程调用时理论上 renderer 已就绪。
Recommendation: 若该守卫仅为极少数异常场景,可保留;否则可考虑注释原因以避免误以为存在跨线程可见性要求。
|
|
||
| public UniversalJunctionDialog(String title, DialogStyle style) { | ||
| super(title, style); | ||
| setFillParent(true); |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: medium
全屏布局(setFillParent(true))配合复杂的拖拽预览(ghost、root hint、slot 高度实时计算、手动重排)。hide() 中调用 clearDragPreview() 清理状态,setup() 调用 allRegions.clear() 以避免 re-show 残留。
Recommendation: 拖拽期间可能触发场景树重排(invalidateHierarchy、clearChildren、applySlotOrder)。现有做法尽量避免 detach 持有触摸焦点的元素(如 applySlotOrder 重排 cells/children 而非整层 clear),思路合理。建议后续测试多指针、快速拖拽、窗口缩放等边界场景。
| */ | ||
| @Override | ||
| public float getPowerProduction() { | ||
| if (power == null || power.graph == null) return 0f; |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: high
新增 if (power == null || power.graph == null) return 0f,并将遍历改为有界下标 for (int k = 0; k < n; k++) { Building e = power.graph.all.items[k]; ... if (e == null) continue; ... },避免 Seq 尾部空槽导致 NPE。
Recommendation: 按 size 而非 capacity 迭代且判空是正确做法,符合 Mindustry Seq 实现特性。
| MOD = mods.getMod(Silicon.class); | ||
| if (MOD != null) MOD.meta.subtitle = MOD.meta.version; | ||
| }); | ||
| Events.on(EventType.ClientLoadEvent.class, e -> silicon.util.Changelog.checkAndShow()); |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: medium
在 ClientLoadEvent 注册 silicon.util.Changelog.checkAndShow()。启动时必弹日志仅用于调试(类注释说明),但会在每次客户端加载时执行弹窗。
Recommendation: 确认该行为是否为预期(调试期常开)。若需「仅新版本弹出」或记住上次版本,Changelog 内已有目标版本选择逻辑,按现实现每次加载都会尝试弹窗(满足「每次启动必弹」的调试需求)。
| private static final String resultName = "silicon-search-result"; | ||
| private static final String searchRowName = "silicon-block-search"; | ||
| /** 搜索栏控件统一高度(图标/输入框/清除按钮共用,避免数值耦合) */ | ||
| private static final float searchH = 38f; |
There was a problem hiding this comment.
Robin — 💡 SUGGESTION · confidence: high
引入 searchH = 38f 统一图标、输入框、清除/历史按钮高度,消除魔法数值,提升可维护性。
Recommendation: 可考虑将该常量暴露为配置或与主题尺寸保持一致(如现有 UI 尺寸),但当前做法是合理的局部统一。
| Block block = Vars.content.blocks().find(b -> b.name.equals("silicon-" + e.item)); | ||
| if (block == null) block = Vars.content.blocks().find(b -> b.name.endsWith("-" + e.item)); | ||
| return block != null ? block.uiIcon : null; | ||
| }); |
There was a problem hiding this comment.
Robin — 💡 SUGGESTION · confidence: medium
图标查找回退:先 "silicon-" + e.item,再 endsWith("-" + e.item)。后者可能命中同名后缀的其他方块(跨 mod 或重名),存在误匹配风险。
Recommendation: 优先匹配 silicon- 前缀且精确对应,或限制仅搜索本 mod 内容(Vars.content.blocks().find(b -> b.name.startsWith("silicon-") && b.name.endsWith("-"+e.item)))以降低误命中。
| @Override | ||
| public boolean configTapped() { | ||
| // #28 同队校验 | ||
| public void tapped() { |
There was a problem hiding this comment.
Robin — 💡 SUGGESTION · confidence: high
切换逻辑由配置菜单改为直接点按(tapped() 内翻转并 configure(fE)),并注释保留了旧的 configTapped() 与 buildConfiguration 的注释代码。
Recommendation: 移除已注释的旧实现(// configurable = true、// configTapped()、注释的 buildConfiguration 内部按钮逻辑),减少噪声,便于后续维护。
…Tapped/buildConfiguration 旧按钮逻辑),instanceof 恒真处直接强转;Changelog 图标回退限定 silicon- 前缀避免跨 mod 误匹配
处理说明(针对本轮 Robin 审查 05:03)已修复(d3b9ec9):
确认项 / 误报(无需改动):
本地离线构建(--no-daemon --offline)BUILD SUCCESSFUL。 |
|
/robin |
🏹 Robin✅ Review done. I flagged 21 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
🚨 3 High |
Summary
本次改动整体质量较高,完成了 UniversalJunction 对话框重构、Switch 切换逻辑收敛、新增启动时更新日志弹窗、优化发电机电网遍历等。主要风险集中在客户端事件触发时机、网络同步语义(Switch.configure)、Changelog 的版本判定与资源查找以及部分 UI 拖拽预览的可见性/状态清理。存在少量细节问题需处理。
📄 Findings Not Posted Inline
src/silicon/ui/UniversalJunctionDialog.java:166) — 注释掉了直接「保存」按钮(原本同步 weights 并 configure(build.weightsString()) 后 hide)。改为仅「返回」与「编辑」(清空等操作)。需确认关闭对话框时是否自动同步权重。UniversalJunctionDialog 本身未在 hide() 强制 syncWeights,RegionState.syncWeights() 仅在拖拽落点等交互时调用。若用户拖拽后直接点返回(hide),可能未同步最近一次预览状态?查看 hide() 仅清理 dragPreview,不同步。建议在 hide() 前对 allRegions 调用 syncWeights(),确保最新布局写回 build.weights 并 configure。
在 hide() 覆盖中:先 for (RegionState rs: allRegions) rs.syncWeights(); build.configure(build.weightsString()); 然后清理并 super.hide()。避免「拖拽后直接返回」导致配置未落盘。
🔵 2 (src/silicon/Silicon.java:85) — ClientLoadEvent 注册 Changelog.checkAndShow()。事件顺序上 ClientLoadEvent 在客户端资源加载完成后触发,调用 Core.app.post 延后显示对话框是合适的(避免阻塞加载)。但若在单机模式下也会触发(ClientLoadEvent 是客户端事件),符合「每次启动必弹」预期。
保留。
🔵 3 (src/silicon/ui/BlockSearch.java:50) — 提取 private static final float searchH = 38f,并统一用 size(searchH) 替代原先 image.padRight(8f)+field.height(38f)+button.size(38f)。减少魔法数、提升一致性。改动正确。
保留。
🔵 4 (src/silicon/util/Changelog.java:20) — 类注释中「每次启动必弹(供调试方便观察)」说明了当前行为。代码逻辑确实按 target 选择版本组弹窗(可能是当前版本或上次见过版本组),但注释表述与实现略有差异:实现是「选定 ≤ current 的最高版本 target,再弹出全部 version==target 的条目」。若同一 target 有多条,全部弹出。注释简明扼要,问题不大。
可微调注释表述以更贴近实现:例如「弹出 ≤ 当前版本的最高版本对应的全部改动条目」。
🔵 5 (src/silicon/world/blocks/container/DualPurposeStorager.java:51) — 注释更新为「两静态层贴图(中间液体层用原版 drawTiledFrames 过程绘制,无需贴图文件)」。对应移除 liquidRegion 读取是正确的。需注意 mod 资源包中是否仍存在 -liquid 贴图(若其他地方引用会找不到),但本文件已不再使用,注释也明确不再依赖。
保留注释。建议同步检查 assets 中该方块是否还有冗余 -liquid 贴图(非本 diff 范围)。
💡 6 (src/silicon/util/Changelog.java:109) — display 名称回退:Core.bundle.getOrNull("block.silicon-"+e.item+".name") 失败时用 e.item。对于非方块条目(例如未来可能用其他 key)较脆弱,但当前 ENTRIES 全部是方块名,足够。
可考虑也尝试 "item.silicon-..." 或直接用 item 原名的 bundle key。非必须。
💡 7 (src/silicon/ui/UniversalJunctionDialog.java:63) — 新增 SOLID Drawable(继承 BaseDrawable 覆盖 draw 两个重载)。实现简洁,使用 Fill.crect。arc 的 Drawable 通常也可用 PixmapTexture 或 NinePatch,但自绘 Fill.crect 更轻且无纹理依赖,适合纯色按钮底。可接受。
保留。注意 draw(rotation) 分支未旋转填充(当前仅 crect 原点左下矩形),但按钮无需旋转,安全。
💡 8 (src/silicon/ui/UniversalJunctionDialog.java:260) — applySlotOrder() 同时重排 getCells() 和 getChildren()。Table 布局中 cells 与 children 顺序通常一致,但强行维护两者较冗余。当前做法是防御性,避免顺序不一致导致显示错乱。
可仅重排 cells 或确认项目 arc 版本下 children 顺序由 cells 决定,简化实现。非必要。
💡 9 (src/silicon/world/blocks/defense/Switch.java:103) — updateTile 将 fE 同步为 front.enabled(单向)。这与 configure 回调「设置 front.enabled 并回写 fE」形成闭环。若 front.enabled 被外部逻辑频繁改动,fE 会随之变化。语义从「开关记忆」向「状态镜像」转变,注释也写明「状态反映器」。这与现有注释「fE 跟随前方建筑的实际 enabled」一致,改动意图清晰。
建议在类注释或字段注释中进一步说明 fE 的新语义(镜像 vs 控制),避免后续维护混淆。
Robin — the Robin Hood of code review. Free for every PR.
| group = BlockGroup.logic; | ||
| // 配置同步开关自身状态 fE(联网经 Call.tileConfig 全端一致);front 目标由 updateTile 同帧应用 | ||
| config(Boolean.class, (building, value) -> ((SwitchBuild) building).fE = value); | ||
| config(Boolean.class, (building, enabled) -> { |
There was a problem hiding this comment.
Robin — 🚨 HIGH · confidence: high
Switch.configure(Boolean) 直接将 front().enabled = enabled 且不检查 front 是否为 SwitchBuild、是否为同队之外(代码已加同队与非开关校验)以及未限制仅服务器执行。Mindustry 规范要求 configure 仅限客户端->服务器,且服务器端必须按方块语义谨慎修改其他建筑。当前实现在 configure 回调内就修改目标建筑 enabled,若通过意外途径在客户端触发(或未来变更)可能破坏多人游戏一致性。同时 tapped/buildConfiguration 也直接改 fE 并 configure,需确认所有配置路径均走服务器受控。
Recommendation: 保留同队与非 SwitchBuild 校验;优先在服务器端生效(可通过 net.server 判断)或将状态变更限制在 build 为服务器端实体时执行。避免在 configure 回调中无条件直接改写前方建筑状态,必要时改用仅设置自身 fE 并在 updateTile/server 逻辑应用,或显式校验 net.server/net.client。
config(Boolean.class, (building, enabled) -> {
Building front = building.front();
if (front == null || front.team != building.team || front instanceof SwitchBuild) return;
front.enabled = enabled;
((SwitchBuild) building).fE = enabled;
});
| if (front() != null && front().team == team && !(front() instanceof SwitchBuild)) { | ||
| configure(!fE); | ||
| Sounds.click.at(this); | ||
| fE = !fE; |
There was a problem hiding this comment.
Robin — 🚨 HIGH · confidence: high
tapped() 直接改 fE = !fE 并 configure(fE)。由于 configure 回调已设置 front.enabled = enabled 且回写 fE,这种「先改本地再调用 configure」的顺序在网络环境下可能导致本地先行改变与服务器回调不一致,且如果目标建筑在同帧被外部逻辑改动,fE 的「记忆」语义与旧版「点按翻转」不一致。项目铁律要求 configure 仅限客户端->服务器;在客户端 UI 触发时 tapped 是客户端调用,configure 会走 Call.tileConfig 发往服务器,服务器执行 configure 回调后会通过网络同步状态。需确认 fE 是否应仅由服务器端根据 front.enabled 同步(updateTile 已实现),避免客户端在 tapped 时预先翻转 fE。
Recommendation: 不要在 tapped() 中预先 fE = !fE。直接调用 configure(!((SwitchBuild)this).fE)(或取当前期望值)由服务器端根据目标状态计算并回写 fE,或者完全由 updateTile 驱动 fE 同步(单向:fE <- front.enabled)。减少客户端本地乐观改动,降低多人不同步风险。
@Override
public void tapped() {
if (front() != null && front.team == team && !(front() instanceof SwitchBuild)) {
fE = !fE;
configure(fE);
}
}
| } | ||
|
|
||
| @Override | ||
| public void buildConfiguration(Table table) { |
There was a problem hiding this comment.
Robin — 🚨 HIGH · confidence: high
buildConfiguration(Table) 内同样直接翻转 fE 并 configure(fE)。这会在配置表格打开时立即切换开关状态(甚至可能在服务端 buildConfiguration 调用时触发),严重违背「配置界面仅收集参数、不立即应用非用户显式操作」的预期。多人模式下 UI 在客户端构建时也会执行,可能导致意外 configure 调用。
Recommendation: 移除 buildConfiguration 中的状态翻转与 configure 调用。改为仅向表格添加切换按钮,由按钮的点击回调发起 configure(或直接在按钮回调内用 tapped 逻辑)。不要在 buildConfiguration 内修改建筑运行时状态。
@Override
public void buildConfiguration(Table table) {
Building front = front();
if (front != null && front.team == team && !(front instanceof SwitchBuild)) {
fE = !fE;
configure(fE);
}
}
| * 每次启动必弹当前版本的改动内容(供调试方便观察)。 | ||
| * 目标版本 = ≤ 当前版本中的最高版本(ENTRIES 按「旧→新」排列,遍历覆盖即得)。 | ||
| */ | ||
| public static void checkAndShow() { |
There was a problem hiding this comment.
Robin —
checkAndShow() 使用 UpdateChecker.isNewer(e.version, current) 反复比较来选定 target(遍历全部 ENTRIES)。逻辑上「取 ≤ 当前版本的最高版本」需要正确的版本序比较。若 ENTRIES 中版本顺序不是严格递增(旧->新)或 isNewer 实现与预期相反(如 isNewer(a,b)=true 表示 a>b),可能选错 target(例如当前版本高于所有条目时返回 null 是合理,但若顺序错乱会错误匹配)。此外只弹「target == 某个条目」的整组(show.add 全部 version==target),若同一版本多条是预期,但条目顺序未保证也无问题。建议校验 ENTRIES 顺序或直接按版本降序选首个 <= current。
Recommendation: 确保 ENTRIES 按版本升序或降序有序,或改为:过滤 e.version <= current 后取 max by 版本比较,再收集同一 target。也可先找出所有 <= current 的版本并选取最高者,避免依赖遍历覆盖的隐式假设。
String target = null;
for (Entry e : ENTRIES) {
if (!UpdateChecker.isNewer(e.version, current)) target = e.version;
}
if (target == null) return;
|
|
||
| list.table(row -> { | ||
| // 优先取 mod 方块(silicon-<内部名>),再回退任何其后缀匹配的方块;结果按 item 名缓存 | ||
| TextureRegion icon = iconCache.get(e.item, () -> { |
There was a problem hiding this comment.
Robin —
iconCache 使用 ObjectMap.get(e.item, () -> { ... }) 懒加载 TextureRegion。查找逻辑:先找 block.name.equals("silicon-"+e.item),再回退 startsWith("silicon-")+endsWith("-"+e.item)。这在 content 尚未完全加载(Vars.content.blocks())或在极早时序(ClientLoadEvent 触发时)调用时可能返回 null。虽然 checkAndShow 在 ClientLoadEvent 上调度 Core.app.post 延后执行,Vars.content.blocks() 此时通常已就绪,但并非绝对。且若 block.uiIcon 尚未就绪也会缓存 null,导致后续不再重试。
Recommendation: 在 post 中也可二次确认,或在查找失败时不缓存 null(用 computeIfAbsent 替代带默认工厂的 get 行为,或显式 putIfAbsent),避免永久缓存查找失败。必要时增加日志(debug/warn)便于排查缺图标。
TextureRegion icon = iconCache.get(e.item, () -> {
Block block = Vars.content.blocks().find(b -> b.name.equals("silicon-" + e.item));
if (block == null) block = Vars.content.blocks().find(b -> b.name.startsWith("silicon-") && b.name.endsWith("-" + e.item));
return block != null ? block.uiIcon : null;
});
| // 提前 return 前必须归零:currentPowerProduction 是上一帧缓存,getPowerProduction() 无门控直接返回它—— | ||
| // 禁用/PowerVoid 后若不归零,幽灵供电会持续进入电网;该值还会随存档读写。 | ||
| if (!enabled) { | ||
| currentPowerProduction = 0f; |
There was a problem hiding this comment.
Robin —
优化为遍历 power.graph.all 统计同队本类建筑(O(m),m 为电网内建筑数)。这是正确的语义(只统计本队、本图内同类建筑)。但需注意:power.graph 可能在构造/断网时短暂变化;且 power.graph.all 为电网快照(graph 更新时重建)。在 updateTile(物理线程)中读取 graph.all 是安全的(Mindustry 电网更新顺序:Groups.powerGraph.update() -> ... -> updateTile(),graph 在该帧已更新)。此外对 graph.all 的 for-each(数组迭代)避免了先取全队列表再线性查 graph 的双重开销,改动合理。
Recommendation: 保留优化。可考虑用局部变量缓存 graph.all 引用(已用 power.graph.all 直接引用)避免重复属性访问,微优化但非必要。
| */ | ||
| @Override | ||
| public float getPowerProduction() { | ||
| if (power == null || power.graph == null) return 0f; |
There was a problem hiding this comment.
Robin —
新增 if (power == null || power.graph == null) return 0f。PowerSource.build 的 power 字段在某些状态(方块未完全链接、图初始化)可能为空,直接访问 power.graph.all 会 NPE。空判是必要的防御。改动正确。
Recommendation: 保留。
| // 按有效 size 迭代:背板数组 items 的尾部含空槽(容量>size), | ||
| // for-each 裸数组会扫到并依赖 e!=null 兜底——改为有界下标,语义不变、不扫空槽 | ||
| // 按有效 size 迭代:Seq 的 items 尾部含空槽(容量>size),for-each 会扫到 null → 有界下标 + null 守卫 | ||
| int n = power.graph.all.size; |
There was a problem hiding this comment.
Robin —
改用 int n = power.graph.all.size; for (int k=0; k<n; k++) { Building e = power.graph.all.items[k]; ... } 并保留 if (e == null) continue。理由是 Seq.items 尾部可能有空槽。该做法比 for-each 安全且避免扫空槽。语义不变,正确。
Recommendation: 保留。
|
|
||
| public UniversalJunctionDialog(String title, DialogStyle style) { | ||
| super(title, style); | ||
| setFillParent(true); |
There was a problem hiding this comment.
Robin —
构造函数 setFillParent(true)。全屏布局配合拖拽预览(在 Core.scene.root 添加 ghost/hint)需要妥善清理(hide() 已清理 allRegions.clearDragPreview())。全屏对话框在小分辨率下可能遮挡过多 UI,但交互需求(整框拖拽、四列并排)需要较大空间,改动合理。
Recommendation: 确认 hide() 在任何关闭路径(back 按钮、esc、外部关闭)都被调用。BaseDialog.closeOnBack() 已设置,但需确保 clearDragPreview 覆盖所有区域状态。当前实现已遍历 allRegions 清理,足够。
|
|
||
| public void setup() { | ||
| allRegions.clear(); // shown → setup 可能多次调用:先清空,避免 re-show 时累积陈旧区域状态 | ||
| cont.table(grid -> { |
There was a problem hiding this comment.
Robin —
setup() 首行 allRegions.clear()。shown(this::setup) 可能多次触发(对话框重显),清空旧状态避免累积陈旧区域。正确。
Recommendation: 保留。
…接返回也落盘);Switch.tapped 不再客户端乐观翻转(configure(!fE) 由服务器回写)、移除 buildConfiguration 状态翻转、fE 语义注释;Changelog 显式按版本取 max(不依赖 ENTRIES 顺序)、图标未命中不缓存 null
处理说明(本轮审查,commit
|
|
/robin |
🏹 Robin✅ Review done. I flagged 6 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
🔵 3 Low | 💡 3 Suggestions
Summary
本次 diff 主要包含:新增更新日志弹窗(Changelog)、UniversalJunctionDialog UI 重构(整框拖拽、预览与布局优化)、Switch 行为调整、DualPurposeStorager 液体绘制方式简化,以及少量 UI 常量与资源 key 补充。改动方向合理,资源回退与防御性检查到位;但部分新 UI 的拖拽预览状态管理较复杂,存在少量可优化点。
Robin — the Robin Hood of code review. Free for every PR.
| list.image(Tex.whitePane, Color.yellow).growX().height(2f).padBottom(4f); | ||
| list.row(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: medium
draw() 中液体层绘制前增加了 Vars.renderer != null 防御性判断,但注释称“正常渲染期 renderer 已就绪”。此判断是安全的冗余检查,未发现实际崩溃风险。
Recommendation: 可保留该空判作为防御性保护,或在注释中补充触发场景(如异步加载时序)以提升可读性。
if (liquids.currentAmount() > LIQUID_THRESHOLD && Vars.renderer != null) {
| Block block = Vars.content.blocks().find(b -> b.name.equals("silicon-" + e.item)); | ||
| if (block == null) block = Vars.content.blocks().find(b -> b.name.startsWith("silicon-") && b.name.endsWith("-" + e.item)); | ||
| // 未命中就不入缓存,避免内容尚未加载时把 null 永久缓存 | ||
| if (block != null && block.uiIcon != null) { |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: medium
方块图标缓存仅在 block != null 且 block.uiIcon != null 时写入缓存(未命中不缓存 null),避免了内容加载时序导致的永久性 null 缓存,这是合理的设计。
Recommendation: 若需进一步降低 find() 调用,可考虑在内容加载完成(ContentInit/EventType.ContentInitEvent)阶段预热缓存,但当前实现已避免高频重复查找。
if (block != null && block.uiIcon != null) {
icon = block.uiIcon;
iconCache.put(e.item, icon);
}
| if (front() != null && front().team == team && front().enabled != fE) front().enabled = fE; | ||
| // #43 同步方向:本块为「状态反映器」——fE 跟随前方建筑的实际 enabled(含外部逻辑处理器改动), | ||
| // 自身 configure 也写 front 再置 fE;两处都写 front,故 fE 恒等于 front.enabled,无双向竞争。 | ||
| Building f = front(); |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: medium
updateTile() 由 front().enabled != fE 时将 fE 同步到 front.enabled(行105-109),而 config 回调也会写入 front.enabled(36-45)。两处均写入同一目标且有 team 与非 SwitchBuild 校验,语义清晰,但状态同步路径分散在 updateTile 与 configure 两处。
Recommendation: 建议在类注释或代码注释中明确“fE 为前方建筑 enabled 的镜像状态,仅用于渲染/存档,不作为控制源”,以避免后续维护时引入双向写入竞争。
if (f != null && f.team == team) {
if (f.enabled != fE) fE = f.enabled;
}
| }; | ||
| UniversalJunction.UniversalJunctionBuild build; | ||
| /** 单个黄色按钮正在被拖动时置 true:抑制整框重排的灰色占位框,避免按钮拖拽与整框拖拽互相干扰 */ | ||
| boolean draggingButton = false; |
There was a problem hiding this comment.
Robin — 💡 SUGGESTION · confidence: high
新增 draggingButton 标志用于抑制整框拖拽与按钮拖拽的互相干扰(行76及多处判断)。该状态是纯 UI 局部状态,未涉及网络同步,设计合理。
Recommendation: 可在字段注释中补充“仅在本对话框的输入事件处理期间生效”的范围说明,便于后续维护复杂的拖拽预览链路。
/** 单个黄色按钮正在被拖动时置 true:抑制整框重排的灰色占位框,避免按钮拖拽与整框拖拽互相干扰 */
boolean draggingButton = false;
| @@ -57,6 +83,7 @@ public UniversalJunctionDialog(String title) { | |||
|
|
|||
| public UniversalJunctionDialog(String title, DialogStyle style) { | |||
| super(title, style); | |||
There was a problem hiding this comment.
Robin — 💡 SUGGESTION · confidence: high
构造函数调用 setFillParent(true)(行85),配合 setup() 多次调用时先清空 allRegions(行111),避免 re-show 时累积陈旧区域状态,处理得当。
Recommendation: 可考虑在 hide() 统一调用 clearDragPreview() 并清理临时根层提示(hint/ghost)以降低内存占用和场景残留风险(当前已有 clearDragPreview() 覆盖主要预览状态)。
setFillParent(true);
row();
| if (current.isEmpty()) return; | ||
|
|
||
| // 取 ≤ 当前版本的最高版本(显式按版本比较,不依赖 ENTRIES 的排列顺序) | ||
| String target = null; |
There was a problem hiding this comment.
Robin — 💡 SUGGESTION · confidence: high
目标版本选择改为“显式按版本比较,不依赖 ENTRIES 的排列顺序”(行65-70),通过 UpdateChecker.isNewer 计算 ≤ currentVersion 的最高版本,避免了顺序假设。
Recommendation: 建议在 ENTRIES 初始化处补充注释说明版本顺序约定(旧→新或任意顺序均可),与该比较策略保持一致。
String target = null;
for (Entry e : ENTRIES) {
if (UpdateChecker.isNewer(e.version, current)) continue;
if (target == null || UpdateChecker.isNewer(target, e.version)) target = e.version;
}
…同步」范围说明;ENTRIES 补「顺序无关、显式按版本取最高」注释
结论:本轮(05:55)审查 无实际缺陷,均为 Low/Suggestion 级确认与注释建议逐条核对:
本轮无 High/Medium 缺陷(对比更早的若干轮,High/Medium 项均已在前几次提交中修复或确认为误报)。本地离线构建 |
There was a problem hiding this comment.
自动审查结果(Robin Review / workflow_run 二段式)
本 PR 引入更新日志弹窗、重构通用接线器(UniversalJunction)配置面板的拖拽交互、调整 Switch 语义为「状态反映器」、简化 RollGenerator/PowerSource 性能逻辑并移除 DualPurposeStorager 的 -liquid 贴图依赖。多数改动实现质量良好(preview 折叠、避免 detach 触摸焦点、纯 setPosition 复位等注释充分),但 Switch 的语义反转存在网络同步与状态反馈问题,另有少量 UI 状态残留与资源清理缺失。
共 13 条发现,0 条已发为行内评论。
审查意见先说结构:
|
| 级别 | 问题 |
|---|---|
Changelog 每次启动必弹,但自称是调试用途 |
|
| PR 范围超出标题(含两个行为级改动) | |
| 🔵 | Changelog 类注释描述已删除的 seen 机制 |
| 🔵 | Switch 的 configurable = false 是隐式依赖,建议加注释(最值得补) |
| 🔵 | Switch 丢失点击音效 |
| 🔵 | RollGenerator 注释被换成旧版,丢失关键解释 |
| 🔵 | 两个 ClientEvent 监听可合并 |
无阻塞项。第 1 条建议在合并前定一下 —— 那是玩家每次启动都会看到的弹窗。
…ch 补 configurable 必须为 false 的原因注释、恢复点击音效 Sounds.click;RollGenerator 恢复丢失的关键注释(断电≠停 updateTile、O(R²G)/O(V²G));Silicon 合并两个 ClientLoadEvent 监听
处理说明(本轮人工复审,commit
|
…框」相同的 previewInsert 占位机制(rebuildSlotContents+setSlotHeight),由布局实时重排,灰框与按钮真正交换位置
…otContents 会 detach 被拖按钮触发合成 touchUp 提前放置;现被拖按钮 cell 移到 previewRow 行留空、灰框画在行位置,实现真正换位且不打断拖拽
There was a problem hiding this comment.
自动审查结果(Robin Review / workflow_run 二段式)
本 PR 涉及 UniversalJunctionDialog 大规模交互重构(整框拖拽换位、动态高度、红色优先级框自动伸缩)、Switch 从 configTapped 改为 tapped+configure、Changelog 新增弹窗、DualPurposeStorager 去掉 -liquid 贴图改用 drawTiledFrames,以及若干注释/性能清理。方向上网络同步链路(configure 由服务器执行、目标固定为正前方建筑)是正确的,但存在数处中等程度的逻辑缺陷:Switch 切换后记忆状态 fE 在两台 Switch 互指/外部改写场景下会来回抖动,Switch 未写 write/read 保存 fE 记忆值,Changelog 的 Javadoc 与实际「仅版本变化才弹」语义矛盾,Switch 由 configTapped 改为 tapped 后会与 rotate 的点击缩放/放置交互产生耦合,且 UniversalJunctionDialog.hide() 无条件回写权重会在客户端未初始化 build 时误清空既有路由。
共 13 条发现,0 条已发为行内评论。
|
/robin |
🏹 Robin✅ Review done. I flagged 14 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
本 PR 整体质量较高:Switch 的 config 回调改为由服务端解析 front() 并做同队/非开关校验,堵住了客户端指定任意目标的安全口子;UniversalJunctionDialog 针对 arc「detach 元素触发合成 touchUp」这一坑做了系统性的规避(applySlotOrder/applyContentOrder/setVacating 只改 Cell 不 clearChildren),注释也如实记录了踩坑原因;BlockSearch 抽出 searchH 消除魔数、PowerSource 补 power/graph 空守卫均为正确改进。问题集中在三处:Switch 的 configurable 前提可能自相矛盾导致功能整体失效、对话框 hide() 无条件回写权重会静默改写新放置方块的路由、满槽位时仍绘制「新建槽位」灰框预览(预览≠实际)。此外还有红框竖排导致的整屏高度溢出、拖拽中途关闭会残留 root 层 ghost/hint 等问题。
📄 Findings Not Posted Inline
💡 1 (src/silicon/ui/UniversalJunctionDialog.java:100) — hide() 里连续两次遍历 allRegions(一次 syncWeights、一次 clearDragPreview),且每个区域各发一次 build.configure(weightsString()),关闭面板最多产生 4 次内容完全相同的整串配置请求。
合并为一次遍历;并考虑把权重写回聚合成一次 configure 调用或在确有变化时才写。
💡 2 (src/silicon/world/blocks/container/DualPurposeStorager.java:123) — draw() 内的 Vars.renderer != null 在正常渲染期恒为非空(渲染期 renderer 必然已就绪),属于防御性冗余判断,注释里也承认是防御性写法。
可去掉该判空保持与原版 LiquidBlock 绘制路径一致;若保留,请在注释中说明触发 null 的具体场景(如 headless 绘制)。
Robin — the Robin Hood of code review. Free for every PR.
| group = BlockGroup.logic; | ||
| // 配置同步开关自身状态 fE(联网经 Call.tileConfig 全端一致);front 目标由 updateTile 同帧应用 | ||
| config(Boolean.class, (building, value) -> ((SwitchBuild) building).fE = value); | ||
| config(Boolean.class, (building, enabled) -> { |
There was a problem hiding this comment.
Robin —
注释声称「configurable 故意保持 false,靠 tapped()+configure()」,但紧接着的 config(Boolean.class, ...) 在 Mindustry v146+ 的实现里本身就会把 configurable 置为 true;而 InputHandler 对 configurable 建筑会消费点按(consumed),tapped() 不会触发。反过来若确为 false,则 Building.configure() 内的 if(!block.configurable) return; 守卫会让 configure 完全无效。两种情况都与注释的前提不符,开关可能完全点不动。
Recommendation: 先在 v159.7 源码确认 Block.config(...) 是否置 configurable、以及 Building.configure() 是否有该守卫:若两者都成立,本方案不可行,应回到 configurable=true + configTapped() 返回 false 抑制菜单的旧写法,或改走 Call.tileConfig 之外的自定义客户端→服务器 RPC;确需 configurable=false 时必须在 config(...) 之后显式再赋值一次。
// 现状:config(...) 可能把 configurable 重新打开
config(Boolean.class, (building, enabled) -> { ... });
// 建议:显式且可验证
config(Boolean.class, (building, enabled) -> { ... });
configurable = false; // 需先确认 Building.configure 无 !configurable 守卫
| } | ||
|
|
||
| @Override | ||
| public void hide() { |
There was a problem hiding this comment.
Robin —
hide() 无条件对所有区域调用 rs.syncWeights()(按注释该方法会 configure 写回方块),而 setup() 对 freshDefault(权重全 2)的新方块把 4 个按钮全部渲染进红框(红框 = 权重 0 = 不输出)。结果:玩家对新放置的中枢仅仅「打开面板再点返回」,weights 就被从全 2 静默改写为全 0,方块直接失去路由能力——这与第 147 行注释「仅还原显示:不改写 weights,也不在此触发 configure,避免打开面板即重置路由」的设计意图直接冲突。
Recommendation: 引入 dirty 标记:只有用户实际拖放/新建槽位过(placeInSlot/placeInRed/placeIntoSlot/createSlotFor)才在 hide() 里 syncWeights;或保存一份打开时的原始 weights,hide() 时若布局未变则跳过写回。
boolean dirty = false; // 任一 place*/create 操作置 true
public void hide(){
if(build != null && dirty){ for(RegionState rs : allRegions) rs.syncWeights(); }
...
}
| * 来源框被拖成空框(srcSlotBox)后仍占据其 cell 空间(视觉上就是两白框之间的空白/空槽位), | ||
| * 预览不对它做「折叠」:否则下方白框会整体上跳一个整框高度,灰框与指针错位并出现交错。 | ||
| * 灰框画在 phantom 处 = drop 后新框的落点;两可见框之间才用下移扩隙并居中的方式。 */ | ||
| private void drawBoxReflowPreview(float sx, float sy) { |
There was a problem hiding this comment.
Robin —
drawBoxReflowPreview 在「未落在任何白框/红框」时无条件绘制「新建槽位」灰框,完全不检查 MAX_SLOTS;而 touchUp 的落点分支里只有 rs.slotBoxes.size < MAX_SLOTS 才会 createSlotFor,否则会「放回原位」。当已有 4 个槽位时,拖拽预览会推开相邻白框显示一个新建槽位灰框,松手却什么也不发生——预览与实际行为不一致,用户会反复尝试。
Recommendation: 在 updateDragPreview 调用 drawBoxReflowPreview 前加 if(rs.slotBoxes.size < MAX_SLOTS) 守卫,满槽时直接走 removeHint()+setSourceVacating 分支,不做任何新建槽位预览。
if(!inRed && rs.slotBoxes.size < MAX_SLOTS){ ... drawBoxReflowPreview(sx, sy); return; }
|
|
||
| public UniversalJunctionDialog(String title, DialogStyle style) { | ||
| super(title, style); | ||
| setFillParent(true); |
There was a problem hiding this comment.
Robin —
面板改为 setFillParent(true),同时 BTN_H 从 52 提到 100、红框从 2x2 宫格改为竖排。每个输入区域的红框高度变为 4*(100+8)+24 ≈ 456px,加标题/槽位层后 4 个区域纵向合计超过 1900px,远超 1080p 可用高度;arc 的 Table 在子元素超出父高度时不会裁剪而是溢出,最下面 1~2 个输入方向的按钮会被挤出屏幕外无法点击。
Recommendation: 把内容包进可滚动容器(dialog.cont.pane(...) 配 grow + 内部 ScrollPane),或恢复红框 2 列宫格 / 把 BTN_H 降回 60 左右,使 4 个区域总高在 1080p 下能容下。
dialog.cont.pane(new ScrollPane()).grow(); // 或恢复 2 列:
redBox.add(redButtons.get(i)).growX().height(BTN_H).pad(4f);
if(i % 2 == 1) redBox.row();
| hint.setSize(w, h); | ||
| hint.touchable = Touchable.disabled; | ||
| hint.setPosition(centerX - w / 2f, bottomY); | ||
| Core.scene.root.addChild(hint); |
There was a problem hiding this comment.
Robin —
拖拽影(ghost,line 533)与灰色提示框(hint,line 1132)都被 addChild 到 Core.scene.root,但清理只发生在 clearGhost()/clearRootHint(),二者都只由 touchUp 触发;hide() 只清 slotBoxes 的 wholeDragPreview 状态。若拖拽途中面板被关闭(ESC/back)或触摸焦点被 cancelTouchFocus 打断,root 层的 ghost/hint 会永久残留在屏幕上,draggingButton 也会一直为 true 导致整框拖拽此后完全失效。
Recommendation: 把在途 ghost/hint 的清理提取为对话框级方法,在 hide() 中遍历所有 RegionState 的 Direction 一并清理并复位 draggingButton;或统一在 touchUp/touchCancelled 之外提供 cancelDrag() 供关闭路径调用。
public void hide(){
for(RegionState rs : allRegions){ rs.syncWeights(); rs.cancelAllDrags(); }
...
}
| if (i < 0 || i >= rs.slotBoxes.size) return false; | ||
| float y = currentStackTop(); | ||
| for (int k = 0; k < i; k++) { | ||
| y -= rs.slotHeightWithPreview(k) + 40f; |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: high
槽位间距 40f 以字面量形式散落在 4 处:rebuildSlots 的 padBottom(40f)(259 行)、inBaseBox 的 +40f(907 行)、restoreBoxPristine 的 +40f(1096 行)、drawBoxReflowPreview 的 GAP = 40f(1025 行)。命中判定与视觉布局强耦合,改一处而漏另一处就会让落点判定错位——本 PR 在 BlockSearch 里刚为此抽了 searchH 常量,这里应对齐同样做法。
Recommendation: 抽出 private static final float SLOT_GAP = 40f; 并在上述 4 处统一引用。
private static final float SLOT_GAP = 40f;
| /** 方块图标缓存:避免每次弹窗对每个条目重复 Vars.content.blocks() 线性查找 */ | ||
| private static final ObjectMap<String, TextureRegion> iconCache = new ObjectMap<>(); | ||
|
|
||
| /** |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: high
checkAndShow 的 Javadoc 写「每次启动必弹当前版本的改动内容(供调试方便观察)」,但第 66 行实现是「版本未变则不弹」,类级 Javadoc(20-22 行)也写的是「仅版本变化时弹」。调试期注释遗留在正式方法上,会误导后续维护者以为调试开关还开着。
Recommendation: 删除/改写 58-61 行的调试期描述,与类级注释保持一致。
|
|
||
| /** 更新日志条目(版本 → 文案)。顺序无关紧要:checkAndShow 显式按版本比较取 ≤ 当前版本的最高版本。 */ | ||
| private static final Entry[] ENTRIES = { | ||
| new Entry("a0.12.2.0", "universal-junction", "配置界面重构:拖拽换位、动态分组、预设与保存"), |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: high
ENTRIES 的 desc/item 文案全部硬编码中文,未走 bundle;而同一 PR 刚刚为 changelog.title 同时补了 bundle.properties 与 bundle_zh_CN.properties。英文包用户打开更新日志会看到中文条目,与项目其余 UI 全部走 bundle 的做法不一致。
Recommendation: 把 desc(及 item 显示名)抽成 bundle key(如 changelog.desc.a0.12.2.0.1 / changelog.item.*),在两个 bundle 中各写一份;ENTRIES 只保留版本与 key。
new Entry("a0.12.2.0", "universal-junction", "changelog.desc.a0.12.2.0.1")
| } | ||
|
|
||
| public static class Entry { | ||
| public final String version; |
There was a problem hiding this comment.
Robin — 🔵 LOW · confidence: medium
Entry 的三个 public final 字段(version/item/desc)没有任何 Javadoc,违反项目规范「public 字段必须有 Javadoc」。
Recommendation: 为 version(版本号)、item(方块内部名,用于取图标与 bundle key)、desc(改动描述)各补一行 Javadoc。
| Events.on(EventType.ClientLoadEvent.class, e -> { | ||
| MOD = mods.getMod(Silicon.class); | ||
| if (MOD != null) MOD.meta.subtitle = MOD.meta.version; | ||
| silicon.util.Changelog.checkAndShow(); |
There was a problem hiding this comment.
Robin — 💡 SUGGESTION · confidence: high
在 ClientLoadEvent 回调里用全限定名 silicon.util.Changelog.checkAndShow() 调用,与项目「按包分组 import、优先使用 import」的 import 规范不一致。
Recommendation: 在 import 区加入 import silicon.util.Changelog;,调用处改为 Changelog.checkAndShow();。
万向交叉器新版 UI 调整(含附带改动)
主改动:万向交叉器配置面板新版 UI
附带改动(及理由)
Changelog.java(新增):启动时更新日志弹窗——仅在版本号变化时弹(常规更新日志语义)。Switch.java(行为调整):由「配置菜单」改为「点按即切换」。原configurable=true会让InputHandler把点击标记为已消费,导致tapped()不触发,故必须保持configurable=false;fE为前方建筑enabled的镜像状态。RollGenerator.java/PowerSource.java(性能/健壮性):电网统计由 O(R²G)/O(V²G) 降为 O(m);保留!enabled/PowerVoid时currentPowerProduction归零守卫与power.graph空判。DualPurposeStorager.java(渲染):去掉liquidRegion静态层,中间液面改用原版drawTiledFrames动态绘制(-liquid贴图不再依赖)。