Many new features and tweaks

[torcado194]

Hi there!

I want to start this off by saying I love this plugin, I think it's entirely more intuitive than the built-in tilemap system. I started off learning godot with version 4.0 almost solely because of the purportedly improved tile system, and I still haven't gotten used to it. After using this plugin for only a day, I've completely grasped how to use it effectively for my needs.

I began these modifications as separate git branches, but it became too much of a hassle to manage and I ended up writing everything at once. If you would like me to split this into different PRs I can try to do so.

Changes

Improved painting

Painting now draws unbroken lines when moving the cursor quickly. Here are some before and after examples:

Before	After

This also works when painting tile peering sides.

Note: This doesn't create perfect unbroken lines with tile shapes other than square. There's an alternative algorithm that was written for the new tileset system internally that's used for other tile shapes, I'll look into implementing this later.

Line paint tool

Selecting the Line tool, or holding Shift with the normal Paint tool active, will draw lines. This mirrors the built-in editor behavior.

Note: like with the improved painting, lines are slightly wrong with non-square tile shapes

Overwrite painting mode

A new button to the right of the paint tools toggles "overwrite" mode. While this mode is active, drawing will only change tiles on the tilemap if there is a tile with a matching set of peering sides in the current terrain.
This allows you to easily manage alternative tilesets, such as a tileset with slopes instead of square corners, or a tileset of alt decoration tiles.

Select and Copy/Paste/Delete terrain peering

A new button on the terrain side allows you to select terrain tiles. Pressing Delete will delete the tiles' peering sides. Pressing Ctrl+C or Ctrl+X will copy or cut the tiles, and Ctrl+V will put the tiles in a staging area on your cursor. Clicking will then overwrite any peering data with that of the staged tiles that were copied.
This allows you to easily duplicate a tileset's peering sides, for instance if you have many tilesets with the same layout, without requiring you to draw it all again.

Ctrl-scroll zoom keybind

You can now hold ctrl while scrolling to zoom the tileset canvas.

More clear terrain tiles

This change puts an outline around the tiles selected for the current terrain, just to make it more clear what the current terrain tiles are without needing to look for the matching color between the tile centers and the terrain icon. You can see this change in some of the gifs above.

---

These are mostly just changes I wanted for my own sake, but I figured I should make a PR for them :) Feel free to suggest any changes or additions! There are likely places in the code that could be improved, I tried my best to mimic the existing style and understand the internal systems.

Thanks! ❤️

[Portponky]

Wow, these are some huge QOL improvements. Thanks so much for taking the time to do this. I'll check through the code tonight and give any feedback.

[Portponky]

Hi, I've had a look over the code. Great work. There are a few changes I could suggest, or if you would prefer, I could merge the PR as it is, and make those changes myself. If you would like to see what changes I have in mind, I can make specific comments. It's certainly nothing major, just a few tweaks. Let me know what you would prefer.

[torcado194]

Thanks for the review!

I would be interested in hearing what changes you have in mind :)

[torcado194]

These should all be fixed now :) (apologies if resolving the github reviews myself wasn't the correct way to do it)

[Portponky]

That's great. I think you resolved the first few without supplying a patch though. If you don't get around to fixing those, I can fix them afterwards as it's just minor refactoring. I'll give the code a test with a bunch of different tilesets tomorrow and if it has no major issues, I will merge it. :+1:

[torcado194]

Oops my bad, I didn't commit the file. Should be good now.

Sounds good, thanks again! :)

[torcado194]

one more small update: painting/drawing lines now works correctly with other tile shapes

(tiles look silly because they aren't actual hexagons, just squares. but you get the idea)

[Portponky]

Been very busy the last couple days, sorry about that. I've been experimenting with the overwrite mode, and it doesn't seem to do what I expect it to do (or it has some issue?) I'll continue to investigate.

[torcado194]

No worries! with overwrite mode, it only works on tiles within the terrain you're painting with that have matching peering sides. so like here, both terrains include both tilesets

if that's not the case then there's probably an issue.

This behavior could very well be changed, it might be better to have "matching tiles" check for peering sides of any id, not matching ids (so, a slope tile from any terrain can replace a corner tile from any terrain, because they both have the same 3 peering sides enabled somewhere)

The method I chose was mostly arbitrary, both would solve the usecase I mentioned in the original post.

[Portponky]

There's a few bugs in the overwrite code. The most important is that dictionary keys are not hashed in a stable manner; each dictionary gets a randomization to its hash function. That means that two dictionaries with the same keys will (potentially) have them in different orders.

The output of tile_peering_for_type was therefore in dictionary key order. This means that two tiles with the same peering bits might differ in order: [0, 8] compared to [8, 0]. As the replace cell(s) function does a direct comparison (which is good) this caused false negatives. This is easy to correct with a sort at the end of tile_peering_for_type:

    result.sort()
    return result

Another bug is that the replace cell(s) function skips tiles with no peering bits. However, it's totally possible that you would want to overwrite single tiles, so this skip should not occur.

Along with these additions, some speedups can be made, namely caching the peering types for the multi-cell function, and breaking when finding a match to prevent unnecessary checks. Also, eliminating some variables (GDScript, as a rule of thumb, tends to run faster with fewer lines of code, even if those lines are slightly denser). Here are my suggested functions:

## Replaces an existing tile on the [TileMap] for the [code]layer[/code]
## and [code]coord[/code] with a new tile in the provided terrain [code]type[/code] 
## in [TileSet] [code]ts[/code] *only if* there is a tile with a matching set of 
## peering sides in this terrain.
## Returns [code]true[/code] if any tiles were changed. Use [method replace_cells]
## to replace multiple tiles at once.
func replace_cell(tm: TileMap, layer: int, coord: Vector2i, ts: TileSet, type: int) -> bool:
    if !tm or !tm.tile_set or layer < 0 or layer >= tm.get_layers_count() or type < 0:
        return false

    var cache := _get_cache(tm.tile_set)
    if type >= cache.size():
        return false

    if cache[type].is_empty():
        return false

    var td = tm.get_cell_tile_data(layer, coord)
    if !td:
        return false

    var placed_peering = tile_peering_for_type(td, type)
    for pt in get_tiles_in_terrain(ts, type):
        var check_peering = tile_peering_for_type(pt, type)
        if placed_peering == check_peering:
            var tile = cache[type].front()
            tm.set_cell(layer, coord, tile[0], tile[1], tile[2])
            return true

    return false

## Replaces existing tiles on the [TileMap] for the [code]layer[/code]
## and [code]coords[/code] with new tiles in the provided terrain [code]type[/code] 
## in [TileSet] [code]ts[/code] *only if* there is a tile with a matching set of 
## peering sides in this terrain for each tile.
## Returns [code]true[/code] if any tiles were changed.
func replace_cells(tm: TileMap, layer: int, coords: Array, ts: TileSet, type: int) -> bool:
    if !tm or !tm.tile_set or layer < 0 or layer >= tm.get_layers_count() or type < 0:
        return false

    var cache := _get_cache(tm.tile_set)
    if type >= cache.size():
        return false

    var changed = false
    var potential_peering = get_tiles_in_terrain(ts, type).map(
        func(tile): return tile_peering_for_type(tile, type)
    )
    for c in coords:
        var td = tm.get_cell_tile_data(layer, c)
        if !td:
            continue

        var placed_peering = tile_peering_for_type(td, type)
        for check_peering in potential_peering:
            if placed_peering == check_peering:
                var tile = cache[type].front()
                tm.set_cell(layer, c, tile[0], tile[1], tile[2])
                changed = true
                break

    return changed

With these changes, the overwrite appears to be working exactly as you describe. If you are happy with this, feel free to update the PR. I'm going to test out the other edge cases in the meantime. Thanks!

Edit: Fixed oversight in replace_cell above

[univeous]

These new features look great. Personally, I'd want to go beyond just copying and pasting peering data and have the ability to save them somewhere so we can summon them from the void. This would make it very easy to set up new peering data on different tilesets.

[Portponky]

Overwrite mode doesn't work too well with types defined by categories :thinking: so I will have a think about an improvement in that regard.

[torcado194]

Oh, thanks for spotting those! Yeah without knowing how the internal structures were handled I was just making my best guess referencing the other functions.

You're right about not skipping tiles with no peering sides. It feels a little weird to me, I guess because I'm mentally considering the "painted tiles" in the terrain as potential candidates for what i want to replace, and this change opens up the whole tileset for that check. But I think it's a correct change, I can't come up with any reasons why it shouldn't be that way.

Yeah, categories make it tricky. While typing up my thought process I ended up implementing a fix anyway. The new push has changes in replace_cell(s) so that it now includes all categories the provided terrain is in for its match checks (and conceptually it would check against categories the placed tile is in, but if they aren't part of the same category then they shouldn't match anyway, so it only collects categories from the provided terrain)

I wasn't sure how to best integrate your replace_cells changes into the new code, or if it even needs it now. Let me know your thoughts on that, and on the new process in general.

[torcado194]

These new features look great. Personally, I'd want to go beyond just copying and pasting peering data and have the ability to save them somewhere so we can summon them from the void. This would make it very easy to set up new peering data on different tilesets.

When first thinking about how to implement this, I was considering how to serialize this data to store in the clipboard. But I tested other parts of the editor and discovered that they also just use internal memory and don't store anything to the clipboard (e.g. copying nodes in the tree), so I just followed that convention. That serialization would be necessary for saving out terrains, I'd have to think about it more. Maybe Portponky would have a better idea of how to handle that.

[univeous]

When first thinking about how to implement this, I was considering how to serialize this data to store in the clipboard. But I tested other parts of the editor and discovered that they also just use internal memory and don't store anything to the clipboard (e.g. copying nodes in the tree), so I just followed that convention. That serialization would be necessary for saving out terrains, I'd have to think about it more. Maybe Portponky would have a better idea of how to handle that.

Sounds reasonable. After some thought, I don't think the feature I mentioned is as important as I thought it was before. Of course being able to have that feature would still make people's lives a little easier. My other complaint is that we should make the default function of the scroll wheel zoom, and only scroll up and down when holding ctrl, which would be consistent with the default tileset and tilemap panel.

[Portponky]

My other complaint is that we should make the default function of the scroll wheel zoom, and only scroll up and down when holding ctrl, which would be consistent with the default tileset and tilemap panel.

Unfortunately this makes it pretty unusable on a laptop touchpad. Most other apps use ctrl as a zoom modifier; it is the default panels that are wrong here imo.

[Portponky]

This looks pretty good overall, so I will try to merge it this evening! Great work.

[torcado194]

Great, thanks a lot! :)