Sponge icon indicating copy to clipboard operation
Sponge copied to clipboard

More ChangeBlockEvent.All inconsistent

Open aromaa opened this issue 4 years ago • 7 comments

I am currently running

  • SpongeVanilla version: 1.16.5-8.0.0-RC1036

Issue Description Using the following plugin to test different scenarios:

@Plugin("spongetesting")
class SpongeTestingPlugin @Inject constructor(private val plugin: PluginContainer)
{
	private var cancelledOperation: Operation? = null;

	@Listener
	fun onRegisterCommand(event: RegisterCommandEvent<Command.Parameterized>)
	{
		val operationRegistryType: DefaultedRegistryType<Operation> = RegistryTypes.OPERATION.asDefaultedType { Sponge.game() }

		val operationParameter: Parameter.Value<Operation> = Parameter.registryElement(
			TypeToken.get(Operation::class.java),
			operationRegistryType
		).key("operationType").build()

		event.register(this.plugin, Command.builder().executor { context ->
			this.cancelledOperation = context.requireOne(operationParameter)

			context.sendMessage(Identity.nil(), Component.text("Cancelling now: ${this.cancelledOperation!!.key(operationRegistryType)}"))

			return@executor CommandResult.success()
		}.addParameter(operationParameter).build(), "changeblockevent", "cbe")
	}

	@Listener
	fun onBlockEvent(event: ChangeBlockEvent.All)
	{
		println("Starting block change event! In total: ${event.transactions().size}")

		for (transaction in event.transactions())
		{
			when (transaction.operation())
			{
				Operations.PLACE.get() -> println("Place on ${transaction.original().position()}! ${transaction.original().state()} -> ${transaction.defaultReplacement().state()}")
				Operations.BREAK.get() -> println("Break on ${transaction.original().position()}! ${transaction.original().state()} -> ${transaction.defaultReplacement().state()}")
				Operations.MODIFY.get() -> println("Modify on ${transaction.original().position()}! ${transaction.original().state()} -> ${transaction.defaultReplacement().state()}")
				Operations.GROWTH.get() -> println("Growth on ${transaction.original().position()}! ${transaction.original().state()} -> ${transaction.defaultReplacement().state()}")
				Operations.DECAY.get() -> println("Decay on ${transaction.original().position()}! ${transaction.original().state()} -> ${transaction.defaultReplacement().state()}")
				Operations.LIQUID_SPREAD.get() -> println("Liquid spread on ${transaction.original().position()}! ${transaction.original().state()} -> ${transaction.defaultReplacement().state()}")
				Operations.LIQUID_DECAY.get() -> println("Liquid decay on ${transaction.original().position()}! ${transaction.original().state()} -> ${transaction.defaultReplacement().state()}")
			}

			if (transaction.operation() === this.cancelledOperation)
			{
				transaction.isValid = false
			}
		}
	}
}

When cancelling PLACE:

  • [ ] Placing a block on a block that gets replaced (like grass, vines, etc) causes the item on hand to be consumed.

When cancelling BREAK:

  • [x] Breaking tall grass in survival results the other half to break, this doesn't reproduce in creative. https://gyazo.com/685fc4a65618977dcea5f3a76d9333d5
  • [ ] On activating TNT, its entity is still spawned. https://gyazo.com/a4bbeef8cbcf2c4dccd1733e741ac4a6

When cancelling MODIFY:

  • [ ] On using composter, the item used is eaten.
  • [ ] On using jukebox, the item used is eaten.
  • [ ] Prevents the use of dispensers, should this be the case? We already have events for these. Also furnaces work just fine with this.
  • [ ] Prevents the chest animation. There is no way to tell whatever the chest was opened or closed nor is there any API for it. Also chest animation is "block action", not block state.
  • [ ] Prevents placing snow on top of grass block.

Faulty events:

  • [ ] Liquid spread is always water -> water with same level.
  • [ ] Liquid decay is called as spread.
  • [ ] Liquid levels are backwards (with the source beig 0 instead of 7).
  • [ ] Liquid breaking blocks like cobweb, grass etc causes PLACE operation.
  • [ ] Sugar cane causes MODIFY events with same sugar cane -> sugar cane events.

Noisy events:

  • [ ] Liquid causes MODIFY events around the water blocks when waterlogged blocks are placed.
  • [ ] Fire causes MODIFY events with same fire -> fire state.
  • [ ] Sand causes MODIFY events with same sand -> sand state when blocks are placed on top.
  • [ ] Standing on top of preasure plates spams the powered=true event.
  • [ ] Using scaffolding is REALLY noisy.
  • [ ] Unpowering redstone is noisy'ish.
  • [ ] Opened barrels are noisy.

Expectations:

  • [ ] Breaking waterlogged blocks should procude BREAK operation, not PLACE (Funnily enough, this is only true for tall seagrass top block). https://gyazo.com/7a30b4588761c821d6f868bdb01e5265
  • [ ] Turning grass block to grass path is PLACE operation instead of MODIFY, this feels inconsistent.

Something to discuss:

  • [ ] When fire is extinguished naturally its operation is BREAK, maybe MODIFY for non player related?

IIRC this was server-side:

  • [ ] Blocks with obnoxious sound effects (anvil place, beacon break) should not produce them when cancelled.

aromaa avatar Jul 04 '21 13:07 aromaa

Fixed the first issue, can you re-test/verify some of the other behaviors (if possible, I'll be able to test/verify again around Thursday/Friday).

gabizou avatar Jul 07 '21 01:07 gabizou

I'm unable to test for while as I don't have access to my laptop

aromaa avatar Jul 07 '21 20:07 aromaa

I tested "Cancelled neighbor updates for vines causes item duplication.", "Pistons break blocks (for example, cactus) when powered, but not when placing piston on already powered block. " and "TNT entity is spawned even when the break is cancelled when activated by redstone." and they are still there.

[18:54:58] [Server thread/INFO]: SpongeVanilla
    Minecraft: 1.16.5
    SpongeAPI: 8.0.0-SNAPSHOT
    Sponge: 1.16.5-8.0.0-SNAPSHOT
    SpongeVanilla: 1.16.5-8.0.0-RC941
    JVM: 16.0.1/64-bit (Oracle Corporation)
    OS: Windows 10/10.0 (amd64)

masagameplay avatar Nov 20 '21 16:11 masagameplay

Updated the issue to reflect the issues I have found in 1.16.5-8.0.0-RC1036

aromaa avatar Jan 15 '22 22:01 aromaa

  • On activating TNT, its entity is still spawned. https://gyazo.com/a4bbeef8cbcf2c4dccd1733e741ac4a6

So this one is complicated. TNT is actually spawned before the block is broken, so in technicality, you can't rely on cancelling the block breaking to cancel the tnt spawn. You sadly must do both (this is an engine decision in how TNT is wired up, the block interacts by spawning the new primed TNT and then removes itself).

gabizou avatar Jan 16 '22 07:01 gabizou

You sadly must do both (this is an engine decision in how TNT is wired up, the block interacts by spawning the new primed TNT and then removes itself).

If this isn't going to get fixed then we should document these edge cases so people don't fall to pitfalls. Its common to deny all break actions and by this way you might get bugs where you can infinitely dupe TNT. Think about protected regions.

aromaa avatar Jan 16 '22 12:01 aromaa

Its common to deny all break actions and by this way you might get bugs where you can infinitely dupe TNT. Think about protected regions.

Absolutely, agreed 100%.

gabizou avatar Jan 16 '22 21:01 gabizou