Open TelepathicGrunt opened 5 months ago
Yes
As long as we encourage modders to not properly isolate their client-specific code, I think it would be premature to enforce removal of @OnlyIn
.
What would proper isolation of client-specific code entail?
@sciwhiz12
if (FMLEnvironment.dist == Dist.CLIENT) {
ModClientClass.doStuff();
}
or
if (FMLEnvironment.dist.isClient()) {
ModClientClass.doStuff();
}
From what I understand, OnlyIn actually has zero effect on mod's own code. No stripping is done. Thus keeping the annotation just causes confusion for new modders as they think it does something instead of writing properly safe code. If a modder is only used for marking what code is clientside for organization, the same goal can be achieved by just doing a comment like //CLIENTSIDED
or so.
We already suggest proper isolation of client code. See https://github.com/neoforged/MergeTool/blob/main/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java
@OnlyIn
does have an effect on mod code, but it should not be used by mods whatsoever.
The docs in Dist require the modder to already know about Dist which then they are already were going to use it properly. Only really strong way to kill off OnlyIn within modder code is to stop allowing it in modder code. otherwise people are just going to keep copying bad code from other mods or bad tutorials and nothing changes.
As a possible solution to the answer of how people are going to learn about avoiding @OnlyIn
once an error or warning is thrown in dev as proposed solution, we could create a shortlink under https://links.neoforged.net/ to direct people to an appropriate help page (as we do for the early loading screen errors).
Also, I'm interested to hear what @Technici4n considers as proper isolation of client-specific code, as well as why he considers us to be currently encouraging against it.
@OnlyIn
has the same effect on mod code as it has on vanilla code:
As far as I know from previous discussions, what Tech is referring to is something like Fabric Loom's split sourcesets, which would enforce the isolation of client-only code. While it's theoretically a good solution in the long term, I personally don't think such measures are needed to go ahead with disallowing @OnlyIn
in mod code.
With https://github.com/neoforged/NeoForge/pull/929 released, I believe modders now can define two entry points. One for client and one for normal. So another way of doing properly isolation of client code
Making @OnlyIn
skip mod classes would also technically provide a speed boost as it can skip searching for OnlyIn for the majority of classes:
@Override
public EnumSet<Phase> handlesClass(Type classType, boolean isEmpty) {
- return isEmpty ? NAY : YAY;
+ return isEmpty || !classType.getInternalName().startsWith("net/minecraft/") ? NAY : YAY;
}
I think we should do that in 1.21 and until then crash uses of it in mod code.
Too many people are still using OnlyIn in their own mod’s code. Time to bring down the hammer