Mod compatibility issues

[kristofbolyai]

Describe the bug PlayerRenderMixin makes the mod crash when used along any other mod that redirects / injects into and cancels renderNameTag (a pose will be left hanging in PoseStack, making the checkPoseStack fail in LevelRenderer#renderLevel).

To Reproduce Install any mod that has the same mixin, like Artemis.

Expected behavior The mod should not crash.

Additional context https://hst.sh/mosavokuca

[kristofbolyai]

A "simple" way to fix this would be to do the same thing I did, in Artemis. Yes, it is still bad from compatibility standpoint, and mods might render text over each other. BUT, will the mod crash? No, it won't. I think that is a fair compromise to make.

[valoeghese]

Another alternative might be to change the mixin priority so that other mixins run first, if the cancelling of renderNameTag happens at head. That way they cancel before our code even runs

[valoeghese]

I changed the priority to 2000 in dev, which should make it run after other mixins (the default priority is 1000, and higher priority runs later. See https://github.com/2xsaiko/mixin-cheatsheet/blob/master/mixin-priority.md)

[kristofbolyai]

I changed the priority to 2000 in dev, which should make it run after other mixins (the default priority is 1000, and higher priority runs later. See https://github.com/2xsaiko/mixin-cheatsheet/blob/master/mixin-priority.md)

Well, that probably fixes it. Yeah, it is a weird fix, but probably the best we can do.

[kristofbolyai]

I changed the priority to 2000 in dev, which should make it run after other mixins (the default priority is 1000, and higher priority runs later. See https://github.com/2xsaiko/mixin-cheatsheet/blob/master/mixin-priority.md)

Although I am not sure if this solves the initial problems of return not being called. With the increased priority, you inject at head before us, but if we cancel the event, you still won't get to inject at return, making you leave a hanging pose. Maybe decrease the mixin priority to run after us, making it so you don't even add the pose if we already cancelled on head (although you have to test if mixins still get called after being cancelled).

[valoeghese]

that's what happens with higher priority. Greater priority makes it run later, according to that doc anyway. Don't worry: I also never remember which way around it is. I have to check the cheatsheet every time i mess with priority

[kristofbolyai]

that's what happens with higher priority. Greater priority makes it run later, according to that doc anyway. Don't worry: I also never remember which way around it is. I have to check the cheatsheet every time i mess with priority

It would be really weird that high priorities run later.

[valoeghese]

It isn't that weird if you consider that often the code that runs later gets the final say in what happens

[valoeghese]

Imagine you have three mixins that set the same field and don't cancel, all at the same location. It is the final mixin that gets the final say in setting the field

[valoeghese]

if it turns out that the doc is wrong and higher priority runs first, I'll change it again. I'll stick with the doc for now though

[kristofbolyai]

@valoeghese If you give me a jar, I can test it with Artemis (or you just put artemis as a mod in your dev env runs/mod folder)

[valoeghese]

If you still would like a jar to test lmk what version you're using

[kristofbolyai]

1.18.2. Have you had a release since this fix?

[valoeghese]

it should be fixed in the latest update sorry for not getting back to you sooner

[valoeghese]

closing this issue as it should have been fixed feel free to reopen if the issue persists