feat!: Use ServiceLoader for Renderer registry, RendererReadyEntrypoint#5185
feat!: Use ServiceLoader for Renderer registry, RendererReadyEntrypoint#5185sylv256 wants to merge 17 commits intoFabricMC:26.1from
Renderer registry, RendererReadyEntrypoint#5185Conversation
|
Setting to draft as changes were requested |
...enderer-api-v1/src/client/java/net/fabricmc/fabric/impl/client/renderer/RendererManager.java
Outdated
Show resolved
Hide resolved
fabric-renderer-indigo/src/client/java/net/fabricmc/fabric/impl/client/indigo/Indigo.java
Show resolved
Hide resolved
- Crash when no `RendererProvider` is present - Replace `fabric-renderer-indigo:contains_renderer` mechanism with the `RendererProvider#priority` system
# Conflicts: # fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/api/client/renderer/v1/Renderer.java # fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/impl/client/renderer/RendererManager.java # fabric-renderer-indigo/src/client/java/net/fabricmc/fabric/impl/client/indigo/Indigo.java # fabric-renderer-indigo/src/client/java/net/fabricmc/fabric/impl/client/indigo/IndigoMixinConfigPlugin.java # fabric-renderer-indigo/src/client/java/net/fabricmc/fabric/impl/client/indigo/renderer/IndigoRenderer.java
- Also remove unnecessary ServiceLoader for Indigo
...erer-api-v1/src/client/java/net/fabricmc/fabric/api/client/renderer/v1/RendererProvider.java
Outdated
Show resolved
Hide resolved
...erer-api-v1/src/client/java/net/fabricmc/fabric/api/client/renderer/v1/RendererProvider.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| /** | ||
| * Get or instantiate an implementation of {@link Renderer}. |
There was a problem hiding this comment.
This javadoc should clarify that this method is called only if this provider is selected and that this method is guaranteed to run at the same time as ClientModInitializer.
There was a problem hiding this comment.
I'm not sure if this is relevant anymore.
...enderer-api-v1/src/client/java/net/fabricmc/fabric/impl/client/renderer/RendererManager.java
Show resolved
Hide resolved
...enderer-api-v1/src/client/java/net/fabricmc/fabric/impl/client/renderer/RendererManager.java
Outdated
Show resolved
Hide resolved
...enderer-api-v1/src/client/java/net/fabricmc/fabric/impl/client/renderer/RendererManager.java
Outdated
Show resolved
Hide resolved
...enderer-api-v1/src/client/java/net/fabricmc/fabric/impl/client/renderer/RendererManager.java
Outdated
Show resolved
Hide resolved
...enderer-api-v1/src/client/java/net/fabricmc/fabric/impl/client/renderer/RendererManager.java
Outdated
Show resolved
Hide resolved
| if (needsLoad) { | ||
| for (ModContainer container : FabricLoader.getInstance().getAllMods()) { | ||
| final ModMetadata meta = container.getMetadata(); | ||
| indigoApplicable = RendererProvider.get().id().equals("fabric-renderer-indigo"); |
There was a problem hiding this comment.
If the API exposes the entire RendererProvider instance instead of just the ID, then using an instanceof check like FREX instead of having the ID in the API at all would work. I suggested only exposing the ID because mods should not be creating renderers or checking provider priorities manually.
There was a problem hiding this comment.
Mods may have a genuine need to check the ID of the RendererProvider, so I think we should keep it rather than forcing them to rely on Renderers' implementation details. Everything else can be OverrideOnly.
If it's somehow any better, we can do an instanceof check here since that works in implementation. I just wanted to have a good example of how to use the API.

Problem
Because
Renderercan be rendered at any point before or during Minecraft's initialization until it is used, problems arise when it's necessary to use it early. For example, in order to disable Indigo, FRAPI implementations need to add a custom field to theirfabric.mod.json. Renderers should not need to add a field to their FMJ just to disable Indigo, let alone any other Renderer. Also, mods extending FRAPI such as Conduit need a defined point in time during and after whichRendereris ready to use. All of this introduces inconsistent behavior and quickly becomes a headache. See #3109 for more details.Solution
This Pull Request overhauls and adds new concepts to FRAPI's
Rendererregistration.RendererProviderInstead of manually calling
Renderer#register, renderers are now registered with a Service Loader. This lets mods get basic information about the renderer that will be loaded at any point during the game's execution as they can be loaded immediately, and this is especially useful for Mixin Config Plugins.RendererReadyEntrypointIn
fabric.mod.json, it is defined asfabric-renderer-ready.Sometimes, mods will want to use
Rendererright away, and this is where the issues with the prior system really reveal themselves. Previously, if a mod wantedRenderer, they would have to either useRenderer#getand hope it's already registered, or they would Mixin intoRenderer#registerto ensure thatRendererhas been registered before using it.Conduit is a library built to create and implement extensions for FRAPI. In order for Conduit's extensions, implementations, and users to use FRAPI and Conduit effectively, a
RendererReadyEntrypointis required. Some operations are best done as soon as theRendereris ready, so having an entrypoint to ensure code that requiresRendererdoes not use it before it is registered is powerful.Tests & Integrity
I've added two checks: one in FRAPI itself to ensure a renderer is loaded, and one in its Client Testmod that asserts
RendererReadyEntrypointis invoked. If either of these conditions fail, an exception is thrown.Additional Information
Proof that it works

Fixes #3109