Fabric API

Fabric API

106M Downloads

Working around non-extensible block entities and entities

i509VCB opened this issue ยท 2 comments

commented

I've noticed this is an issue around the place with making custom block entities or entities that extend their original vanilla type.

So signs for example (assume #682 has all the other stuff for signs covered). We want to add a new sign. Now the sign block entity's constructor does not allow you to pass any custom block entity type.

public SignBlockEntity() {
    super(BlockEntityType.SIGN);
    this.textColor = DyeColor.BLACK;
}

Ideally the constructor above would have a sister constructor like below, but one does not exist.

public SignBlockEntity(BlockEntityType<? extends SignBlockEntity type) {
    super(type);
    this.textColor = DyeColor.BLACK;
}

So what's solutions do we have to this:

Add your blocks to the vanilla block entity type
It can work for carbon copy block entities, but anyone how wants to tweak how the block entity acts is out of luck. Also requires an incompatable change to make the supported blocks map mutable.

override get Entity/BlockEntityType

You could override the getType method. This still allows you to return your own entity type if you extend the vanilla ones. However this prohibits you from using the FabricEntityTypeBuilder.

Mixin witchcraft I

Fabric API can't do this themselves due to the fact this is a redirect on the get field conditionally. Does not stack so only one mod can work at a time, which is unacceptable.

A callback?

This works off of the concept above but we target BlockEntity to be more generic. Everytime a block entity is created, mods listen and if they wish, set their own type. Issues can involve mods accidentally overriding vanilla types, and a ton of instanceof calls for developers. This is somewhat hacky.

Adding/generating additional constructors

This involves adding constructors to all entity and block entity classes which don't have these constructors that allow you to pass the type. Forge does this and it works easily for them since they can patch the game to their will. However we don't have that luxury, so this would involve a bunch of asm.

The generated constructors would ideally look like below.

public SignBlockEntity(BlockEntityType<? extends SignBlockEntity type) {
    super(type);
    this.textColor = DyeColor.BLACK;
}

public SignBlockEntity() {
    this(BlockEntityType.SIGN);
}

However this approach could cause issues with mixins targeting the block entity constructors. Since there are two constructors, a few injections/redirects may not work. May be resolved by mod developers using surrogates, but unsure. Some block entities like the bed may require an additional two constructors because of the color of the bed being passed in.

The approach also requires some tweaks to Loom and loader to allow generation of constructors in dev and production. Probably the largest approach of them all.

Other approaches can be mentioned below as ideas.

commented

This will be dealt with on a case-by-case basis according to the guidelines (simplicity, interest to a broader set of mods, etc...).

commented

I really don't know whether it's worth extending these vanilla types not designed for reuse. It's not only that we can't construct them properly, but also that the methods are often not too customizable or inaccessible. It also tends to cause additional porting issues.

IC2 doesn't do this at all IIRC, but it also greatly benefits from integrating types in its own hierarchy. The sign issue sounds more like it wants to change the existing code/data, not actually extend the class with fundamentally different behavior. So, who wants to extend otherwise locked down leaf classes and why is it substantially better than a parallel implementation in those cases?