makamys / Neodymium

Reimplements chunk rendering using modern OpenGL features to improve performance (1.7.10)
Other
115 stars 10 forks source link

FastCraft compat is not getting applied with non-MultiMC launchers + LiteLoader mod #54

Closed xJon closed 5 months ago

xJon commented 5 months ago

(Using The 1.7.10 Pack) Upon first entering the world a lot of chunks in front of the player don't render: javaw_oWT8GsScEV

F3 + A appear to resolve it but it seems that there is always at least one unloaded chunk when the player turns backwards: javaw_xboFrNhieW

I assume this is due to an incompatibility of some sorts, do you have any clues? Using FastCraft 1.25, no Optifine.

makamys commented 5 months ago

FastCraft does sound like a likely culprit, it would be worth a check if it still happens if you remove it. Neodymium is supposed to be compatible with it as of 0.1.7, but maybe something still isn't working. I might be able to say more if you post a log.

By the way, since the pack doesn't have OptiFine, you might be better off using 1.23 instead of 1.25, since the latter had some optimizations removed for compat with OF shaders, which won't do you much good if OF isn't there in the first place.

xJon commented 5 months ago

I can confirm removing Fastcraft prevents this issue from happening. Here's a game log with Fastcraft, with the issue present: https://mclo.gs/i8wS1Zq

Regarding FastCraft versions, I have been following your recommendations over the Gist, but from what I understood 1.25 offers inferior performance only when Optifine E7 is present:

Performance may be better with an Optifine version that doesn't include Shadersmod or without Optifine.

Therefore version 1.25 is more versatile - I did not see testing showing OF D6 + FC 1.23 is superior to OF D6 + FC 1.25.

xJon commented 5 months ago

(For reference, this issue still occurs with Neodymium 0.1.9 and enableVanillaChunkMeshes=true)

makamys commented 5 months ago

Hm, I'm not seeing the FastCraft is present, applying hack to forcingly enable FastCraft's OptiFine compat line in the log, but I do see it when I run the pack myself (0.10.11 with Nd 0.2.3 added). Maybe mclo.gs is being misleading, can you upload fml-client-latest.log directly?

Therefore version 1.25 is more versatile - I did not see testing showing OF D6 + FC 1.23 is superior to OF D6 + FC 1.25.

That's a good point. It's reasonable to assume 1.25 is fine when OF is not present then.

xJon commented 5 months ago

Strange, I also can't find that line (nor fml-client-latest.log, as it's named latest.log). Nevertheless as you were able to reproduce the issue, is that still necessary?

makamys commented 5 months ago

I wasn't able to reproduce it, the presence of the line indicates that the compat is getting triggered as expected. Chunks also appear to be loading fine for me in-game.

Looks like the Technic Launcher has some logging quirks then... You could try running the pack using MultiMC/Poly/Prism, or adding NotEnoughVerbosity which should force the default logger config to be used.

xJon commented 5 months ago

Adding NotEnoughVerbosity did not change anything unfortunately. Using the Prism Launcher though, I can see both a fml-client-latest.log file and the FastCraft reference line in it - and chunks appear to render fine. What's happening here?

xJon commented 5 months ago

I thought it could be due to Technic defaulting to Mojang Java runtimes to avoid some incompatibilities, but overriding that and using Java 1.8.0_401 does not resolve this.

makamys commented 5 months ago

Now this is weird. Normally the mixin environment goes through three phases: PREINIT, INIT, then DEFAULT. Neodymium applies FC compat when its mixin plugin is invoked in the INIT phase.

I attached a debugger to the pack running in Technic Launcher, and the INIT phase is getting skipped! It just goes straight from PREINIT to DEFAULT. I guess I could work around it by having my code try to run in DEFAULT as well, but I'll dig into why this is happening, since this is likely to affect other mods as well (CoreTweaks fortunately doesn't have any INIT mixins).

And yeah, I tried running with u51 when trying to reproduce it in Prism too.

xJon commented 5 months ago

Perhaps you can double check, but I was able to reproduce this also using the CurseForge launcher.

makamys commented 5 months ago

So apparently Mixin detects when the INIT phase starts by looking for Forge's "Validating minecraft" debug-level log message. So this issue and the logging weirdness are one and the same. 🤦 I'm guessing ATLauncher is also affected then.

xJon commented 5 months ago

Not the most sophisticated detection system I guess.

makamys commented 5 months ago

Mixin has a few cursed hacks like this. To be fair, it's not really possible to cleanly hook into the loading process the way it needs to without adding support from Forge's/LaunchWrapper's side. (Though why that wasn't done is beyond me; Mixin started in 2015 and 1.7.10 Forge was still being maintained at that point. I also don't get why it doesn't just hook into the class loading of cpw.mods.fml.common.Loader which happens right after that print. Maybe it's for better cross-version compat? Or maybe switching phases during class loading isn't possible.)

Anyway, I figured out the cause of the logging weirdness. It's because Technic Launcher puts the LaunchWrapper jar before the the Forge jar on the classpath, causing its log config to be used. This seems like an unintentional bug to me (a similar thing happened even to lwjgl3ify once), so I'll probably try to implement a fix in CoreTweaks or UniMixins.

xJon commented 5 months ago

Awesome, thank you for finding out! I wonder what else has been negatively affected by this. Will be awaiting your fix.

makamys commented 5 months ago

Probably not many things; searching for INIT mixins on the entirety of GitHub only returns 46 results. Out of these, the only 1.7.10 mods are Neodymium, FalseTweaks and D-Tools.

xJon commented 5 months ago

I assume this would also be affecting 1.12.2?

makamys commented 5 months ago

Seems so.

makamys commented 5 months ago

UniMixins 0.1.17 has been released which should fix the issue on every launcher. After more research the cause turned out to be very specific: LiteLoader has to be present as a Forge mod. Otherwise the launchers worked fine because Mixin already had code that forces that log message to get printed, it's just LiteLoader that interfered with it since it also does some shenanigans to the logger.

I also reported the classpath ordering issue to Technic.

xJon commented 5 months ago

Thank you for looking into this @makamys - your fix works perfectly. Closing this