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

Scripting: LayerEdit.apply() doesn't trigger Automap While Drawing #4016

Open
eishiya opened this issue Jul 20, 2024 · 0 comments
Open

Scripting: LayerEdit.apply() doesn't trigger Automap While Drawing #4016

eishiya opened this issue Jul 20, 2024 · 0 comments
Labels
bug Broken behavior.

Comments

@eishiya
Copy link
Contributor

eishiya commented Jul 20, 2024

Modifying a TileMap with LayerEdit.apply() doesn't trigger Automap While Drawing. Changes made with TileMap.merge() do trigger it.

Here's my code that handles click+drag painting, this is part of Tool.tilePositionChanged. Since the mouse is held down and no preview is necessary, this code paints to the layer immediately instead of creating and merging a preview.

if(this.paintingDrag) {
    let line = Geometry.pointsOnLine(this.lastPosition, newPosition);
    let layer = this.map.selectedLayers[0];
    let layerEdit = layer.edit();
    layerEdit.mergeable = true; //This doesn't affect the bug, it's the same with mergeable = false too.
    let fullTile = this.getSlopeTile(0, 0, Qt.AlignBottom); //this just tells us which tile to paint
    for(let point of line) {
        layerEdit.setTile(point.x, point.y, fullTile? fullTile.tile : null, fullTile? fullTile.flags : 0);
    }
    layerEdit.apply();
}

Here's what the behaviour looks like, with an Automapping rule that replaces the green tile with a brown tile. In this case, one tile gets Automapped, the start of the stroke - this single tile is placed with TileMap.merge() in mousePressed, rather than LayerEdit.apply(). The tiles that remain green are the ones placed by apply() in the snippet above.
Automapping with apply()
For comparison, here is the same green tile being drawn with Tiled's Stamp Brush tool rather than with a scripted tool. This is the behaviour I expected even with a scripted tool, all the tiles get Automapped:
Automapping with Stamp Brush

Edit: On Discord, bjorn mentioned that he was reluctant to emit the regionEdited signal (which is used for Automap While Drawing) on apply() due to the potential for infinite loops as scripts can also listen to this signal. However, this issue exists with map.merge() as well, which does emit it. A potential solution is to provide a parameter for apply() and merge() that determines whether regionEdited is emitted. This way, scripts that don't listen for the signal (such as tools) can emit it if they want, while scripts that listen for regionEdited have a way to avoid emitting it.

@eishiya eishiya added the bug Broken behavior. label Jul 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken behavior.
Projects
None yet
Development

No branches or pull requests

1 participant