MinecraftForge / MinecraftForge

Modifications to the Minecraft base files to assist in compatibility between mods. New Discord: https://discord.minecraftforge.net/
https://files.minecraftforge.net/
Other
6.96k stars 2.7k forks source link

[1.21.3] Rendering: Add State Extraction hook, custom state fields API #10165

Open StavWasPlayZ opened 3 weeks ago

StavWasPlayZ commented 3 weeks ago

I'm making this first an issue to discuss if the following is ok/relevant to be considered into Forge.
I'm willing to convert it into a PR if all is fine.

TL;DR: A new ExctractRenderStateEvent.Pre/Post should be implemented to compensate for the lack of an entity reference in the reformed RenderLivingEvent, as well as to adapt for Minecraft's new render states abstraction.

There is also the possibility to provide an API for modders to add custom rendering state fields to different entities and states.


ExctractRenderStateEvent

In 1.21.2, a refactoring change has made it so that rendering-related fields in EntityRenderers are now separated into another data class - EntityRenderState.
Hence, the new EntityRenderer#render method is now defined with the following signature:

public void render(S state, PoseStack poseStack, MultiBufferSource multiBufferSource, int packedLight);

In this form, the entity is now omitted from context.
There is no way, as of current, to trace back to the original entity that possesses the renderer OR its state (as far as I'm aware).

This consequently affects RenderLivingEvent - which no longer passes an entity reference.

Here is an example method from 1.21.1 that used this reference, for instance:

// Handle block instrument arm pose
@SubscribeEvent
public static void prePlayerRenderEvent(final RenderPlayerEvent.Pre event) {
    final Player player = event.getEntity();

    if (!(InstrumentOpenProvider.isOpen(player) && !InstrumentOpenProvider.isItem(player)))
        return;

    final Block block = player.level().getBlockState(InstrumentOpenProvider.getBlockPos(player)).getBlock();
    if (!(block instanceof AbstractInstrumentBlock instrumentBlock))
        return;

    final PlayerModel<AbstractClientPlayer> model = event.getRenderer().getModel();
    model.leftArmPose = model.rightArmPose = instrumentBlock.getClientBlockArmPose();
}

This method operates a bunch of checks on the player, that if passed - applies a certain ArmPose to them before rendering is applied.

This is now impossible.

The solution I came with (diff) was to hook an event to the end of the EntityRenderer#extractRenderState methods - where the initialization of the state's fields are to be completed, and we can manipulate them so as to not be overwritten or be too late into rendering.

Since the way this method works is by chaining a bunch of overrides-calls-super, we can't determine where the method really ends - so I placed the hook at the end of the EntityRenderer#createRenderState, which is basically the wrapper method mostly referred to for this operation anyways.

Also added Pre/Post phases, just because we can.

Custom Render State Fields API

The approach to add one's own render state field and then use it for rendering without an entity reference aligns with that of the new Minecraft API.

This was my initial solution to the problem as well.

My implementation (same diff) involves defining the following class, to identify custom fields:

/**
 * Represents a custom {@link EntityRenderState} field.
 * @param initValSupplier Called on {@link EntityRenderer#extractRenderState}. Supplies the initial value to be provided to this field.
 * @param entityType The entity type to have the field attached to
 * @param renderStateType The render state type to have this field attached to
 * @param <T> The field type
 */
@OnlyIn(Dist.CLIENT)
public record CustomRenderStateField<T>(
    // For filtering:
    Class<? extends Entity> entityType,
    Class<? extends EntityRenderState> renderStateType,
    RenderStateSupplier<T> initValSupplier
) {
    @FunctionalInterface
    public static interface RenderStateSupplier<T> {
        /**
         * Supplies a value to be provided to a render state field.
         * @param entity The entity having this field attached to
         * @param state The state that the field will get attached to
         * @param packedLight
         * @return The value to be put in the state field
         */
        T get(EntityRenderer<?, ?> entityRenderer,
              Entity entity,
              EntityRenderState state,
              float packedLight);
    }
}

Which would then be put into a client-only registry - mine uses a simple HashMap:

private static final HashMap<ResourceLocation, CustomRenderStateField<?>> CUSTOM_RENDER_STATE_FIELDS = new HashMap<>();

Example custom field:

public static final CustomRenderStateField<Optional<ArmPose>> INSTRUMENT_BLOCK_PLAYED = CustomRenderStateFieldRegistry.register(
    GInstrumentMod.loc("instrument_block_played"),
    new CustomRenderStateField<>(
        Player.class,
        GIRenderStates::extractInstrumentBlockPlayingState
    )
);

And at the end of vanilla's render state initialization (aka where the ExctractRenderStateEvent.Post will be hooked), we can perform the following operation:

public static <E extends Entity, S extends EntityRenderState> void initFields(EntityRenderer<E, S> entityRenderer,
                                                                              E entity,
                                                                              S state,
                                                                              float packedLight) {
    CUSTOM_RENDER_STATE_FIELDS.values().stream()
        .filter((field) ->
            field.entityType().isInstance(entity) && field.renderStateType().isInstance(state)
        )
        .forEach((field) ->
            state.setCustomField(
                field,
                field.initValSupplier().get(entityRenderer, entity, state, packedLight)
            )
        );
}

Where get/setCustomField are new methods patched into the EntityRenderState class:

private final HashMap<CustomRenderStateField<?>, Object> customStateFields = new HashMap<>();

@SuppressWarnings("unchecked")
public <T> T getCustomField(CustomRenderStateField<T> field) {
    return (T) customStateFields.get(field);
}
public <T> void setCustomField(CustomRenderStateField<T> field, T value) {
    customStateFields.put(field, value);
}

This would allow us to define and use custom render fields, and access a set of defined entity-related data.

Example usage:

@SubscribeEvent
// Handle block instrument arm pose
public static void onLivingRenders(final RenderLivingEvent.Pre<?, ?, ?> event) {
    final PlayerRenderState renderState = (PlayerRenderState) event.getState();

    renderState.getCustomField(GIRenderStates.INSTRUMENT_BLOCK_PLAYED)
        .ifPresent((pose) -> {
            renderState.mainHandState.pose = renderState.offhandState.pose = pose;
            renderState.mainHandState.isEmpty = renderState.offhandState.isEmpty = false;
        });
}

Note: RenderPlayerEvent is not hooked to anything in Forge 53.0.9. It is instead calling another RenderLivingEvent. Will push PR to fix.

Jonathing commented 3 weeks ago

This is just my opinion, but if you are going to propose a change to Forge and have code ready to put into it, I think you might as well create a PR for it. It will be easier for me, as a code reviewer, to actually read what you are trying to do, as it is easier for me to understand from code diffs rather from explanatory paragraphs. Though since rendering is not my area of expertise, I'd like @LexManos or @basdxz to comment on this one.

StavWasPlayZ commented 3 weeks ago

I really apologize if it came out as an inconvenience. It was more so my intention to discuss and get feedback on that API before actually putting my time implementing it.

I did, however, attach a diff - albeit produced on my own mod project, so as to actually test these ideas in production.

The only real difference is that it contains mixins instead of patches.

Jonathing commented 3 weeks ago

No need to apologize, I just think it is easier that way.