Paper: BlockState.update() doesn’t update all tile entity types correctly when not using a snapshot

[me4502]

What behaviour is expected:

It to apply the data to the tile entity

What behaviour is observed:

It makes no impact

Steps/models to reproduce:

Get a BlockState as a non-snapshot from a sign, set a line, and it doesn’t apply on update.

Paper build number:

Found the bug in the code, not in game. But it was after someone reported an issue about it to me.

Anything else:

The update method checks if it’s a snapshot, if it’s not it doesn’t call the apply function. That apply function is required for a few of the tile entity types. The check should be removed.

[electronicboy]

has this been tested using builds from upstream? Lets pretend am smart!

[me4502]

The non-snapshot feature is added by Paper. The specific check in question was added by me in a PR

This issue is basically “revert
https://github.com/PaperMC/Paper/pull/1535 “

[me4502]

@Misio12320 is there a reason you've thumbs downed both of my comments?

[aikar]

Are you saying that your change did nothing but break things and we just need to revert it entirely? Surely the thinking was right on that original PR, can we fix it somehow to avoid most of that update operation and only fix whats broken with this state?

[aikar]

As i see, sign does copy state locally, so thats a case where we could update .setline to have diff behavior if it's not a snapshot to apply directly.

[me4502]

Well in its current state it’s broken, yeah. Updating the setLine behaviour, and any other tile entities that this currently breaks would work too. Whichever is more preferable I guess.

[aikar]

actually yeah looks like that PR was never needed, as applyTo checks that TE != snapshot before copying the TE data. so we can just revert.