tr7zw / Exordium

There is no good reason to render the hotbar at 100+ fps
Other
231 stars 33 forks source link

MixinExtras is set up incorrectly #83

Closed LlamaLad7 closed 1 year ago

LlamaLad7 commented 1 year ago

You shade it without relocating it, causing https://github.com/QuiltMC/quilt-loader/issues/315. You also crucially need to mergeServiceFiles or it will break on modern forge. You also initialize it in a ClientModInitializer, that's way too late for most stuff. You don't seem to initialize it at all on forge. You need to do it in an IMixinConfigPlugin.

tr7zw commented 1 year ago

Ah, wasn't made aware that it needs to be relocated. Will look into it next week(I'm off to a festival, so not at home).

LlamaLad7 commented 1 year ago

The whole setup process is detailed in the readme.

tr7zw commented 1 year ago

Yea, I know the pain of people not shading from a different project, so much so that I added some code that will yell at the developer in case it's not setup correctly 😅.

tr7zw commented 1 year ago

Decided to use the include way, since I'm not planning a Forge version any time soon.

LlamaLad7 commented 1 year ago

You're still initializing it far too late.

tr7zw commented 1 year ago

You might want to rework the readme a bit. I just read

The final step after setting up your build script is to initialize MixinExtras. To do this, simply call MixinExtrasBootstrap.init(); somewhere suitably early in your program.

And took this as "as early as possible"(which would be onInitializeClient in my case). But then the next sentence says that it basically has to be PreLaunch(Something you usually never have to touch/the Fabric documentation recommends against).

LlamaLad7 commented 1 year ago

As early as possible is preLaunch. Main entrypoint are fired after much of the game is already initialized and so after lots of mixins are already applied.

tr7zw commented 1 year ago

Yes, but the wording makes it seem like the Client/Common entry points are fine(and it seems to work, at least initially). Most devs probably never had to touch the preLaunch point/are even aware that it's there.

LlamaLad7 commented 1 year ago

I think it's fine given I specifically say to use it. Regardless, you don't need to initialize it yourself anymore from the latest betas onwards.