Skript

Skript/Server Version

[00:05:29 INFO]: [Skript] Skript's aliases can be found here: https://github.com/SkriptLang/skript-aliases
[00:05:29 INFO]: [Skript] Skript's documentation can be found here: https://docs.skriptlang.org/
[00:05:29 INFO]: [Skript] Skript's tutorials can be found here: https://docs.skriptlang.org/tutorials
[00:05:29 INFO]: [Skript] Server Version: git-Paper-435 (MC: 1.20.4)
[00:05:29 INFO]: [Skript] Skript Version: 2.8.3 (skriptlang-github)
[00:05:29 INFO]: [Skript] Installed Skript Addons: 
[00:05:29 INFO]: [Skript]  - skript-reflect v2.4 (https://github.com/SkriptLang/skript-reflect)
[00:05:29 INFO]: [Skript]  - SkBee v3.3.0 (https://github.com/ShaneBeee/SkBee)
[00:05:29 INFO]: [Skript]  - SkBriggy v1.2.0
[00:05:29 INFO]: [Skript] Installed dependencies: 
[00:05:29 INFO]: [Skript]  - Vault v1.7.3-b131

Bug Description

After a bunch of weird stuff going on on my server, I came to realize sending an entity or checking something on a falling block entity would cause the Chunk(0,0) to load.
After racking my brain for an hour, I found a simple test and was able easily reproduce this.

Expected Behavior

Not to load the chunk.

Steps to Reproduce

With this simple test:

on chunk load:
	broadcast "&aLoad: &7%event-chunk%"

on chunk unload:
	broadcast "&cUnload: &7%event-chunk%"

command /test:
	trigger:
		spawn falling dirt 3 above target block
		set {_e} to last spawned entity
		send {_e}

Outcome:

Errors or Screenshots

No response

Other

After a little bit of digging, I came to this conclusion:

Skript/src/main/java/ch/njol/skript/entity/FallingBlockData.java

Line 96 in 1132757

    
           types = new ItemType[] {new ItemType(BlockCompat.INSTANCE.fallingBlockToState(e))};

the culprit:

Skript/src/main/java/ch/njol/skript/bukkitutil/block/NewBlockCompat.java

Lines 355 to 358 in 1132757

    
           public BlockState fallingBlockToState(FallingBlock entity) { 
        
           	BlockState state = entity.getWorld().getBlockAt(0, 0, 0).getState(); 
        
           	state.setBlockData(entity.getBlockData()); 
        
           	return state;

This dude is loading a block to get its state, modify it (which Skript shouldn't be doing) and returning the state.

Agreement

I have read the guidelines above and affirm I am following them with this report.

Im curious why a BlockState is needed in the first place.
Ill have to dig thru the class a lil more and do some testing, but I feel like ItemType should be stripped from this and blockData should be used instead

Yeah, that kind of falls under the item rework I'm planning, but for now I think it has to stay.

Doing a Quick Look thru the block combat classes and ItemData class, BlockState is never cached.
Internally, its converted to a BlockData, so basically its going:
FallingBlock -> BlockState -> ItemType(BlockState -> BlockData -> BlockValues -> ItemType)
Using BlockState here just seems like an extra unneeded step.

another possible improvement if we have to keep the blockstate is to use the block at the location of the FallingBlock when available. that way no chunk needs to be loaded

Im digging around a bit (testing out a change), BlockState is never used anywhere in ItemType/ItemData, and is pretty simple to strip out.

I think that this is the only non-NMS way to create a BlockState, unless I'm mistaken.
Paper 1.20+ has a getBlockState method for FallingBlock, which we should use, but I don't think much else can be done here.

	public BlockState fallingBlockToState(FallingBlock entity) {
	BlockState state = entity.getWorld().getBlockAt(0, 0, 0).getState();
	state.setBlockData(entity.getBlockData());
	return state;

Share to