Move TileMap layers to their own class

[groud]

Sorry for the huge PR, but it, hopefully, does not change the TileMap behavior. It does the following:

• Move layer data and methods to a TileMapLayer class. This helps keeping things more independent from one layer to another. It's kind of needed to cleanly improve performances on following up PRs.
• Make all layer-indexing functions work with negative indices.
• Likely fix a bug with navigation.
• Might improve performances significantly on some cases as I reduced the number of calls to canvas_item_set_draw_index. It might create bugs too as I am not 100% of the change. Though it looks like it worked well on my test project.

Might fix: #77477, to be tested.
Bugsquad edit: Fixes #79553

[KoBeWi]

Tested with my project. Overall seems to work fine and layer modifications are noticeably faster, but something wrong is happening with physics.

I have a condition like this: floor_collision.get_collider().is_in_group(&"wallrun") and it results in an error that is_in_group() method is not found in RefCounted. Then I have this code:

for body in collider.get_overlapping_bodies():
	if body.is_in_group(&"powable"):
		body.do_pow(down)
		powed_any = true

And it results in outright crash:

[0] HashMap<StringName,Node::GroupData,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,Node::GroupData> > >::_lookup_pos (C:\godot_source\core\templates\hash_map.h:104)
[1] HashMap<StringName,Node::GroupData,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,Node::GroupData> > >::has (C:\godot_source\core\templates\hash_map.h:311)
[2] Node::is_in_group (C:\godot_source\scene\main\node.cpp:2077)
[3] call_with_ptr_args_retc_helper<Node,bool,StringName const &,0> (C:\godot_source\core\variant\binder_common.h:339)
[4] call_with_ptr_args_retc<Node,bool,StringName const &> (C:\godot_source\core\variant\binder_common.h:588)
[5] MethodBindTRC<Node,bool,StringName const &>::ptrcall (C:\godot_source\core\object\method_bind.h:598)
[6] GDScriptFunction::call (C:\godot_source\modules\gdscript\gdscript_vm.cpp:1943)
[7] GDScriptInstance::callp (C:\godot_source\modules\gdscript\gdscript.cpp:1791)
[8] Object::callp (C:\godot_source\core\object\object.cpp:717)
[9] Variant::callp (C:\godot_source\core\variant\variant_call.cpp:1174)
[10] GDScriptFunction::call (C:\godot_source\modules\gdscript\gdscript_vm.cpp:1662)
[11] GDScriptInstance::callp (C:\godot_source\modules\gdscript\gdscript.cpp:1791)
[12] Object::callp (C:\godot_source\core\object\object.cpp:717)
[13] Variant::callp (C:\godot_source\core\variant\variant_call.cpp:1174)
[14] GDScriptFunction::call (C:\godot_source\modules\gdscript\gdscript_vm.cpp:1662)
[15] GDScriptInstance::callp (C:\godot_source\modules\gdscript\gdscript.cpp:1791)
[16] Node::_gdvirtual__physics_process_call<0> (C:\godot_source\scene\main\node.h:319)
[17] Node::_notification (C:\godot_source\scene\main\node.cpp:61)
[18] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[19] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[20] Node2D::_notificationv (C:\godot_source\scene\2d\node_2d.h:37)
[21] CollisionObject2D::_notificationv (C:\godot_source\scene\2d\collision_object_2d.h:40)
[22] PhysicsBody2D::_notificationv (C:\godot_source\scene\2d\physics_body_2d.h:42)
[23] CharacterBody2D::_notificationv (C:\godot_source\scene\2d\physics_body_2d.h:327)
[24] Object::notification (C:\godot_source\core\object\object.cpp:798)
[25] SceneTree::_process_group (C:\godot_source\scene\main\scene_tree.cpp:934)
[26] SceneTree::_process (C:\godot_source\scene\main\scene_tree.cpp:1022)
[27] SceneTree::physics_process (C:\godot_source\scene\main\scene_tree.cpp:468)
[28] Main::iteration (C:\godot_source\main\main.cpp:3382)
[29] OS_Windows::run (C:\godot_source\platform\windows\os_windows.cpp:1479)
[30] widechar_main (C:\godot_source\platform\windows\godot_windows.cpp:182)
[31] _main (C:\godot_source\platform\windows\godot_windows.cpp:204)
[32] main (C:\godot_source\platform\windows\godot_windows.cpp:218)
[33] WinMain (C:\godot_source\platform\windows\godot_windows.cpp:232)
[34] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[35] <couldn't map PC to fn name>

In both cases I'm accessing a TileMap collider, which seems to be RefCounted. Did the collision change from TileMap node to TileMapLayer refcounted?

[groud]

In both cases I'm accessing a TileMap collider, which seems to be RefCounted. Did the collision change from TileMap node to TileMapLayer refcounted?

Yes, but the TileMapLayer is refcounted too, so that should not be a problem ?

I am not really sure how to reproduce the bug though, seems like get_coords_for_body_rid() and get_layer_for_body_rid() work fine for me. 🤔

[groud]

Ah nevermind, found the issue! That was an oversight with a call to get_instance_id() instead of tile_map_node->get_instance_id().

Can you try again ?

[KoBeWi]

I can't test right now, will do tomorrow.

[KoBeWi]

The error is fixed.

[KoBeWi]

I did not review the whole code, because GitHub is super slow with this many changes. Though I assume it's mostly a copy paste of the previous code.

I didn't spot any regressions from testing with my project, but I don't use all TileMap features. Most notably I didn't test terrains and navigation.

[YuriSizov]

Approving in principle, the nitpick needs fixing.

This also breaks compatibility with GDExtension by making get_used_rect const. This needs to be handled gracefully.

[groud]

This also breaks compatibility with GDExtension by making get_used_rect const. This needs to be handled gracefully.

Does it though ? I am not sure it has any big implication on either the C or the C++ bindings, besides removing errors about not respecting const-ness. I mean that, theoretically, you don't need any change to your code to make things work.

[YuriSizov]

Does it though ?

It does at least in the sense that we include const-ness in the hash calculation. So you get a hash mismatch, which I think means that ABI compatibility is going to be broken for that method.

[YuriSizov]

The new compat changes seem correct 👍

[YuriSizov]

Thanks! Now the real work begins 🙃