cominixo / BetterF3

A Fabric mod for Minecraft that provides a highly customizable, more human-readable Debug HUD.
MIT License
135 stars 35 forks source link

(Forge) Mod loading fails in a development environment due to misuse of DistExecutor#safeRunWhenOn #79

Closed RedstoneDubstep closed 2 years ago

RedstoneDubstep commented 2 years ago

In BetterF3Forge#init, two instances of the method DistExecutor#safeRunWhenOn are used to validate that this mod runs when loaded on the client dist, or to send a warning if it is loaded on the server. By using a double lambda for the Supplier<SafeRunnable> argument, the contract of SafeReferent is violated. The correct usage of safeRunWhenOn and other safe methods of DistExecutor is to supply a method reference, as seen in the documentation of DistExecutor$SafeReferent, instead of supplying a lambda expression. When calling safeRunWhenOn in this wrong manner, it gets detected by DistExecutor#validateSafeReferent, which throws a RuntimeException, therefore preventing this mod from loading and causing a crash in the process.

A suggested fix would be to move the supplied lambdas to their own methods in another class (an example for this can be found in the documentation of DistExecutor$SafeReferent) and referencing these in the supplier. Another, more simple solution would be to switch out the calls to DistExecutor with an if (FMLEnvironment.dist == Dist.CLIENT) check, although that solution removes some of the sanity checks of DistExecutor, potentially resulting in attempted classloading of classes not present on the desired dist.

The issue does not occur in production due to a check in DistExecutor#validateSafeReferent, however it affects mod developers using this mod as a dependency (the mods.toml file of this mod's jar needs to be removed after every dependency refresh to prevent this mod from preventing all mod loading in dev). As such, this issue is not game-breaking on the user side, but a fix would still be appreciated by developers working with this mod.