Make mod’s own use of OnlyIn annotation throw warning or crash in dev

[TelepathicGrunt]

Too many people are still using OnlyIn in their own mod’s code. Time to bring down the hammer

[Gabwasnt]

Yes

[Technici4n]

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.

[sciwhiz12]

What would proper isolation of client-specific code entail?

[TelepathicGrunt]

@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.

[Shadows-of-Fire]

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.

[TelepathicGrunt]

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.

[sciwhiz12]

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.

[XFactHD]

@OnlyIn has the same effect on mod code as it has on vanilla code:

• Annotated classes will cause an exception when loaded on the wrong physical side
• Annotated fields and methods will be stripped from the bytecode if the containing class is loaded on the wrong physical side (field initializers will not be stripped, which makes use of the annotation on fields especially dangerous)

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.

[TelepathicGrunt]

With #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

[Matyrobbrt]

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.