Paper icon indicating copy to clipboard operation
Paper copied to clipboard

TrialSpawnerConfiguration does not play nicely with live blockstates

Open lynxplay opened this issue 1 year ago • 7 comments

Live blockstates do not need to be applied to have their changes reflected in game. The current TrialSpawnerConfiguration however is a fully copy and does only apply when updating.

To fix this, a new implemented of the interface is needed that re-creates the internal record on every setter call and is used if the block state is not a snapshot.

lynxplay avatar Jul 06 '24 18:07 lynxplay

I'd love to work on this if it hasn't been started by somebody else yet!

Strokkur424 avatar Jul 15 '24 18:07 Strokkur424

Go for it 👍

lynxplay avatar Jul 15 '24 18:07 lynxplay

Update: I am still having technical issues with my Windows + WSL setup. If anybody is faster than me, feel free to take this one instead of me

Strokkur424 avatar Aug 09 '24 09:08 Strokkur424

Hi @lynxplay , I am new around here and would like to have a look into this issue if it is still open. It would be great if some more detail is provided around this issue. Thanks!

NithinSravan avatar Oct 06 '24 18:10 NithinSravan

@NithinSravan Paper adds non-snapshot BlockState (or really TileState)s. Changes made can then avoid unneeded copies improving performance. But the TileSpawnerConfiguration implementation doesn’t respect this.

Machine-Maker avatar Oct 07 '24 00:10 Machine-Maker

Some more information I got provided when I tried to take a look at it:

  • The general expectation is that any form of mutation should mutate the real object in the world
  • The following can be used for "replication steps". It should modify the in-game block entity, but that is currently not the case:
final BlockState state = block.getState(false);
if (state instanceof final TrialSpawner trialSpawner) {
    trialSpawner.getNormalConfiguration().setBaseSpawnsBeforeCooldown(100);
}

Also the TrialSpawnerConfig mentioned by lynx refers to net.minecraft.world.level.block.entity.trialspawner.TrialSpawnerConfig. Hope this helps at least a tiny bit @NithinSravan

Strokkur424 avatar Oct 09 '24 18:10 Strokkur424

Does there need to be an entire new implementation of TrialSpawnerConfiguration? Or should the existing CraftTrialSpawnerConfiguration be modified and check isSnapshot on every setter call instead?

tlm9201 avatar Oct 25 '24 02:10 tlm9201

Hi @lynxplay, I’d like to work on this issue if it’s still available. I’ve gone through the description and would be happy to start working on a fix/implementation once you confirm. Thanks!

jha9262 avatar Nov 03 '25 13:11 jha9262