Skip to content

修复 Mojang 运行时类名转译#714

Open
Jie-150 wants to merge 1 commit into
TabooLib:dev/6.3.0from
Jie-150:dev/6.3.0
Open

修复 Mojang 运行时类名转译#714
Jie-150 wants to merge 1 commit into
TabooLib:dev/6.3.0from
Jie-150:dev/6.3.0

Conversation

@Jie-150

@Jie-150 Jie-150 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@FxRayHughes

Copy link
Copy Markdown
Contributor

Code Review — #714 修复 Mojang 运行时类名转译

改动很小(+18/-4),但动的是 RemapTranslation.translate 里 Mojang Mapping 环境的分支,属于 NMS 转译核心路径。方向我认同——原来那句「不管是 Spigot.Fullname 还是 Mojang.Fullname 都不需要动」确实过于乐观,把责任全推给 Paper PluginRemapper。

有两点建议在合并前处理:一是 fallback 的线性扫描开销(实测单次 60.6 微秒,而仓库里已有现成的 HashMap 可用),二是PR 缺少描述正文,看不出具体修的是什么场景。

审阅方式:读 patch + 对照仓库源码 + 用 Mojang 官方 1.21.4 映射表实测规模与冲突率、JMH 风格微基准测扫描开销。未实跑 gradle 测试(改动路径需要真实 Paper 环境)。


🟡 问题 1 — fallback 的短名线性扫描可以换成现成的 classMapMojangS2F

新增代码:

fun translateMojangToRuntimeOrKeep(key: String): String {
    val runtimeName = key.replace('/', '.')
    if (hasRuntimeClass(runtimeName)) {
        return key
    }
    val shortName = runtimeName.substringAfterLast('.')
    val mappingName = MinecraftVersion.paperMapping.classMapSpigotToMojang[runtimeName]
        ?: MinecraftVersion.paperMapping.classMapSpigotToMojang.values.singleOrNull { it.substringAfterLast('.') == shortName }
        ?: return key
    return if (hasRuntimeClass(mappingName)) mappingName.replace('.', '/') else key
}

第二行 fallback 是对 classMapSpigotToMojang.values 做全量线性扫描。而 Mapping.kt:31-32已经有一张专门做这件事的表

// <Mojang.SimpleName, Mojang.Fullname>
val classMapMojangS2F: MutableMap<String, String> = HashMap(),

构建处 Mapping.kt:188

mapping.classMapMojangS2F[mojangName.substringAfterLast('.', "")] = mojangName

正是「Mojang 短名 → Mojang 全名」。MinecraftServerUtil.kt:67 已经在用它做同样的短名查找。

实测开销

我用 Mojang 官方 1.21.4 server mappings(piston-data.mojang.com,101167 行)实测:

map size = 6329
单次线性扫描: 60.6 微秒
HashMap 单次查找: 0.070 微秒
1000 次调用累计: 60.6 毫秒
10000 次调用累计: 606.4 毫秒

相差约 870 倍translate() 会被 ASM ClassRemapper 对类里每一个类型引用调用一次,一个 NMSProxy 实现类轻易有几十上百个 net/minecraft 引用;而 translate 这一层没有缓存——runtimeClassCache 只缓存 Class.forName 的结果,不缓存映射查找。

不过要说明:这条 fallback 只在 classMapSpigotToMojang[runtimeName] 未命中时才走,而 hasRuntimeClass(runtimeName) 又在它之前提前返回。所以真正命中扫描的是「运行时不存在、且不在 Spigot→Mojang 表里」的 key,比例应该不高。但 AsmClassTranslationBinaryCache,首次转译(冷缓存)时这个开销会集中体现。

建议

val mappingName = MinecraftVersion.paperMapping.classMapSpigotToMojang[runtimeName]
    ?: MinecraftVersion.paperMapping.classMapMojangS2F[shortName]
    ?: return key

语义上有一点差异需要注意:classMapMojangS2FHashMap,短名冲突时后写入的覆盖先写入的Mapping.kt:188 直接 [key] = value),而 singleOrNull 在冲突时返回 null 从而保守地 return key。所以直接替换会把「冲突时不动」变成「冲突时取任意一个」。

我实测了冲突率:

net.minecraft.* 类数: 6317
短名唯一的: 6049
短名冲突的短名个数: 11   (其中 package-info 占 247 个类)
去掉 package-info 后真正冲突: 10 个短名 / 21 个类
冲突占比: 4.2%(几乎全是 package-info)

冲突最多的是 BlockPredicate(3)、EntitySelector/ExecuteCommand/Main/EntityDataAccessor/Blocks/Items/TagEntry/Control/ContainerListener(各 2)。

所以要保住 singleOrNull 的保守语义,建议预建一张「短名 → 全名,冲突则标记为不可用」的表,只算一次:

// Mapping 里加,或在 RemapTranslation 里 lazy 建
private val uniqueMojangShortNames: Map<String, String> by lazy {
    MinecraftVersion.paperMapping.classMapSpigotToMojang.values
        .groupBy { it.substringAfterLast('.') }
        .filterValues { it.size == 1 }
        .mapValues { it.value.single() }
}

这样既是 O(1) 查找,又完整保留了「冲突不动」的行为。


🟡 问题 2 — PR 没有描述正文,无法判断修复的具体场景

gh pr view 714 的正文是空的。这批 PR(#704#713)都有「原有问题 / 典型触发场景 / 本 PR 修改 / 验证」四段结构,本 PR 只有标题「修复 Mojang 运行时类名转译」。

对 remap 这种影响面大、又很难在本地复现的改动,缺少描述会让评审只能靠读代码倒推意图。建议至少补上:

  • 触发条件:什么服务端(Paper 版本)、什么 mapping 环境、哪个类的转译出了错
  • 错误现象:是 ClassNotFoundExceptionNoSuchMethodError,还是转译到了语义不同的同名类
  • 为什么 Paper PluginRemapper 没能处理:原注释明确写了「如果是 Spigot.Fullname,Paper PluginRemapper 会进行转译」,本 PR 推翻了这个假设,值得说明是哪种情况下它不生效

这条不是代码问题,但对后续维护(尤其是这段注释被删掉之后)很关键。


🔵 次要

a. 新增的 import 是多余的。

import taboolib.module.nms.remap.RemapTranslation.Companion.extraTransformers

extraTransformers同一个文件内 RemapTranslation.Companion 的成员,applyExtraTransforms:52)本来就能直接访问,不需要 import。这行大概是 IDE 自动加的,建议删掉。同时那次 Remapper import 的重排(移到 org.objectweb.asm.commons 分组)是纯格式调整,与本 PR 主题无关,可以保留但不必要。

b. 方法命名与既有风格的一致性。 新方法叫 translateMojangToRuntimeOrKeep,而既有的是 translateMojangToSpigotOrKeepRuntime。两者结构对称(都是「转译 + 保留运行时」),但后缀顺序相反(OrKeep vs OrKeepRuntime),读起来容易混。建议统一成 translateMojangToRuntimeOrKeepRuntime 或把旧的改成 translateMojangToSpigotOrKeep

c. 注释被删除了但没有替代说明。 原代码那两行注释解释了「为什么什么都不做」:

// 如果为 Mojang Mapping 环境,这里不管是 Spigot.Fullname 还是 Mojang.Fullname 都不需要动
// 如果是 Spigot.Fullname,Paper PluginRemapper 会进行转译

改成 translateMojangToRuntimeOrKeep(key) 后这段背景消失了。新方法上的 KDoc 只写了「将 Mojang 类名转为 Runtime 类名,运行时已有类名优先保留」,没说明为什么现在需要动了、以及 Paper PluginRemapper 的哪部分不可依赖。建议在调用点或 KDoc 里补一句。

d. hasRuntimeClass 可能被调用两次。 新方法开头 hasRuntimeClass(runtimeName),末尾 hasRuntimeClass(mappingName)。两次都走 runtimeClassCache.getOrPut,有缓存所以不重复触发 Class.forName,没问题。只是提一下 runtimeClassCache每个 RemapTranslation 实例独立的(:33 是实例字段),而 AsmClassTranslation.kt:76 每转译一个类就 new 一个 remapper,所以缓存实际是单类生命周期的。这是既有设计,本 PR 没改变,但结合问题 1 的扫描开销,意味着跨类之间没有任何复用。


🟢 已核对无误

结论
改动影响范围 RemapTranslation.translateisUniversal && isMojangMapping && !startsWith("net/minecraft/server/v1_") 分支。RemapTranslationTabooLib:79)完全覆盖了 translate,所以 TabooLib 自身类不受影响;RemapTranslationLegacy / RemapTranslationUnobfuscated 同理
生效条件 AsmClassTranslation.kt:72-80 确认:只有 isUniversalCraftBukkit 且转译对象不是 TabooLib 类时才用 RemapTranslation 基类。即只影响插件本体类在 Paper 1.20.5+ 上的转译,与类文档描述一致
classMapSpigotToMojang 的方向 Mapping.kt:186 确认是 <Spigot.FullName, Mojang.FullName>。新代码用 [runtimeName] 查,处理的是「key 是 Spigot 全名」的情况,方向正确
保守性 三处 return key 兜底:运行时已有该类、映射表查不到、映射目标在运行时不存在。任何不确定的情况都保持原样,不会把正确的名字改坏。这个设计是对的
hasRuntimeClass 优先 与既有 translateMojangToSpigotOrKeepRuntime:127-138)的策略一致——运行时已存在就不动。:142-145 的注释解释了原因(1.17-1.20.4 存在 Mojang 与 Spigot 同名但语义不同的类,如 MobEffect),新方法沿用同一原则
singleOrNull 的保守语义 短名冲突时返回 null → return key,不会误转译。我用 1.21.4 官方映射实测:6317 个 net.minecraft.* 类中 6049 个短名唯一,真正冲突的只有 10 个短名 / 21 个类(其余 247 个是 package-info)。所以这个 fallback 的命中率高且误判风险低
package-info 不会误命中 虽然有 247 个 package-info,但它们短名相同 → singleOrNull 返回 null → return key。而且 package-info 不会作为类型引用出现在字节码里
RemapReflex* 的关系 本 PR 只改 ASM 静态转译路径,未触及 RemapReflexPaper / RemapReflexSpigot 的运行时反射转译,两条路径互不干扰
extraTransformers 机制未受影响 applyExtraTransforms:51-63)与 incision 模块的 PipelineBackend(通过反射注册)逻辑未变

总结

修复方向对——原来在 Mojang Mapping 环境下直接 return key,完全依赖 Paper PluginRemapper,新代码补了「运行时找不到就查映射表」这一层兜底,而且三处 return key 的保守设计让它不会把正确的名字改坏。

建议合并前处理:

  1. 问题 1:fallback 的 values.singleOrNull { ... } 换成 O(1) 查找。仓库里 Mapping.kt:32classMapMojangS2F 就是为此准备的(MinecraftServerUtil.kt:67 已在用)。若要保留「短名冲突时不动」的保守语义,建议预建一张过滤掉冲突短名的表。实测线性扫描单次 60.6 微秒 vs HashMap 0.070 微秒。
  2. 问题 2:补 PR 描述。remap 改动难以本地复现,缺少触发场景和错误现象会让后续维护者无法判断这段逻辑为何存在。
  3. 🔵 里那行多余的 extraTransformers import 建议删掉。

说明:本次审阅未实跑 gradle 测试——该改动路径需要真实 Paper 1.20.5+ Mojang Mapping 环境才能触发,本地无法验证。映射表规模、短名冲突率与扫描开销为本机实测(Mojang 官方 1.21.4 server mappings,6329 条类映射);其余结论基于 patch 与仓库源码推导,已逐条注明依据位置。

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