Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/filter in script #380

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ALingll
Copy link

@ALingll ALingll commented Jan 29, 2024

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();
  1. The orHigher() / orLower() methods of CreateFilterOp are always unusable:

The JavaScript code that caused the error is as follows:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]filter(Continuous layer with value filter) don‘t work well in script
1 participant