FastAsyncWorldEdit

FastAsyncWorldEdit

Bukkit Plugins
152k Downloads

Mask queries don't work anymore

MineFact opened this issue · 10 comments

commented

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

image

Debug Messages:

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

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

Anything else?

No response

commented

Feel free to open a Pull Request with these changes

  • 👍1
commented

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.

  • 👍1
commented

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

commented

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.

  • 👍1
commented

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

commented

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

commented

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.