Mask queries don't work anymore

[MineFact]

Server Implementation

Paper

Server Version

1.20.4

Describe the bug

queryRel Masks don't seem to work anymore. Whenever I try to use it, it doesn't work and spams the same NullPointerException Cannot invoke "com.sk89q.worldedit.internal.expression.ExpressionEnvironment.getBlockTypeRel(double, double, double)" because "this.environment" is null in the console over and over again.

• I tried it with different servers even with ones that only have FAWE installed
• I tried it with newest server versions and older server versions (1.19.4)
• I tried it with newest and older FAWE versions till half a year ago
• I tried it with normal WorldEdit and there it works. Only FAWE has this problem.
  => No matter what I do I can always reproduce this error with any queryRel and any mask or gmask that should produce a result but fails.

I also looked at the code and compiled it with some debug messages. Whenever I type the queryRel command, new Function classes get created and the Environment variable in the Function class is set. But once I execute the command another Function class is created but this time no Environment is set which then obviously leads to a NullPointerException.

What should I do now? I really need this to work and I'm willing to help out to fix this problem.

I talk about this line in the code but it will probably also fail for the other methods like query or queryAbs: https://github.com/IntellectualSites/FastAsyncWorldEdit/blob/main/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Functions.java#L425

To Reproduce

1. Create a brush (e.g. //brush sphere stone)
2. Create a mask with queryRel (e.g. //mask =queryRel(0,1,0,0,0) - Mask all blocks that have air above them)
3. Click with the brush and see the errors in the console. Nothing gets replaced even though it should work.

Expected behaviour

Normally the queryRel should work. In my Steps to Reproduce example it should replace all surface blocks with stone.
It says that it worked but no blocks get placed and an error gets printed in the console.

Screenshots / Videos

Debug Messages:

• FUNCTIONS CREATED message in Functions.java constructor
• ENVIRONMENT SET message in Functions.setEnvironment() function
• Environment check on command execution
  

Error log (if applicable)

https://paste.gg/p/anonymous/a1cdb839af32405a98c6bcbd68ac0357

Fawe Debugpaste

https://athion.net/ISPaster/paste/view/89e8261d1a60492f859535bd46e51942

Fawe Version

2.9.1-SNAPSHOT

Checklist

• I have included a Fawe debugpaste.
• I am using the newest build from https://ci.athion.net/job/FastAsyncWorldEdit/ and the issue still persists.

Anything else?

No response

[OneLiteFeather]

@IntellectualSites/core-team should we do a "Piston" label additionally? Looks like it is one of some breaking changes.

[SirYwell]

That isn't caused by Piston. We clone the expression

FastAsyncWorldEdit/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java

Lines 201 to 203 in e682917

	public Expression clone() {
	return new Expression(initialExpression, new HashSet<>(providedSlots));
	}

we currently don't set the environment.

[OneLiteFeather]

Is this an easy to fix issue or is it more a deeper issue that newcomers can't fix?

[SirYwell]

From a quick look, the environment needs to be cloned as well (or it needs to be made thread-safe), but other than that, it should be pretty simple.

[MineFact]

Thanks for the comments I took a look at it and yes it seems that cloning the expression but not setting the environment afterwards was the issue.

I fixed it in the code and now the issue is gone.

I only changed the clone function is this enough for pull request or does something else need to be changed as well? From my testing everything is fine with those changes.

These are the changes that I made:
https://github.com/MineFact/FastAsyncWorldEdit/blob/0e15e83cecfa51c2f9a095450873fb3c3b14646a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java#L200-L206

[SirYwell]

If you look at https://github.com/IntellectualSites/FastAsyncWorldEdit/blob/e682917c437096885b84332cf421d62fc395bcf8/worldedit-core/src/main/java/com/sk89q/worldedit/regions/shape/WorldEditExpressionEnvironment.java, there is a mutable field that is set for every block before evaluating the expression for that position. If multiple threads to that concurrently, things will break. I think there is no other sane way than also cloning/duplicating the environment.

[MineFact]

So like this?

WorldEditExpressionEnvironment clone function is here:
https://github.com/MineFact/FastAsyncWorldEdit/blob/c217f68c0aca695ed4348e483ca74fd66d4a341a/worldedit-core/src/main/java/com/sk89q/worldedit/regions/shape/WorldEditExpressionEnvironment.java#L98-L100

Expression clones the WorldEditExpressionEnvironment before applying it to the cloned Expression:
https://github.com/MineFact/FastAsyncWorldEdit/blob/c217f68c0aca695ed4348e483ca74fd66d4a341a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java#L202-L206

[PierreSchwang]

That would add .5 to zero every clone which is not wanted I guess. You'd have to normalize to -.5 again (as you dont have access to zero but only zero2)

https://github.com/MineFact/FastAsyncWorldEdit/blob/c217f68c0aca695ed4348e483ca74fd66d4a341a/worldedit-core/src/main/java/com/sk89q/worldedit/regions/shape/WorldEditExpressionEnvironment.java#L45
https://github.com/MineFact/FastAsyncWorldEdit/blob/c217f68c0aca695ed4348e483ca74fd66d4a341a/worldedit-core/src/main/java/com/sk89q/worldedit/regions/shape/WorldEditExpressionEnvironment.java#L99

[MineFact]

@PierreSchwang ah yeah thx you are right I didn't see that.

It should be fixed now. I also tested it ingame and everything works fine!

https://github.com/MineFact/FastAsyncWorldEdit/blob/7b7357e6e4ab48b96a023ebebb52d04246a41f6d/worldedit-core/src/main/java/com/sk89q/worldedit/regions/shape/WorldEditExpressionEnvironment.java#L100-L102

[SirYwell]

Feel free to open a Pull Request with these changes