Skip to content

Instantly share code, notes, and snippets.

@Earthcomputer

Earthcomputer/sideonly-rework.md

Created August 23, 2020 20:25
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Earthcomputer/c9aded5e879fbad6b84ab513e121a16c to your computer and use it in GitHub Desktop.
Save Earthcomputer/c9aded5e879fbad6b84ab513e121a16c to your computer and use it in GitHub Desktop.
Download ZIP
Rework of SideOnly inspections
Raw
sideonly-rework.md

Rework of SideOnly inspections

MinecraftDev's SideOnly inspections gave an error when you tried to access client-only code from common code, helping to prevent a common cause of crashes on the dedicated server.

Motivation

While they were very useful in the right circumstances, the old SideOnly inspections have a number of flaws which ultimately meant they didn't see the widespread use that was originally hoped for. The problems I'm about to outline led to them being disabled by default, so not everyone even knew they existed. While I was maintaining a fork of MinecraftDev with Fabric support (now merged upstream), I randomly came across them when scrolling through my inspections list, hacked together a fix so it would work with Fabric's @Environment annotation, and enabled them by default in my fork. This was a partial success, but still shared some of the issues as before. So, what are these issues? They can be broken down into 5 key problems with the old inspections:

  1. Litter. In order to use these inspections properly, you had to litter your own code with @SideOnly. Some people don't like putting annotations on their code for some reason, so this discouraged them from using the inspections.
  2. Side effects. There is another effect of putting @SideOnly on mod code which dissuaded a lot of people. FML stripped classes, fields and methods with this annotation, causing potentially unexpected behaviour at runtime, unless you really know what you're doing. A notable example is initializing a value to a sided field could crash because the code that actually assigns the field isn't marked with the annotation. This led to several experienced modders recommending against its use.
  3. No control/data flow analysis. Sometimes, it's obvious that a certain block of code will only run on the client, due to a world.isRemote/world.isClient check or similar. Some modders argued that references to client-side code should be allowed in these areas. While this was not actually safe in all cases (it's possible to trip the verifier as the class loads, even if you don't actually run the code), we can definitely do better than disallowing it in all cases, depending also on the best practices of the modding platform you're on.
  4. Error level. When enabled, these inspections display as an error by default, rather than a warning. This irritated some, especially in cases when they knew the code was valid. This broke the principle that code that is definitely wrong should be an error, code that is probably wrong should be a warning.
  5. Not extensible. The old inspection only worked for the @SideOnly annotation from 1.12 Forge, and works for neither Forge's newer @OnlyIn annotation, Fabric's @Environment annotation, nor indeed any other annotations for the same purpose. This means that even if you do turn on the inspection nowadays, it probably won't do anything (unless you are still on ancient versions, in which case update). Moreover, the code for the inspections isn't written in a way that makes it easy to support other annotations.

@CheckEnv(Env.CLIENT)

The first thing we have done to address these problems is introduce another annotation. Why another? We already have three! We believe this annotation has a few advantages for use in mod code over @OnlyIn or @Environment:

  • No mod code is stripped at runtime, eliminating problem 2.
  • The annotation can go on packages, partially eliminating problem 1 because you don't have to put the annotation on every class in that client package you have.
  • Since there's no runtime stripping, code annotated with this annotation can safely be referenced from common code as long as it isn't executed, eliminating the concern outlined in problem 3.

The annotation is available on maven central, so to use it, you only have to add this to your build.gradle:

dependencies {
	compileOnly 'com.demonwav.mcdev:annotations:1.0'
}

If possible, MinecraftDev may be able to add this dependency as part of a quick fix.

Inference

To reduce the number of places you need to put annotations on your code further, there are a number of situations where MinecraftDev can infer the sidedness of a member without you having to explicitly annotate it:

  • In a class where any of the superclass or interfaces is sided, that side is inherited.
  • In an inner class (including anonymous classes) where the outer class is sided, that side is inherited.
  • In a mixin targeting a sided class, the side of the target class is inherited.
  • In a lambda expression inside a net.minecraftforge.fml.DistExecutor call, the context side can usually be inferred from the method call expression.
  • If a Fabric mod states that it's for a specific environment inside its fabric.mod.json, all its classes are implicitly of that side.
  • Methods that override or implement a hard-sided or soft-sided method are implicitly soft-sided with this side.
  • Lambda expressions are implicitly soft-sided if the surrounding method is either soft-sided or hard-sided. This accurately reflects the synthetic method in the bytecode, which will not have a hard-sided annotation.

Note that some of these inferences can make a mod class implicitly hard-sideonly (see below). This does not mean that the class will be stripped in the same way as if it were explicitly hard-sideonly, but can still crash even if referenced in some cases, in the same way that hard-sideonly classes can.

Hard- and soft-sidedness

With the new annotation comes the distinction between "hard-sidedness" and "soft-sidedness":

  • Hard-sided classes, fields and methods are either stripped at runtime or not present in the first place. Members annotated with @SideOnly, @OnlyIn or @Environment are hard-sided.
  • Soft-sided classes, fields and methods are present at runtime on both sides, however should not be accessed on the wrong side. Members annotated with @CheckEnv are soft-sided. The only effect of being soft-sided is to control the new SideOnly inspection (and possibly other static analysis tools in the future).
  • Common classes, fields and methods are those that are available and can be called on both sides. These are the members that have no annotation, and have no inferred sidedness annotation by MinecraftDev either (see above).

The access rules are as follows, in addition to being symmetrical between physical client and physical server:

  • Hard-clientside classes, fields and methods can access hard-clientside, soft-clientside and common members.
  • Soft-clientside classes can access hard-clientside, soft-clientside and common members.
  • Soft-clientside fields can access soft-clientside and common members, and hard-clientside members if the containing class is also soft- or hard-clientside.
  • Soft-clientside methods can access soft-clientside and common members, and hard-clientside members if the containing class is also soft- or hard-clientside. Additionally, soft-clientside methods can access hard-clientside classes in their parameter types, return type and exception types.
  • Common classes and fields can access common members only.
  • Common methods can access common members, and soft-clientside members in blocks of code that can be proven with dataflow analysis to only run on the client.

Rewrite

The SideOnly inspection has been rewritten.

  • To the extent that is possible with the current IntelliJ version, control flow and data flow analysis is used to determine whether a block of code could be executed on the client or the dedicated server. At time of writing (IntelliJ 2019.3), the only thing that is possible to check is the world.isRemote/world.isClient field, although this will likely change in the future.
  • The default severity of the inspection has been lowered from error to warning, to reflect the UI guideline mentioned in problem 4.
  • The implementation is more flexible to allow support for other annotations in the future.

Other inspections

Along with the new SideOnly inspection, a few other related inspections have been introduced:

  • An inspection which warns about any usage of @OnlyIn or @Environment.
  • An inspection which warns about incorrect usages of DistExecutor.

Recommended practices

Overall, this inspection doesn't change the best practices in modding. You should still logically separate your client code from your common code.

Forge

In Forge, the preferred way to conditionally execute code on a particular distribution remains to use DistExecutor, and not with an if statement. Direct use of @OnlyIn remains highly frowned upon in all cases, and @CheckEnv should be used instead. Usage of @OnlyIn for sided interface stripping (with the _interface option) should also be avoided, and has mostly been replaced with equivalent APIs in Forge. If you are creating a client-side-only mod with Forge, it is recommended that you disable the SideOnly inspection for this project only (use project settings as opposed to IDE settings).

Fabric

If you need to conditionally execute code for a particular environment, wrap a method call to a soft-sideonly in an if statement, and call the hard-sideonly code inside that method. Do not reference the hard-sideonly code directly. Direct use of @Environment is frowned upon, and @CheckEnv should be used instead. Usage of @EnvironmentInterface for sided interface stripping is still valid. If you are creating a client-side-only mod with Fabric, you can change the environment option in your fabric.mod.json, MinecraftDev will detect that and allow you to call client-side methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment