Chocohead / OptiFabric

OptiFabric venturing out into the 1.16+ world
https://www.curseforge.com/minecraft/mc-mods/optifabric
Mozilla Public License 2.0
371 stars 106 forks source link

mod compact #1623

Open Climax-Arc opened 2 weeks ago

Climax-Arc commented 2 weeks ago

this mod link is crashing the game and talking to the mod dev we fig out its OptiFabric's mod compatibility issue. here are the logs https://mclo.gs/lBiuWw9

Dracconis commented 2 weeks ago

The problem is not with Optifabric or the mods listed in the error log. The problem is with the Prism Launcher. That is because using the exact same versions of the mods and fabric launcher, the MC Vanilla Launcher loads up the game just fine with no errors or crashes.

Climax-Arc commented 2 weeks ago

tried it with mc official launcher but same crash (while taking screenshot with f2) crashreport

how to reproduce crash game version: 1.20.1 mods: optifine, optifabric, screenshotview, fabric api open a single player world and take a screenshot with f2 (rip in advance D: )

Dracconis commented 2 weeks ago

So far it is the devs issue. I have tried 1.1.6 and 1.1.7 of said mod and game does not crash and allows the taking of screenshots. Testing the final versions. You need to talk to the dev so that they can figure out what they changed between version 1.1.7 of the mod to the later released versions. Because ALL of the versions past 1.1.7 crash the game, so it is not an Optifabric issue it is a screenshotviewer dev issue.

LGatodu47 commented 1 week ago

Hello, I'm the dev behind Screenshot Viewer.

So far it is the devs issue.

I'm not so sure about that: if we take a look at the crash report, we can clearly see it's a mixin injection exception, meaning that a mixin couldn't be processed correctly. This mixin is, as you might have guessed, from my mod, and the error we get says the following: Critical injection failure: @Inject annotation on screenshot_viewer$inject_saveScreenshotInner$lambda could not find any targets matching 'Lnet/minecraft/class_318;method_1661(Lnet/minecraft/class_1011;Ljava/io/File;Ljava/util/function/Consumer;)V' in net.minecraft.class_318. Let me bring some details: the crash basically states that it cannot find a lambda method (named method_1661) in the ScreenshotRecorder class (that's the name in yarn mappings). This lambda method is located inside method method_1662 (unmapped as saveScreenshotInner) and looks like this:

try {
    nativeImage.writeTo(file2);
    MutableText text = Text.literal(file2.getName()).formatted(Formatting.UNDERLINE).styled(style -> style.withClickEvent(new ClickEvent(ClickEvent.Action.OPEN_FILE, file2.getAbsolutePath())));
    // <- injection occurs here
    messageReceiver.accept(Text.translatable("screenshot.success", text));
} catch (Exception exception) {
    LOGGER.warn("Couldn't save screenshot", exception);
    messageReceiver.accept(Text.translatable("screenshot.failure", exception.getMessage()));
} finally {
    nativeImage.close();
}

The injection is necessary in order to make the redirection of screenshot links work (clicking on the printed message opens the screenshot manager with the taken screenshot focused). When I looked more into this repository, I found this in the README.MD file:

Optifine's replacement classes change the name of some lambada methods, so I take a good guess at the old name (using the original minecraft jar).

That's what lead me to think that this is specifically an Optifine issue. The name of the lambda method changes, that's why the mixin can't find it anymore. I believe the fix to this issue would be the addition of a compatibility mixin plugin in the me.modmuss50.optifabric.compat package.

Dracconis commented 1 week ago

Hello, I'm the dev behind Screenshot Viewer.

So far it is the devs issue.

I'm not so sure about that: if we take a look at the crash report, we can clearly see it's a mixin injection exception, meaning that a mixin couldn't be processed correctly. This mixin is, as you might have guessed, from my mod, and the error we get says the following: Critical injection failure: @Inject annotation on screenshot_viewer$inject_saveScreenshotInner$lambda could not find any targets matching 'Lnet/minecraft/class_318;method_1661(Lnet/minecraft/class_1011;Ljava/io/File;Ljava/util/function/Consumer;)V' in net.minecraft.class_318. Let me bring some details: the crash basically states that it cannot find a lambda method (named method_1661) in the ScreenshotRecorder class (that's the name in yarn mappings). This lambda method is located inside method method_1662 (unmapped as saveScreenshotInner) and looks like this:

try {
    nativeImage.writeTo(file2);
    MutableText text = Text.literal(file2.getName()).formatted(Formatting.UNDERLINE).styled(style -> style.withClickEvent(new ClickEvent(ClickEvent.Action.OPEN_FILE, file2.getAbsolutePath())));
    // <- injection occurs here
    messageReceiver.accept(Text.translatable("screenshot.success", text));
} catch (Exception exception) {
    LOGGER.warn("Couldn't save screenshot", exception);
    messageReceiver.accept(Text.translatable("screenshot.failure", exception.getMessage()));
} finally {
    nativeImage.close();
}

The injection is necessary in order to make the redirection of screenshot links work (clicking on the printed message opens the screenshot manager with the taken screenshot focused). When I looked more into this repository, I found this in the README.MD file:

Optifine's replacement classes change the name of some lambada methods, so I take a good guess at the old name (using the original minecraft jar).

That's what lead me to think that this is specifically an Optifine issue. The name of the lambda method changes, that's why the mixin can't find it anymore. I believe the fix to this issue would be the addition of a compatibility mixin plugin in the me.modmuss50.optifabric.compat package.

So please help me to understand how it is an optifine issue when as I said I used 1.16 and 1.17 of the mod with the exact same setup as the op and it worked just fine, but in changing the mod to versions 1.2.0, 1.2.1, and 1.3.1 caused the game to crash? What that tells me is that something changed within the mod between version 1.1.7 and 1.2.0+ that wasn't there before and is now causing the crash. Just trying to understand.

LGatodu47 commented 1 week ago

The fact that the games crashes starting from this version is caused by the addition of a Mixin class for the screenshot link redirection when taking a screenshot. image It is an Optifine issue because as I stated earlier Optifine modifies the lambda method I inject code in. My mod works perfectly fine with other optimization mods, but not with Optifine, and it is a well known fact that Optifine does some hacky stuff to the vanilla game in order to make its features work. I guess that's the main reason the Mod Compat label exists for issues with Optifabric.

Dracconis commented 1 week ago

The fact that the games crashes starting from this version is caused by the addition of a Mixin class for the screenshot link redirection when taking a screenshot. image It is an Optifine issue because as I stated earlier Optifine modifies the lambda method I inject code in. My mod works perfectly fine with other optimization mods, but not with Optifine, and it is a well known fact that Optifine does some hacky stuff to the vanilla game in order to make its features work. I guess that's the main reason the Mod Compat label exists for issues with Optifabric.

Then please explain why the additional code, that is now causing the game to crash, is needed when it worked just fine in prev versions? I guess what I am asking is "If it ain't broke, why fix it?"

LGatodu47 commented 1 week ago

That code adds a feature, it does not fixes anything, nothing is broken. Are you asking me to remove a feature? And again the game crashes only with Optifine, not in vanilla. I already explained above why the issue might come from Optifine.