neoforged / NeoForge

Neo Modding API for Minecraft, based on Forge
https://projects.neoforged.net/neoforged/neoforge
Other
1.22k stars 179 forks source link

Common interface for registry-typed object #1448

Open HenryLoenwind opened 3 months ago

HenryLoenwind commented 3 months ago

Add a common interface to all objects that have a type that is registered and their types, e.g. Entity, BlockEntity, AbstractContainerMenu, ...

Currently, it isn't possible to handle those objects in a generic way, i.e. you have to have code that looks identical if you want to handle multiples of those.

void <T extends AbstractContainerMenu> foo(T x) {
  MenuType<T> t = x.getType();
  ResourceLocation y = t.getKey(x);
  ByteBufCodecs.registry(Registries.MENU).encode(b, t);
  ...
}

void <T extends Entity> foo(T x) {
  EntityType<T> t = x.getType();
  ResourceLocation y = t.getKey(x);
  ByteBufCodecs.registry(Registries.ENTITY_TYPE).encode(b, t);
  ...
}

...

To name a few calls that force you to duplicate code because there's no common supertype/interface. This could instead be handled by the same code, for example:

void <T extends ITypedRegistryObject> foo(T x) {
  IRegisteredType<T> t = x.getType();
  ResourceLocation y = t.getKey(x); // t.getRegistry().getKey(x)
  ByteBufCodecs.registry(t.getRegistry()).encode(b, t);
  ...
}

Note that this is more of an issue for library-style code, not so much for mod-functionality code. The latter usually is very limited in what kind of object it handles and doesn't need generics.

I have WIP code that runs into this issue quite head-on and can link to it if needed. I also would volunteer to implement this if this suggestion receives positive feedback.

lukebemish commented 2 months ago

This gets iffy fast. For instance, how do you handle something like a ConfiguredFeature that has a Feature as its type? What about the number of other worldgen things that have a Codec<T> as their type? All in all, I see a few major issues -- (a) there's no good definition for what should or shouldn't have such an interface on it, and (b) the use cases for such an interface are already miniscule; how often do you want to do the same treatment to enough of those different types to make this worthwhile?

Worst of all, in order to get the specific type, you run into some... issues. Consider the following case: I have a Zebra extends Entity and a Giraffe extends Entity. Under your proposal, Zebra should be an ITypedRegistryObject<Zebra> so that I can get a IRegisteredType<Zebra> out of it. Otherwise, you'd need an unsafe generics cast there for that to be type safe. You see the issue? Both Zebra and Giraffe would have to implement the interface separately -- or you'd have to add a generic to Entity so that you can declare it via a recursive generic there. Both of which are shitty solutions, and also make it impossible for someone to have both an A extends Entity and B extends A as sensible non-abstract classes. Obviously all that is a complete no-go, so you have to simplify it down to just Entity implements ITypedRegistryObject, which severely limits what you can do and have type safety -- the most you could get out is a IRegisteredType<? extends Entity>, which is far less useful.

Now, the problems continue: namely, your proposed API assumes that the thing you get out is actually registered and has a key associated. Which it may -- in some cases! But in other cases (namely, datapack registry stuff) it's likely not aware of its own key, and may not be able to retrieve it without access to a registry/level context. All in all, I think such an API introduces far more problems, blurry edges, and unintuitiveness than it offers solutions -- especially as in practice there's two, maybe three types that are like this that folks are consistently doing this sort of stuff for.