Captain-Chaos / WorldPainter

WorldPainter is an interactive graphical map generator for the indie game Minecraft.
http://www.worldpainter.net/
GNU General Public License v3.0
341 stars 58 forks source link

Bugfix/filter in script #380

Open ALingll opened 3 months ago

ALingll commented 3 months ago

Fixes #381

Issue

The issue was triggered by my intention to reorganize and write the WPScript API documentation. While reviewing the code, I noticed that some interfaces were not documented in the existing API documentation. Upon attempting to use them, I discovered the following bugs.

This pull request addresses two bugs in the WorldPainter script:

  1. The Filter objects returned when running CreateFilterOp.onlyOnLayer(layer).withValue(value) and CreateFilterOp.exceptOnLayer(layer).withValue(value) are opposite to the expected behavior:
    
    var layer = wp.getLayer()
    .withName('Swamp') // The name of the standard layer
    .go();

var filter = wp.createFilter() .onlyOnLayer(layer).withValue(8) .go(); //this filter is expected to only affect areas where the layer intensity is 8, but instead, the terrain was painted everywhere except where the layer intensity is 8. // Similarly, the result obtained from exceptOnLayer(layer) is also opposite to the expected behavior.

print(filter.toString())

wp.applyTerrain(3) .toWorld(world) .withFilter(filter) .applyToSurface() .go();


2. The `orHigher()` / `orLower()` methods of `CreateFilterOp` are always unusable:

The JavaScript code that caused the error is as follows:
```js
var layer = wp.getLayer()
    .withName('Swamp') // The name of the standard layer
    .go();

var filter = wp.createFilter() 
    .onlyOnLayer(layer).withValue(8).toHigher()
    .go();

print(filter.toString())

wp.applyTerrain(3)
    .toWorld(world)
    .withFilter(filter)
    .applyToSurface()
    .go();

In the original source code, to ensure that orHigher() and orLower() are not set simultaneously, the following condition was made:

//org.pepsoft.worldpainter.tools.scripts.CreateFilterOp

public CreateFilterOp orHigher() throws ScriptException {  
    if (exceptOnLastSet) {  
        if (exceptOn instanceof Layer) 
        {  
            throw new ScriptException("No \"except on\" layer value specified for \"or higher\"");  
        } else if (((LayerValue) exceptOn).condition != null) 
        {  
            throw new ScriptException("Only one of \"or lower\" and \"or higher\" may be specified for \"except on\" value");  
        }  
        exceptOn = new LayerValue(((LayerValue) exceptOn).layer, ((LayerValue) exceptOn).value, HIGHER_THAN_OR_EQUAL);  
    } else {  
        if (onlyOn == null) 
        {  
            throw new ScriptException("No \"only on\" layer specified for \"or higher\"");  
        } else if (onlyOn instanceof Layer) 
        {  
            throw new ScriptException("No \"only on\" layer value specified for \"or higher\"");  
        } else if (((LayerValue) onlyOn).condition != null) 
        {  
            throw new ScriptException("Only one of \"or lower\" and \"or higher\" may be specified for \"only on\" value");  
        }  
        onlyOn = new LayerValue(((LayerValue) onlyOn).layer, ((LayerValue) onlyOn).value, HIGHER_THAN_OR_EQUAL);  
    }  
    return this;  
}

However, in practice, the constructor of LayerValue always initializes onlyOn.condition and exceptOn.condition to DefaultFilter.Condition.EQUAL. Therefore, the truth value of the condition (LayerValue) onlyOn).condition != null is always true.

((LayerValue) exceptOn).condition != null //will be true forever
((LayerValue) onlyOn).condition != null //will be true forever

Fix

  1. Modified the modifyStrength() method of ExceptOnTerrainOrLayerFilter and OnlyOnTerrainOrLayerFilter to address the issue of filter operation return values.

  2. Modify the judgment criteria of the orHigher() and orLower() methods to check whether they are set simultaneously based on whether the original value is QUAL or null.

Testing

The JavaScript script for testing is as follows( Assuming the existence of `TestLayer'):

var layer = wp.getLayer()
    .withName('Swamp')
    .go();

var layer0 = wp.getLayer()
    .fromWorld(world)
    .withName('TestLayer')
    .go();

var filter = wp.createFilter()
    .onlyOnLayer(layer).withValue(8).orLower()
    .exceptOnLayer(layer0).withValue(7).orHigher()
    .go();

print(filter.toString())

wp.applyTerrain(3)
    .toWorld(world)
    .withFilter(filter)
    .applyToSurface()
    .go();

by the way

I am currently attempting to rewrite the WPScriptAPI documentation, filling in the gaps left by the old documentation, including sections on how to initialize UI parameters in scripts. Since English is not my first language, this might take some time, but once I'm finished, I'll promptly submit this documentation to the repository. I would greatly appreciate it if you could help review the content.

Additionally, I'm also trying to add some interesting controls to the UI parameters feature of WPScript, including combox and sliders, among others. However, these are still a work in progress. If I complete them, would you be open to incorporating these changes? Cache_7e81a196aed9d17b