Create: Connected

Create: Connected

3M Downloads

Crash - Additional Placements

emihead opened this issue ยท 15 comments

commented

Hi! The game crashes when you place Copycat Stairs on the corner of a block while Additional Placements 2.0.1 is installed.
You may be able to look at how Copycats+ implements their AP compat as reference.

Crash report
https://mclo.gs/VSc6ENX
Log
https://mclo.gs/sZ50K55

commented

Alright, they're up on modrinth, curseforge needs to wait for approval

commented

Wow that's quick ๐Ÿ‘

Is this the API for compatibility? https://github.com/FirEmerald/AdditionalPlacements/blob/1.20.1/src/main/java/com/firemerald/additionalplacements/block/interfaces/IGenerationControl.java

I would prefer something like a block tag or a "generation manager" class we can call to disable generation, since I don't think it is possible for us to optionally implement IGenerationControl in a way that it doesn't crash when AP isn't installed.

commented

ah, I did forget about that. Forge at least used to have an interface you could add onto a class that would allow you to strip out an interface and it's methods from a class if a mod wasn't loaded.

commented

tags wouldn't work, the block creation naturally has to occur before tags can be loaded. similarly, I can't use an event, because the forge event bus doesn't register events until after the registries are built (boo). I can't just have you add blocks into a blacklist collection because sometimes the additional placement variant is created and registered immediately after the normal block has been (because forge won't let me run my block registration code exclusively after all other mods). The only thing I can think of is blacklisting block IDs in this way instead, or maybe blacklisting entire classes.

commented

keep in mind I have to work under the constraints of Forge, NeoForge, and fabric, so a solution that only works on one of those is not going to be accepted.

commented

theoretically, to blacklist an entire class, you could register a generation type for that class that always says a block isn't valid for generation. look at APGenerationTypes to see how you register types, and see AdditionalPlacementsMod (for forge and neoforge) for how the class is registered (fabric uses a custom entrypoint instead, see fabric.mod.json)

commented

Look ma, I'm famous!

All joking aside the "changes" have already been uploaded to the repo and will be present in 2.0.2 which I am just about to start building and publishing. I have to do this for 11 different versions across both curseforge and modrinth so it might be a bit before they're up.

commented

I don't think I'm going to port the Copycats+ compatibility mixins over to C:Connected. The dev of AP is aware of the issue and will roll out some API changes for us to add compatibility. I'll update both C:Connected and Copycats+ when that happens.

commented

Omg I didn't know you were also a Copycats+ dev. That's my bad lmao, good to know you have plans though!

commented

Ran a quick test. Since IPlacementBlock extends IGenerationControl and is mixed into the block classes that are valid for generation, just adding the method without extending the interface is sufficient. I'll update the javadoc on the interface to reflect this, and hopefully we can consider the issue closed.

commented

Any news on this from your end? Anything I need to do to make things work better?

commented

Ah I was busy in another feature branch. Will try it out later today.

commented

Yup it's working d912973

@FirEmerald Since this is working so well, would you be interested in exposing a more advanced API such that copycats can make use of AP's placement logic? I'm thinking about a method I can implement in the block class that tells AP which block state to place when different parts of a block face is clicked.


Update: it works for C:Connected but not for Copycats+ because copycat logic lives in interfaces, which can't override IGenerationControl

In this specific case, CopycatPressurePlateBlock implements ICopycatBlock and IGenerationControl, and the default implementation of generateAdditionalStates in ICopycatBlock can't be used to implement IGenerationControl unless I modify every single copycat block class.

[16:57:27] [Render thread/ERROR] [ne.mi.fm.ja.FMLModContainer/]: Exception caught during firing event: Receiver class com.copycatsplus.copycats.content.copycat.pressure_plate.CopycatPressurePlateBlock does not define or inherit an implementation of the resolved method 'boolean generateAdditionalStates()' of interface com.firemerald.additionalplacements.block.interfaces.IPlacementBlock.
	Index: 1
	Listeners:
		0: NORMAL
		1: ASM: class com.firemerald.additionalplacements.common.CommonModEventHandler onBlockRegistry(Lnet/minecraftforge/registries/RegisterEvent;)V
java.lang.AbstractMethodError: Receiver class com.copycatsplus.copycats.content.copycat.pressure_plate.CopycatPressurePlateBlock does not define or inherit an implementation of the resolved method 'boolean generateAdditionalStates()' of interface com.firemerald.additionalplacements.block.interfaces.IPlacementBlock.
	at TRANSFORMER/[email protected]/com.firemerald.additionalplacements.block.interfaces.IPlacementBlock.canGenerateAdditionalStates(IPlacementBlock.java:153)
	at TRANSFORMER/[email protected]/com.firemerald.additionalplacements.generation.Registration.tryApply(Registration.java:35)

At this point I honestly wouldn't mind if AP just leaves a convenient spot for Copycats+ to inject mixins into, similar to what we are already doing.

commented

@FirEmerald Since this is working so well, would you be interested in exposing a more advanced API such that copycats can make use of AP's placement logic? I'm thinking about a method I can implement in the block class that tells AP which block state to place when different parts of a block face is clicked.

For now you should be able to do the exact same thing by creating the methods
public BlockState getStateForPlacementImpl(BlockPlaceContext context, BlockState currentState)
for controlling the blockstate when placed,

@OnlyIn(Dist.CLIENT)
public void renderPlacementPreview(PoseStack pose, VertexConsumer vertexConsumer, Player player, BlockHitResult result, float partial)

for controlling the rendering of the placement grid (it's pre-transformed to the side you're looking at, with the Z value being -.5 for all vertices and the X and Y ranging from -.5 to .5),

@OnlyIn(Dist.CLIENT)
public void renderPlacementHighlight(PoseStack pose, VertexConsumer vertexConsumer, Player player, BlockHitResult result, float partial)

for controlling the rendering of a placement preview (used for stairs currently, untransformed and the vertex values range from -.5 to .5)
best examples can be found in IStairBlock

Update: it works for C:Connected but not for Copycats+ because copycat logic lives in interfaces, which can't override IGenerationControl

In this specific case, CopycatPressurePlateBlock implements ICopycatBlock and IGenerationControl, and the default implementation of generateAdditionalStates in ICopycatBlock can't be used to implement IGenerationControl unless I modify every single copycat block class.

[16:57:27] [Render thread/ERROR] [ne.mi.fm.ja.FMLModContainer/]: Exception caught during firing event: Receiver class com.copycatsplus.copycats.content.copycat.pressure_plate.CopycatPressurePlateBlock does not define or inherit an implementation of the resolved method 'boolean generateAdditionalStates()' of interface com.firemerald.additionalplacements.block.interfaces.IPlacementBlock.
	Index: 1
	Listeners:
		0: NORMAL
		1: ASM: class com.firemerald.additionalplacements.common.CommonModEventHandler onBlockRegistry(Lnet/minecraftforge/registries/RegisterEvent;)V
java.lang.AbstractMethodError: Receiver class com.copycatsplus.copycats.content.copycat.pressure_plate.CopycatPressurePlateBlock does not define or inherit an implementation of the resolved method 'boolean generateAdditionalStates()' of interface com.firemerald.additionalplacements.block.interfaces.IPlacementBlock.
	at TRANSFORMER/[email protected]/com.firemerald.additionalplacements.block.interfaces.IPlacementBlock.canGenerateAdditionalStates(IPlacementBlock.java:153)
	at TRANSFORMER/[email protected]/com.firemerald.additionalplacements.generation.Registration.tryApply(Registration.java:35)

At this point I honestly wouldn't mind if AP just leaves a convenient spot for Copycats+ to inject mixins into, similar to what we are already doing.

this is more difficult for me to figure out, but for now you could just mixin to Registration.tryApply(Block, ResourceLocation, BiConsumer<ResourceLocation, AdditionalPlacementBlock<?>>)
I would just like to make things not require mixins if at all possible.

commented

Alright well I finally have added solution that I feel should work fine. I wish I could do something more elegant, like using an event, but when testing I found that none of my event subscribers would pick up the events, so I instead added it into my existing registration handler.
The classes you'll be using are RegistrationInitializer and IBlockBlacklister.