Expose TileMapLayer

[groud]

Alright. This PR finally exposes the TileMapLayer node.

It does multiple things:

• Expose the TileMapLayer nodes as individual nodes.
• Adds two buttons to select previous/next layer (work with TileMap or TileMapLayers).
• Adds a "select all layers" button (works with TileMapLayers only)
• Adds an advanced menu option to extract the layers from a TileMap and add them as children of that TileMap node.
• Remove the TileMapLayerGroup node, as it is mostly useless with the changes above.

All editor and tile map feature should more or less work now.

Edit: a video (a bit outdated though, I fixed most issues in this PR comments):

2024-03-27.14-34-26.mp4

Old info

I am still wondering about making some last moment changes though:

• I added a tile_set field to each layer in case you do not need to use multiple layers, and just want to use a single layer without the parent TileMapLayerGroup. This is useful IMO, but it might make the tile_set setup a bit more confusing.
• That make me wonder if having the tile_set in TileMapLayerGroup is that useful after all. Which would mean that this node would barely hold anything (only the selected layers). So maybe, in the end, the TileMapLayer editor should simply store itself the selection status, and update a node's siblings (for highlighting the selected layer) instead. We could remove the TileMapLayerGroup node that way.

In general, I think this new node is quite an improvement. But my main concern is that it makes setting up layers that would have similar properties a bit more cumbersome. Things like "show debug collision" has to be set up per layer, which is a bit more complicated I believe.

A video:

Peek.05-03-2024.14-15.mp4

[KoBeWi]

Is this intended that layer dropdown is available when a TileMapLayer is selected?

EDIT:
Apparently you can edit layers while they are invisible. The editor doesn't show anything, but the tiles are actually added when you paint them.

[groud]

Is this intended that layer dropdown is available when a TileMapLayer is selected?

I guess. I think it's something you mentioned you preferred to keep to avoid inspector updates. But it's true that it might be a bit weird, and you can use the parent TileMapLayerGroup node instead if you need fast updates. So well, I can remove it, I don't mind either way.

Apparently you can edit layers while they are invisible. The editor doesn't show anything, but the tiles are actually added when you paint them.

Edit: ah. Interesting. I guess we should disable the editor in that case.

[KoBeWi]

Yeah when you have a specific TileMapLayer selected, being able to switch to other layers might be confusing. That option should be available only for TileMap and TileMapLayerGroup.

[KoBeWi]

Another thing, which can be done in a follow-up and I could add it myself, is being able to convert TileMaps to TileMapLayerGroups. While I don't care that much about layer nodes, being able to hide them is useful and more convenient/efficient than disabling. Of course there is an option to add layer visibility to TileMap, but I think we shouldn't add more functionality to it anymore.

But my main concern is that it makes setting up layers that would have similar properties a bit more cumbersome. Things like "show debug collision" has to be set up per layer, which is a bit more complicated I believe.

MultiNodeEdit solves that.

[groud]

Another thing, which can be done in a follow-up and I could add it myself, is being able to convert TileMaps to TileMapLayerGroups.

Yes, that's definitely something we should add as soon as possible I agree. But it should be fairly straightforward to implement, so I didn't bother for now.

MultiNodeEdit solves that.

Oh yeah, I know it's possible, it's just a bit more cumbersome to edit I believe. But yeah, I don't know how users would feel about it. I feel like we could maybe remove TileMapLayerGroup completely in the end 🤔, not sure if it would help much though.

[KoBeWi]

btw I checked how slow is switching between layer nodes and it's not bad actually, even in dev build. So I guess inspector updates aren't really a problem (other than they exist and waste your CPU xd).

I think TileMapLayerGroup still makes sense because layer switching is convenient and the dropdown doesn't make much sense in layers themselves (though maybe a shortcut for previous/next layer could work and select them in scene, idk). Still, it's a bit awkward that it has literally one property, I wonder if more settings could be delegated to the group and layers would set "overrides".

[groud]

I think TileMapLayerGroup still makes sense because layer switching is convenient and the dropdown doesn't make much sense in layers themselves (though maybe a shortcut for previous/next layer could work and select them in scene, idk). Still, it's a bit awkward that it has literally one property, I wonder if more settings could be delegated to the group and layers would set "overrides".

The thing is that, I believe, we could easily keep all current behavior we have. Like, we could keep the shortcuts, the highliting and all (the only difference is that the highlighting will require the editor to store those values, instead of the parent node). That should be doable without this TileMapLayerGroup node.

So yeah, that's kind of the whole question. Either we try to add more properties in the TileMapLayerGroup node (and have per-layer overrides), but it might end up more complicated to maintain and is likely more complex to understand. Or we ask a bit more work from users (copy-pasting the TileSet, editing multiple nodes at once for some properties, etc...) but it makes the whole system a lot simpler.

Maybe we should do a poll or something ?

[KoBeWi]

Maybe we should do a poll or something ?

Might be a good idea.

I think having a single node that configures all layers is useful. But it should only have properties that normally you'd set to the same value for all layers.

[KoBeWi]

There's some bug with layer TileSet. If you set it to any value, even the same as parent group, selecting the layer is like 10x slower. Also happens when layer isn't under a group.

EDIT:
Profiler points to TileSetAtlasSource::_get_property_list. TileMapLayerGroup actually has the same issue.

[KoBeWi]

Another bug with TileMapLayer: stand-alone layers load without a TileSet.
EDIT:
For some reason I can't reproduce it now :I

[AThousandShips]

I believe this looks good, will take a proper look at some of the changed logic tomorrow but seems proper and safe

[Mickeon]

In its own docs, whenever TileMapLayer itself is mentioned exactly by name, it should be surrounded by square brackets ([TileMapLayer]) for a self-reference (can't take a deeper look at the moment).

Alternatively, in my opinion, calling it "layer" instead of the whole class name would be sufficient and less "verbose".

[Mickeon]

I was already very peeved at the TileMap/TileSet documentation in the past. But I can chime for a few things here, just to make TileMapLayer slightly more appealing.

[Mickeon]

Regardless of how you feel about it, another huge problem is proxies have never been properly documented in the past. Lack of documentation and no direct, "tangible" consequence when the argument is true, and they may as well be regarded as "useless" . I had to skim through blog posts to figure out how they worked (and I still could not manage to have them working as intended either).

[groud]

Alright, I've just pushed a change that does two thing:

• deprecate the use_proxies argument for TileMap (it now does nothing with TileMap, we could keep compat just in case but I am not sure it's worth the trouble, as it does not look like people use it anyway)
• remove use_proxies from TileMapLayer.

[Mickeon]

Yeah, as I implied before, I'm concerned about this behavior, at least at project run-time. It makes generating TileMaps at runtime noticeably slower, as well as additional dynamic updates. It does not help that there's no way to know what changed in the tile map exactly.

May be worth reconsidering this separately later. What can be done in these situations is calling Object.set_block_signals before massive TileMap changes.

Suggested change

	This signal might be emitted very often when batch modifying a TileMap. Avoid executing complex processing in a connected function, and consider delaying it to the end of the frame instead (i.e. calling [method Object.call_deferred]).
	[b]Note:[/b] This signal may be emitted very often when batch-modifying a [TileMapLayer]. Avoid executing complex processing in a connected function, and consider delaying it to the end of the frame instead (i.e. calling [method Object.call_deferred]) or preventing this signal from being emitted with [method Object.set_block_signals].

[groud]

Yeah, as I implied before, I'm concerned about this behavior, at least at project run-time. It makes generating TileMaps at runtime noticeably slower, as well as additional dynamic updates. It does not help that there's no way to know what changed in the tile map exactly.

What do you mean it makes things noticeably slower ? Have you tried it ? If you don't connect to it, the cost of emitting a signal is close to zero if I am not mistaking. It was already like that with TileMap and I don't think it was ever a bottleneck.

[akien-mga]

• deprecate the use_proxies argument for TileMap (it now does nothing with TileMap, we could keep compat just in case but I am not sure it's worth the trouble, as it does not look like people use it anyway)

"Deprecating" means keeping the functionality working, but notifying that it might be removed. Keeping the API but without its effect is "obsoleting", and in that case it would be more useful to outright break compatibility than to let the API work but projects break at runtime due to silent removal of the functionality.

So I would add back the code to make it correctly deprecated.

[groud]

So I would add back the code to make it correctly deprecated.

Done. I also re-added the documentation accordingly.

I didn't bother mentioning the argument is deprecated, as the whole TileMap node is marked as deprecated anyway.

[akien-mga]

Thanks!

[874808039]

The new TilemapLayer node cannot yet cooperate with NavigationRegion2D and cannot bake navigation polygons.

[nemoDreamer]

Is there any plan to match all the functionality we used to have on TileMap?

For me, this is the main reason to have an official TileMapLayerGroup:

we're missing all the methods that used to aggregate/merge cell usage across layers, like

• get_used_cells
• get_used_rects

and the simple helpers like

• get_layer_z_index
• get_layers_count

Doing local_to_map / map_to_local now requires cycling through children of a "dumb" group Node until one finds a first TileMapLayer (and pray that its transform is the same as the other layers...)

"simple" actions like copying a map from one TileMap to another now take all sorts of jumping through hoops, as there's no overarching node that knows "I'm a map with layers" 😢

Yes, one could write a class of Node2D that does all of this, but why re-invent the wheel when TileMap could have become this simple group node?

[groud]

Is there any plan to match all the functionality we used to have on TileMap?

Not yet. But I guess we could have a "group" node if there's a huge need for it. That might require a proposal though.

we're missing all the methods that used to aggregate/merge cell usage across layers, like

That's a fair point. combining those manually should not be too hard in general, but I agree it's a bit tedious.

get_layer_z_index

You can just get the node's z-index now (like `$TileMapLayer.z_index), not sure how it would help.

get_layers_count

You can just use a parent node and use get_child_count here, I don't think it deserves a helper.

[nemoDreamer]

Thanks for the response!

I guess I'm just surprised that the proposal to remove TileMap didn't consider the TileMap-level methods.

What's the Godot procedure for getting these approved? Just PR-based, or is there a larger community discussion? I'll make sure I'm more up-to-date 😄

[groud]

What's the Godot procedure for getting these approved? Just PR-based, or is there a larger community discussion? I'll make sure I'm more up-to-date 😄

You can create a proposal in the godot-proposal repository, and see if there's traction for it.