Add Hesperus compatibility #51
Conversation
However, the original version uses MixinBootstrap as Mixin support, while REID uses MixinBooter as Mixin support, which will cause conflicts. MixinBooter must be used for the patch to take effect.
|
I would like to mention some things and suggest that compatibility with Phosphor is not necessary and shouldn't be supported from our side. First of all, the #38 "incompatibility" doesn't really have anything to do with REID, it's a matter of Phosphor being shaded in the official Aether II mod. However, there is actually a fork of Aether II here on CF that does not include Phosphor so users can use whatever lighting mod (Hesperus, Alfheim) they would like. This was discussed in an issue by the same poster in CleanroomMC/MixinBooter#73. I think PhosphorCrashFix is an alternative method if someone really wishes to use the original Aether II mod. From what I understand, looking briefly at how you combined REID and Phosphor mixins, you are saying this would allow using MixinBoostrap to load REID. However, this should not be supported because as of 2.1.0 I refactored a lot of mixins to rely on MixinExtras annotations, and as far as I know, MixinExtras is only present if you use MixinBooter. There is also a certain minimum version of MixinExtras that is necessary, which is why the minimum version of MixinBooter is specified. In any case, without meaning to sound rude, I don't think this PR is necessary. At worst, if someone really wants to use the original Phosphor mod, they can try launching with PhosphorCrashFix. Feel free to explain otherwise if I missed anything. |
You read it wrong, this does not require us to use MixinBoostrap to load REID, but requires using the Phosphor branch that supports MixinBooter After all we can't support an old Mixin framework |
|
@jchung01 The patch just provides a MixinChunk class that supports both Phosphor and REID. For example, REID's INewChunk and Phosphor's ILightingEngine When Phosphor is detected, the default ChunkMixin located at REID and Phosphor will be disabled and REID will instead load the variant located under the modsupport package That's why I left a link to Hesperus in the PR message, because it supports MixinBooter |
|
So you are saying this is to support specifically the old, original version of Phosphor? Because Hesperus loads fine alongside REID (using MixinBooter). Even then, adding Phosphor Crash Fix lets the original Phosphor mod load fine with whatever mixin mods, including REID. If there are any specific mixins that aren't loaded when REID and Phosphor/Hesperus are present, then patch those specific ones. No need to rewrite mixins that will be applied independently from each mod. |
It seems that I just got the wrong information when debugging. Even without patching, Hesperus can load normally. I first thought it was a crash caused by patch failure, but it doesn't seem to be the case. However, the existing REID should theoretically invalidate the Hesperus patch, because Mixin has a sequence, and REID should be loaded first than Hesperus. Now it can be considered that this PR is just to make Hesperus work |
|
@jchung01 The most recent commit records are just for modifying the commit message |
|
Load order should have no effect here, REID and Phosphor modify different methods and/or targets within the method. I can verify there are no mixin injection errors: this is a debug.log with just Hesperus, REID 2.1.0, and MixinBooter 8.6 https://mclo.gs/FesMD3a (I am using a build off my most recent commit past 2.1.0, but that shouldn't matter). Unless you know of a specific mixin injection that fails, there should be no issue with REID and Hesperus. You can even run the environment with |
In this case, close this PR |
However, the original version uses MixinBootstrap as Mixin support, while REID uses MixinBooter as Mixin support, which will cause conflicts. MixinBooter must be used for the patch to take effect.
MixinBooter Fork
Fixes #38