search

plusls / oh-my-minecraft-client

oh-my-minecraft-client
GNU Lesser General Public License v3.0
138 stars 62 forks source link

add chatNetworkLagFix: MC-218167 fix #31

Closed leedagee closed 3 years ago

leedagee commented 3 years ago

没弄明白为什么 mixin 和 accesswidener 都不能对 authlib 生效（似乎 authlib 在 fabricloader 之前加载？），只加了 minecraft 里面相应的逻辑，效果大概只缺了一个退出游戏作废缓存中的黑名单列表。

~README{,_EN}.md 还没写~

plusls commented 3 years ago

话说这个 fix 代码是通过 diff 快照得到的么？（没咋了解过这个 bug

leedagee commented 3 years ago

话说这个 fix 代码是通过 diff 快照得到的么？

可以通过网络抓包观察行为得到

（没咋了解过这个 bug

感觉在正版登录的情况下应该挺常见的，比如 BV11Q4y1k7pa 的 00:29 就出现了，我自己几乎每次都卡住

plusls commented 3 years ago

同时，按我的理解，这玩意只有在第一次调用的时候才会去尝试加载，在后续的过程中貌似就不会了？那还有必要卸载它么

leedagee commented 3 years ago

新版的 authlib 里面这个就叫 refresh（authlib 没有混淆），所以应该还有刷新这个列表的作用。但是不改 authlib 里面的东西也没法 invalidate 这个列表，所以好像也确实没必要实现卸载逻辑。

顺便问下 https://fabricmc.net/wiki/tutorial:accesswideners 这里说只有 mixin 做不了才用 accesswidener 主要是什么原因？倒是 forge 那边 mixin 生态不太好，所以 AT 的 doc 没怎么说。

plusls commented 3 years ago

顺便问下 https://fabricmc.net/wiki/tutorial:accesswideners 这里说只有 mixin 做不了才用 accesswidener 主要是什么原因？倒是 forge 那边 mixin 生态不太好，所以 AT 的 doc 没怎么说。

其实这个我也不太清楚，我个人猜测可能是因为这个直接对字节码进行了操作，兼容性什么的会有点问题？（以及这玩意不是 mixin 的东西，这是 fabric 自己实现的）

plusls commented 3 years ago

新版的 authlib 里面这个就叫 refresh（authlib 没有混淆），所以应该还有刷新这个列表的作用。但是不改 authlib 里面的东西也没法 invalidate 这个列表，所以好像也确实没必要实现卸载逻辑。

所以我在想能不能做成进入游戏后自动加载一次拉倒，后续就不需要管了

plusls commented 3 years ago

以及我这可能网比较好，没怎么出现过这个问题，测试估计还得麻烦你来做

leedagee commented 3 years ago

其实这个我也不太清楚，我个人猜测可能是因为这个直接对字节码进行了操作，兼容性什么的会有点问题？（以及这玩意不是 mixin 的东西，这是 fabric 自己实现的）

我知道这个是 fabricloader 负责的，感觉 accesswidener 对所有类都放开了限制，而 mixin 只能做 mixin 里面写好的操作，应该不那么容易被玩坏？

plusls commented 3 years ago

其实这个我也不太清楚，我个人猜测可能是因为这个直接对字节码进行了操作，兼容性什么的会有点问题？（以及这玩意不是 mixin 的东西，这是 fabric 自己实现的）

我知道这个是 fabricloader 负责的，感觉 accesswidener 对所有类都放开了限制，而 mixin 只能做 mixin 里面写好的操作，应该不那么容易被玩坏？

不过我个人比较懒，懒得写 invoker accessor了，就全写到 accesswidener 里了

leedagee commented 3 years ago

刚刚看了一下代码，似乎 accesswidener 都不需要，SocialInteractionsManager 已经有 isPlayerBlocked 了（主要是最开始写的时候试了半天 mixin authlib 没成功，结果还是写了个 mixin

plusls commented 3 years ago

刚刚看了一下代码，似乎 accesswidener 都不需要，SocialInteractionsManager 已经有 isPlayerBlocked 了（主要是最开始写的时候试了半天 mixin authlib 没成功，结果还是写了个 mixin

没事，写了就写了，估摸着也不会有人来动这边的代码，出锅再说把

leedagee commented 3 years ago

所以我在想能不能做成进入游戏后自动加载一次拉倒，后续就不需要管了

要不我还是重新写个最简单的吧

plusls commented 3 years ago

所以我在想能不能做成进入游戏后自动加载一次拉倒，后续就不需要管了

要不我还是重新写个最简单的吧

你这么一说确实，还不如直接改成，“关闭黑名单检测” 这种（翻译的不准确）

leedagee commented 3 years ago

这回没管 mojang 在新快照里的逻辑了，是我自己认为该这么修。但是因为没有微软帐号，能不能正确处理黑名单我还不知道。

leedagee commented 3 years ago

cdn.modrinth.com 貌似用错了回源证书然后 github actions 挂了（

plusls commented 3 years ago

其实吧 要我说还不如直接把这个功能改成，关闭黑名单检查，解释里面说可以解决这个bug（反正国人也不需要这些黑名单

plusls commented 3 years ago

以及我建议直接新开一个分支（

leedagee commented 3 years ago

以及我建议直接新开一个分支（

我 git rebase 了，旧的那种 mixin 方法（更接近 mojang 修复方法的）已经没了（

plusls commented 3 years ago

哦说起来我之前review用不太来，发的有问题

我的想法还是，不如直接把黑名单检查给干掉，毕竟有一个场景，如果玩家进入一个服务器马上打了一个字然后退出来再进一次再打之类的，可能会触发多次？（不想思考这种极端的情况，但是确实有可能发生

对于维护者来讲最简单的还是直接删（逃

plusls commented 3 years ago

而且这样一来这功能也可以在1.18保留，毕竟是个开关黑名单

leedagee commented 3 years ago

重写过的只跳过了 MinecraftClient.shouldBlockMessages 里面调用 isPlayerMuted，从 Network Thread 加消息到主线程的时候，提前在 IO Thread 里面检查黑名单，然后主线程里跳过检查。效果就是收到消息不会立即显示，主线程也不会卡，直到黑名单加载好了，才交给主线程

leedagee commented 3 years ago

说实话，彻底关闭黑名单的 Mixin 我自己用了很久了，只是觉得如果要做一个 fix 而不是 workaround 出来，一刀切还是不太好。当然加一个选择彻底关掉这些功能（包括其他关于社交的功能如 https://minecraft.fandom.com/wiki/Social_Interactions_screen ）的也挺好

plusls commented 3 years ago

因为感觉现在的补丁并不能从根本上解决问题，只是解决了聊天卡顿，你打开社交 gui 还是会卡顿，照这样看来，只要检查黑名单的函数触发就会卡一下，上个版本的补丁反倒才是一个治本的方案。

所以说就很蛋疼，而且也很不优雅（指这种 case by case 的修复