Merging rendering modules
Juuxel opened this issue ยท 6 comments
Fabric API currently has ten rendering modules: blockrenderlayer-v1
, models-v0
, renderer-api-v1
, renderer-indigo
, renderer-registries-v1
, rendering-data-attachment-v1
, rendering-fluids-v1
, rendering-v0
, rendering-v1
and textures-v0
. This is really excessive in my opinion, especially when many of them are single-feature modules (block render layer, data attachment, fluids and textures).
Something like this structure would be better:
renderer-api-v1
andrenderer-indigo
are kept the samerendering-v1
gets the contents from six other modules:blockrenderlayer-v1
,models-v0
,rendering-data-attachment-v1
,rendering-fluids-v1
,renderer-registries-v1
(also an opportunity to fix #427) andtextures-v0
- Both
blockrenderlayer-v1
andtextures-v0
are single-purpose modules that only contain one or two API classes. - The model loading hooks in
models-v0
are also directly related to rendering, so they would make sense as a subpackage inrendering-v1
.
- Both
rendering-v0
is dropped completely
With that model, there would only be three distinct modules: the renderer API, its implementation (Indigo) and the generic rendering module for everything else.
Edit: I forgot about blockrenderlayer-v1
, models-v0
and textures-v0
.
I believe it's gotten better now with the deprecation of some of these modules.
It's not significantly better - two have been deprecated (renderer registries and rendering v0), while textures was removed in 1.19.3.
There are still seven non-deprecated modules dedicated to rendering: blockrenderlayer-v1
, models-v0
, renderer-api-v1
, renderer-indigo
, rendering-data-attachment-v1
, rendering-fluids-v1
, rendering-v1
, (8 if you include particles-v1
).
In addition, fluid rendering is also now split between the dedicated fluid rendering module and the transfer API.
Some of these splits feel pointless:
- Does a single API class,
BlockRenderLayerMap
- with a misleading name for that matter since it's not only for blocks nor a map of any kind - need its own module? - Why does fluid rendering need to be in two different modules?
- JSON models are in
models-v0
, while entity models are inrendering-v1
. And before they were moved to a TAW, model predicate providers were inobject-builder-api-v1
!
rendering-v0 should have been dropped completely a while ago, in my opinion. Regarding the other three modules, the separation is deliberate, as their concerns are separate.
The issue I see is how do we distinguish between rendering-api-v1
and renderer-api-v1
as they sound very similar from afar and may be confused
their concerns are separate
The distinctions are somewhat arbitrary. For example, any renderer implementation will intersect with rendering-data-attachment
, and I'm not thinking of uses for it outside of that. Plus there are other modules related to rendering that don't even bear the name. Model loading and particles come to mind.
Based on the number and nature of questions I get and observe about how to render in Fabric, the overall structure is confusing and disjoint enough to be an obstacle to adoption. I point people to FabricBakedModel and they generally seem to understand that, but nothing leads them from there to all the other pieces that aren't even in the renderer module, and when they do find them it's not clear how they fit together.
I guess the point I'm making here is that Fabric API tends to be organized around its internal construction vs how it will be used, which is generally poor design for any API. Docs can only do so much for an API that isn't organized for use. This is especially baffling given that API nesting is discouraged (or has been, I can't keep track) and the API ships as a monolithic jar.
This is a problem that has always bothered me but was already an entrenched practice when I got here. It would involve many breaking changes to fix, so I gave up the fight long ago. It's at least not worse than that other modding API a few people use.