Logic in Mixin injector prevents extension by other mods

[Crendgrim]

I really like this mod. I've written my own mod, Auto HUD, that (among other things) hides some status items and moves them individually. In order to have the timer text overlay stay consistent, I need to make small changes your render routine. However, as that routine is in an @Inject mixin method, that is impossible as far as I can see: everything I read points to Mixins-into-Mixins not being supported. The only solution I found was to bundle your mod, but that feels dirty and I'd rather keep it separate.

Would it be possible for you to split the render logic into a different file (or at least method)? Then I could properly mixin into your mod instead of having to bundle it with mine.

[magicus]

@Crendgrim Apologies for not responding to this sooner. My intention is that Status Effect Timer should work smoothly with other mods. You are correct that my current approach do not work with mods that change order or hides individual badges.

Mixin combination is not always easy. I'm hesitant on creating workarounds/fixes targeted for a specific mod interaction, but would rather try to find a solution that is as generic as possible.

As far as I know, there is no problem per se with making multiple @Inject mixins into a single method, neither from the same mod or from two or more different mods.

The problem here is that vanilla code basically looks like this:

            for(StatusEffectInstance statusEffectInstance : effects) {
                if (statusEffectInstance.shouldShowIcon()) {
                  // calculate changes in x and y
                  // draw icon
                  // <-- Status Effect Timer additions are needed here
                }
              }

Since the "draw icon" part is not a separate method call, this made injection at the "correct" point more of a hassle, so I opted to instead replicate the x/y calculation, and do all text overlay rendering in a separate function, injected at the end.

To support your use case, I think the proper solution is to actually go through the hassle and craft a mixin descriptor to inject the code in the correct spot, per status icon. It will definitely make it more compatible with other mods. My main worry about this is that it will also make the mod more fragile, and prone to breaking in future versions of Minecraft. But at the end of the day, that is my headache, not yours.

For this to work though, the requirement is that your mod too is just modifying the relevant parts of renderStatusEffectOverlay, and not replacing it wholesale. I located the code to your mod at https://github.com/Crendgrim/AutoHUD but have not had the time to check it yet.

[magicus]

Now I have looked at your code. It seems likely that a change to Status Effect Timer as described above will work with your mod. I'll go ahead with implementing that solution.

I see that you have a lot of mixins which interacts with other mixins. I highly recommend that you take a look at https://github.com/LlamaLad7/MixinExtras. I have successfully used it in another mod project, to get more "composable" mixins. For instance, by using @WrapWithCondition instead of @Redirect multiple mods can add conditions, while all redirects fight with one another.

[Crendgrim]

Ah, nice to hear you're back to working on this. I'll drop my fork of StatusEffectTimer soon, then.

To support your use case, I think the proper solution is to actually go through the hassle and craft a mixin descriptor to inject the code in the correct spot, per status icon. It will definitely make it more compatible with other mods. My main worry about this is that it will also make the mod more fragile, and prone to breaking in future versions of Minecraft. But at the end of the day, that is my headache, not yours.

Alternatively, it is possible to mixin into other mods' classes. This does not extend to Mixins themselves, however, which was my original issue. But if you moved the render logic from the mixin class itself to a dedicated render class (and just called that in the mixin), I could conditionally mixin into StatusEffectTimer to add my matrix shifts. Just as an alternative solution that'll reduce workload for you.

If you do end up wanting to rewrite the mixin, to save you a little bit of hassle, I can tell you that the anonymous Runnable lambda to actually draw the status effect icons is mapped as method_18620 in intermediary.

I highly recommend that you take a look at https://github.com/LlamaLad7/MixinExtras.

I am making heavy use of @WrapWithCondition already! There is actually only a single stray @Redirect that I for some reason missed, which is fixed already in my 1.20.5 draft. I can easily backport that change if it is necessary.

[magicus]

I implemented an injection per individual status effect badge in bea6d05. I think that should resolve the issue, but I have not tested. If you do, please let me know :)

I might have looked at an older version of your code base, sorry if I draw hasty conclusions.

[Crendgrim]

I will still have to mixin into your method to adjust the matrices. I think technically it works to mixin to a non-mixin method in a mixin class, even if it's not officially supported. I'll have to test.

[magicus]

Oh, now I think I understand better now. That was why you wanted me to move the functionality to an external class, so you could add a mixin to my code. A mixin into a mixin is unlikely to work -- the mixin is not "real" code but just a clever way of creating the byte code that will be injected.

sigh I was very pleased about this mod being just a single file. But I guess code golf reasons are not as valid as good interoperability.

I'll try to fix it, but another day...