LoliKingdom / LoliASM

The lolis are now preparing to bytecode manipulate your game.
GNU Lesser General Public License v2.1
105 stars 22 forks source link

Skin texture is released too early #125

Closed Water-Moon closed 1 year ago

Water-Moon commented 1 year ago

When the client is forcefully disconnected from the server (e.g. connection issue), this mod will attempt to release the skin textures before the client is actually disconnected. As a result, the player skin will become a missing texture right before the client actually disconnect, and this problem will persist until client restart.

From the log, it is easy to see that the client attempted to load the skin right after it is deleted, and then threw a FileNotFoundException

Related log

> [11:53:49] [Netty Client IO #9/INFO]: Client disconnected from server > [11:53:49] [Netty Client IO #9/INFO]: Cleaning client cache... > [11:53:49] [Netty Client IO #9/INFO]: Cleared cached client data! Disconnected from server. > [11:53:49] [Client thread/INFO]: Released 2 skin textures > [11:53:49] [Client thread/WARN]: Failed to load texture: minecraft:skins/db9a*** > java.io.FileNotFoundException: minecraft:skins/db9a*** > at net.minecraft.client.resources.FallbackResourceManager.getResource(FallbackResourceManager.java:69) ~[cei.class:?] > at net.minecraft.client.resources.SimpleReloadableResourceManager.getResource(SimpleReloadableResourceManager.java:65) ~[cev.class:?] > at net.minecraft.client.renderer.texture.SimpleTexture.loadTexture(SimpleTexture.java:40) ~[cdm.class:?] > at net.minecraft.client.renderer.texture.TextureManager.loadTexture(TextureManager.java:101) [cdr.class:?] > at net.minecraft.client.renderer.texture.TextureManager.bindTexture(TextureManager.java:67) [cdr.class:?] > at net.minecraft.client.renderer.ItemRenderer.renderArmFirstPerson(ItemRenderer.java:264) [buu.class:?] > at net.minecraft.client.renderer.ItemRenderer.renderItemInFirstPerson(ItemRenderer.java:394) [buu.class:?] > at net.minecraft.client.renderer.ItemRenderer.renderItemInFirstPerson(ItemRenderer.java:359) [buu.class:?] > at net.minecraft.client.renderer.EntityRenderer.renderHand(EntityRenderer.java:1063) [buq.class:?] > at net.minecraft.client.renderer.EntityRenderer.renderHand(EntityRenderer.java:1007) [buq.class:?] > at net.minecraft.client.renderer.EntityRenderer.renderWorldPass(EntityRenderer.java:1981) [buq.class:?] > at net.minecraft.client.renderer.EntityRenderer.renderWorld(EntityRenderer.java:1621) [buq.class:?] > at net.minecraft.client.renderer.EntityRenderer.updateCameraAndRender(EntityRenderer.java:1390) [buq.class:?] > at net.minecraft.client.Minecraft.runGameLoop(Minecraft.java:1119) [bib.class:?] > at net.minecraft.client.Minecraft.run(Minecraft.java:3614) [bib.class:?] > at net.minecraft.client.main.Main.main(SourceFile:123) [Main.class:?] > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_311] > at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_311] > at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_311] > at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_311] > at net.minecraft.launchwrapper.Launch.launch(Launch.java:135) [launchwrapper-1.12.jar:?] > at net.minecraft.launchwrapper.Launch.main(Launch.java:28) [launchwrapper-1.12.jar:?]

Some additional information:

These two adds together means that the /client/mcfixes/SkinDataReleaser in this mod will attempt to get rid of the skin textures while they might be still in use for another few frames, causing the error to be thrown. And once the error occur, there is no turning back.

This theory can be confirmed that a skin will only be broken for the current play session if it is actively rendered at the last few frames, which means being visible as:

Therefore, although I don't understand too much about texture related code, I may suggest two ways of fixing it. The first one is to delay the release of the skin (maybe make a timer on separate thread?). The second is to find and patch the renders that need skin textures.

KasumiNova commented 1 year ago

Same issue

Rongmario commented 1 year ago

Thanks for the investigation, haven't had time to check it out for myself. Fix coming up in next version; closing #114 in favour of this one.

Rongmario commented 1 year ago

Fixed in dev, now released as the world is about to set to null, on the same frame.