Environment stripping improvements - Lambda stripping

[LambdAurora]

Sided lambdas have been quite terrible to modders, every month at least one modder falls under its wrath.

But what is actually the issue?

Let me pull up a very simple example code:

@Environment(EnvType.CLIENT)
public void hello() {
    this.hi(client -> client.player.sendMessage(new LiteralText("Hi!"), false));
}

@Environment(EnvType.CLIENT)
public void hi(ClientConsumer toDo) {
    toDo.consume(MinecraftClient.getInstance());
}

@Environment(EnvType.CLIENT)
public interface ClientConsumer {
    void consume(MinecraftClient client);
}

And let's consider this is in a common class, when loaded on a dedicated server 2 methods will be stripped: hello and hi. BUT WAIT! There isn't just 2 methods, there's 3! The lambda is another method too, but wait, it doesn't have the annotation, meaning it won't get stripped.

When loading the class, the server will crash due to ClientConsumer not existing, which will be quite unintuitive to modders unaware of this specific behavior.

The loader should strip lambda methods that are generated from a stripped method, it's not trivial but it would resolve a lot of seemingly "random" crashes.

Now there's the argument of "you should not use sided methods, you should separate your client code"! Which I partially agree, in some cases separating a feature to the opposite part of the codebase and have all the remaining code in another part will make understanding and maintaining the feature more difficult, sided methods are very practical in that case, they should not be the "always" go-to, but they should not be straight out rejected from designs. Fixing this will make codebases significantly more stable when those kind of methods are involved.

[AlexIIL]

When loading the class, the server will crash due to ClientConsumer not existing

I can't reproduce that with the example given - even if I remove the @Environment(EnvType.CLIENT) from both the hello and hi methods. Can you link to a crash report where this issue actually occurs?

(At least - I'm assuming this has the same behaviour in a development environment as a production environment - I can get it to crash if I then call Class<?> c = [ClassName].class; c.getMethods())

[AlexIIL]

Actually, I suppose I was being a bit pedantic with my previous comment - the example given above can actually be loaded safely on the server, but the following can't:

public void hello() {
    this.hi(client -> {
        ClientWorld cw = client.world;
        World w = cw; // This requires the java verifier loads both ClientWorld and World to make sure you can perform this implicit upcast
        w.random.nextBoolean();
    });
}

[AlexIIL]

This has now been implemented as of 0.17.5-beta.8 - although a full release should follow in a couple of days.

[AlexIIL]

Okay, now released in 0.17.5!