From 490a86155a5eb90a3f01a68ca3aedb3af05d46ed Mon Sep 17 00:00:00 2001 From: Brennan Ward <3682588+Shadows-of-Fire@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:58:11 -0700 Subject: [PATCH 1/4] Side-safety docs --- .../net/neoforged/api/distmarker/Dist.java | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java index 958282b..dabdd9b 100644 --- a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java +++ b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java @@ -20,7 +20,7 @@ package net.neoforged.api.distmarker; /** - * A distribution of the minecraft game. There are two common distributions, and though + * A physical distribution of Minecraft. There are two common distributions, and though * much code is common between them, there are some specific pieces that are only present * in one or the other. * + *

+ * When working with Dist-specific code, it is important to guard invocations such that + * classes invalid for the current Dist are not loaded. + *

+ * This is done by obeying the following rules:
+ * 1. All Dist-specific code must go in a separate class.
+ * 2. All accesses to the Dist-specific class must be guarded by a Dist check. + *

+ * Following these rules ensures that a Dist-induced classloading error will never occur. + *

+ * An example of these rules in action is shown below: + *

+ *

+ * // Class which accesses code that is only present in Dist.CLIENT
+ * public class ClientOnlyThings
+ * {
+ *     public static boolean isClientSingleplayer()
+ *     {
+ *         return Minecraft.getInstance().isSingleplayer();
+ *     }
+ * }
+ * 
+ * // Class which is loaded on both Dists.
+ * public class SharedClass 
+ * {
+ *     // Returns true if the client is playing singleplayer.
+ *     // Returns false if executed on the server (will never crash).
+ *     public static boolean isClientSingleplayer()
+ *     {
+ *         if(currentDist.isClient())
+ *         {
+ *             return ClientOnlyThings.isClientSingleplayer();
+ *         }
+ *         return false;
+ *     }
+ * }
+ * 
+ * + * In this example, any code can now call SharedClass.isClientSingleplayer() without guarding.
+ * However, only code that is specific to Dist.CLIENT may call ClientOnlyThings.isClientSingleplayer(). + * + * @apiNote How to access the current Dist will depend on the project. When using FML, it is in FMLEnvironment.dist */ public enum Dist { From f56d28e743845031823897a3ae74852e598f1606 Mon Sep 17 00:00:00 2001 From: Brennan Ward <3682588+Shadows-of-Fire@users.noreply.github.com> Date: Tue, 17 Oct 2023 01:41:35 -0700 Subject: [PATCH 2/4] more details and clarification --- .../net/neoforged/api/distmarker/Dist.java | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java index dabdd9b..9691c40 100644 --- a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java +++ b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java @@ -30,45 +30,63 @@ * it contains a server, which can simulate the world and communicates via network. * *

- * When working with Dist-specific code, it is important to guard invocations such that - * classes invalid for the current Dist are not loaded. + * Code that is only present in a specific dist is referred to as "dist-specific code", + * and will be marked with {@link OnlyIn}. Code that is always available is referred to + * as "shared code" (or "common code"). It is also common to refer to dist-specific code + * as "client-only code" or "server-only code" to indicate the dist. *

- * This is done by obeying the following rules:
- * 1. All Dist-specific code must go in a separate class.
- * 2. All accesses to the Dist-specific class must be guarded by a Dist check. + * To prevent classloading errors, it is important to ensure that {@link OnlyIn} elements are + * only loaded if their designated dist is the same as the executing dist. *

- * Following these rules ensures that a Dist-induced classloading error will never occur. + * This is done by obeying the following rules: + *

    + *
  1. All dist-specific code must go in a separate class, called a "bouncer" class.
  2. + *
  3. All accesses to the bouncer class must be guarded by a dist check.
  4. + *
*

* An example of these rules in action is shown below: *

- *

- * // Class which accesses code that is only present in Dist.CLIENT
- * public class ClientOnlyThings
+ * 
{@code
+ * // Client-only bouncer class which accesses client-only code. 
+ * // Methods in this class will fail verification if invoked on the wrong dist.
+ * // However, the class can still be referenced from shared code as long as no methods are invoked unconditionally.
+ * public class ClientBouncer
  * {
  *     public static boolean isClientSingleplayer()
  *     {
+ *         // Minecraft is @OnlyIn(Dist.CLIENT)
  *         return Minecraft.getInstance().isSingleplayer();
  *     }
  * }
  * 
- * // Class which is loaded on both Dists.
+ * // Class which may be loaded on either dist.
  * public class SharedClass 
  * {
  *     // Returns true if the client is playing singleplayer.
- *     // Returns false if executed on the server (will never crash).
+ *     // Returns false if executed on the server. Will never crash.
  *     public static boolean isClientSingleplayer()
  *     {
  *         if(currentDist.isClient())
  *         {
- *             return ClientOnlyThings.isClientSingleplayer();
+ *             return ClientBouncer.isClientSingleplayer();
  *         }
  *         return false;
  *     }
  * }
- * 
+ * }
* - * In this example, any code can now call SharedClass.isClientSingleplayer() without guarding.
- * However, only code that is specific to Dist.CLIENT may call ClientOnlyThings.isClientSingleplayer(). + * In this example, any code can now call {@code SharedClass.isClientSingleplayer()} without guarding. + *

+ * The specifics of why this works relies on how the class verifier operates. When the shared method + * is invoked for the first time, the following steps occur: + *

    + *
  1. The class {@code SharedClass} is loaded, if it was not previously accessed
  2. + *
  3. The method {@code SharedClass.isClientSingleplayer} is loaded and verified.
  4. + *
  5. It is checked that {@code ClientBouncer} exists, but the content of its methods are not yet verified.
  6. + *
  7. If running on {@link Dist#CLIENT}, then {@code ClientBouncer.isClientSingleplayer} will be verified.
  8. + *
+ * The final step causes the verifier to resolve a reference to {@code Minecraft}, which is client-only code. + * If this step happend on {@link Dist#DEDICATED_SERVER} (i.e. if the dist check were omitted), the game would crash. * * @apiNote How to access the current Dist will depend on the project. When using FML, it is in FMLEnvironment.dist */ From 3f825325185acd8dbe5ec19049493096b250e491 Mon Sep 17 00:00:00 2001 From: Brennan Ward Date: Sun, 12 Nov 2023 01:58:24 -0500 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: sciwhiz12 --- src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java index 9691c40..76f5d23 100644 --- a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java +++ b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java @@ -66,7 +66,7 @@ * // Returns false if executed on the server. Will never crash. * public static boolean isClientSingleplayer() * { - * if(currentDist.isClient()) + * if (currentDist.isClient()) * { * return ClientBouncer.isClientSingleplayer(); * } @@ -86,7 +86,7 @@ *
  • If running on {@link Dist#CLIENT}, then {@code ClientBouncer.isClientSingleplayer} will be verified.
  • * * The final step causes the verifier to resolve a reference to {@code Minecraft}, which is client-only code. - * If this step happend on {@link Dist#DEDICATED_SERVER} (i.e. if the dist check were omitted), the game would crash. + * If this step happened on {@link Dist#DEDICATED_SERVER} (i.e., if the dist check were omitted), the game would crash. * * @apiNote How to access the current Dist will depend on the project. When using FML, it is in FMLEnvironment.dist */ From b60fd9c050c46416221fe1eda5a77d1af15a2f92 Mon Sep 17 00:00:00 2001 From: Brennan Ward <3682588+Shadows-of-Fire@users.noreply.github.com> Date: Sat, 11 Nov 2023 23:51:18 -0800 Subject: [PATCH 4/4] =?UTF-8?q?stop=20trying=20to=20explain=20things?= =?UTF-8?q?=E2=84=A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit its too hard go look at the JLS or something idk --- .../java/net/neoforged/api/distmarker/Dist.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java index 76f5d23..88c9d3d 100644 --- a/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java +++ b/src/forgeAPI/java/net/neoforged/api/distmarker/Dist.java @@ -77,18 +77,11 @@ * * In this example, any code can now call {@code SharedClass.isClientSingleplayer()} without guarding. *

    - * The specifics of why this works relies on how the class verifier operates. When the shared method - * is invoked for the first time, the following steps occur: - *

      - *
    1. The class {@code SharedClass} is loaded, if it was not previously accessed
    2. - *
    3. The method {@code SharedClass.isClientSingleplayer} is loaded and verified.
    4. - *
    5. It is checked that {@code ClientBouncer} exists, but the content of its methods are not yet verified.
    6. - *
    7. If running on {@link Dist#CLIENT}, then {@code ClientBouncer.isClientSingleplayer} will be verified.
    8. - *
    - * The final step causes the verifier to resolve a reference to {@code Minecraft}, which is client-only code. - * If this step happened on {@link Dist#DEDICATED_SERVER} (i.e., if the dist check were omitted), the game would crash. + * The specifics of why this works are too complicated to be written here. + *

    + * Additionally, this process can be extrapolated for use with any optional dependency. * - * @apiNote How to access the current Dist will depend on the project. When using FML, it is in FMLEnvironment.dist + * @apiNote How to access the current Dist will depend on the project. When using FancyModLoader, it is in FMLEnvironment.dist */ public enum Dist {