Add "Explicit Start Frame" tile animation mode.

[rakkarage]

Closes: godotengine/godot-proposals#7461

TODO:

• Works, but only till quit and reopen. Not persisting HashMap<uint32_t, int> animation_explicit_start_frames.
  • Tried moving it to TileData or TileAlternativesData a few times but could not figure it out... hoping to get some feedback about where and how to store the data.
• Can't overwrite explicit, even after delete. So only really works on tiles that were empty when map opened.
  • Will probably 'fix itself' when first issue is solved. Deciding where and how to store the data.
• Ensure it works from code too.

Thanks.

[rakkarage]

Run ./misc/scripts/validate_extension_api.sh "./bin/godot.linuxbsd.editor.x86_64.mono"
2023-09-28 01:01:31 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.0-stable/gdextension/extension_api.json [5175722/5175722] -> "/tmp/tmp.36TKg1GCjf" [1]
Godot Engine v4.2.dev.mono.custom_build.8e5ec352f - https://godotengine.org
 
ERROR: New API lacks base array: enums
   at: compare_dict_array (core/extension/extension_api_dump.cpp:1135)
Error: Validate extension JSON: Error: Field 'classes/TileMap/methods/set_cell/arguments': size changed value in new API, from 5 to 6.

2023-09-28 01:01:32 URL:https://raw.githubusercontent.com/godotengine/godot-cpp/godot-4.1-stable/gdextension/extension_api.json [5318978/5318978] -> "/tmp/tmp.LPaoKVdLfM" [1]
Godot Engine v4.2.dev.mono.custom_build.8e5ec352f - https://godotengine.org
 
ERROR: New API lacks base array: enums
   at: compare_dict_array (core/extension/extension_api_dump.cpp:1135)
Error: Validate extension JSON: Error: Field 'classes/TileMap/methods/set_cell/arguments': size changed value in new API, from 5 to 6.

Error: Process completed with exit code 1.

added explicit_start_frames param to set_cell
getting an error in action...
and i ran .\godot.windows.editor.dev.x86_64.exe --dump-extension-api

				{
					"name": "set_cell",
					"is_const": false,
					"is_vararg": false,
					"is_static": false,
					"is_virtual": false,
					"hash": 2261257222,
					"hash_compatibility": [
						1732664643
					],
					"arguments": [
						{
							"name": "layer",
							"type": "int",
							"meta": "int32"
						},
						{
							"name": "coords",
							"type": "Vector2i"
						},
						{
							"name": "source_id",
							"type": "int",
							"meta": "int32",
							"default_value": "-1"
						},
						{
							"name": "atlas_coords",
							"type": "Vector2i",
							"default_value": "Vector2i(-1, -1)"
						},
						{
							"name": "alternative_tile",
							"type": "int",
							"meta": "int32",
							"default_value": "0"
						},
						{
							"name": "explicit_start_frame",
							"type": "int",
							"meta": "int32",
							"default_value": "-1"
						}
					]
				},

to get new hashes but still not work.. guess i did it wrong idk

	mappings.insert("TileMap", {
		{ "set_cell", 1732664643, 966713560 },
		{ "set_cells_terrain_connect", 3072115677, 3578627656 },
		{ "set_cells_terrain_path", 3072115677, 3578627656 },
		{ "get_used_cells_by_id", 4152068407, 2931012785 },
	});

so i should change this to

	mappings.insert("TileMap", {
		{ "set_cell", 966713560, 2261257222 },
		{ "set_cells_terrain_connect", 3072115677, 3578627656 },
		{ "set_cells_terrain_path", 3072115677, 3578627656 },
		{ "get_used_cells_by_id", 4152068407, 2931012785 },
	});

or what? Thanks.

[groud]

Thanks for the contribution.

While I think the feature can be useful, I'd rather avoid bloating the data stored in the TileMap with new things right now. We need to rework the way we store the TileMap data, and using a dictionary here instead of the tile_data property is super inefficient, so it cannot be merged as-is. Additionally, I don't think we can instead add a field in the structure stored in tile_data, as it would mean making maps weight heavier for a feature not everyone need.

For now, I think users will have to use scene tiles for that kind of behavior.

[groud]

I added a private unexposed Dictionary to TileMapLayer not TileMap to keep track of explicit start frames.
I tried storing the each explicit start frame in TileData, but tile data is specific to atlas tiles not map tiles?
Not sure it is bloat to store something that we need store but ok.

I meant the TileMapLayer's "tile_data", the one retrieved with layer->get_tile_data() returning a Vector<int> and stored in the "layer_%d/tile_data" property.

Basically, this is the structure that holds what is stored, per cell, in a layer.

[rakkarage]

union TileMapCell {
	struct {
		int32_t source_id : 16;
		int16_t coord_x : 16;
		int16_t coord_y : 16;
		int32_t alternative_tile : 16;
	};

Ya I considered TileMapCell, and TileData, and TileSetAtlasSource beside tile_animation_mode... but TileMapLayer seems better? It could and should only store data for tiles with animations using this mode? The dictionary is not exposed to scripting just an extra optional param to set_cell.

If anyone can suggest a better place to store it or a better way to do it... How is this "super inefficient bloat"? Storing an int? The other tile animation mode is fine cuz it not store ints?

I guess cuz I am doing a dictionary lookup... But it is not running constantly for all tiles... just happens when drawing tiles in that mode and setting tile in that mode. Sure it would be nice if could integrate into some other tile lookup but idk where. Does not seem to interact at all with users who not use this feature or have any effect on their performance... and users who want this feature might be ok with cost of looking up an int to look up an int? no? lighter then looking up a scene tile? idk sorry

Thanks.

[Proggle]

If anyone can suggest a better place to store it or a better way to do it...

Okay, maybe I'm misunderstanding everything that went into your pull request, but is there a pressing need for it to actually store what the original start frame was as a separate value?

When you implemented randomized tile offsets, there's already an offset value for where in its animation the tile starts out. If we calculate (or precalculate) how many seconds that will take, we can reuse most of the structure used in the random tile code.

Basically, if it's a 10 frame animation which lasts 10 seconds, we just tell it to skip ahead 4 seconds at creation, and then leave it to act like a normal tile.

That'd require more work on the backend in tileset, mapping any tile which has coordinates within an animation to the animation itself, but would avoid expanding the data stored in tilemap at all.

[rakkarage]

Thanks.

Yes both animation modes basically just offset the animation start time eventually, but Random Start Time animation mode is just random so it not need to store anything... it keeps it constant during run time by seeding the random number generator with a hash of the layer & coords, so i just generate a new offset instead of load one. Explicit Start Frame animation mode, needs to keep these exact offsets for each cell using this mode after quit and reload. Random Start Time can discard the data and regen it the same, random.

I think.

[Proggle]

Quit and reload should be the same as initializing the tilemap the first time, right?

Would it be too ugly for the tilemap to just be given the coordinate of a nonfirst frame, and then the tileset handles all the nitty gritty details of assigning it an animation with the right offset start?

So (for instance), if the 3 frame animation contains tiles in the atlas at <1,1> <2,1> and <3,1>, and we want the animation to start at the second frame in <2,1>, can we just store <2,1> in the tilemap and ensure the tileset knows this frame is part of an animation?

That also has the advantage of having a very literal mapping between selecting the tile in an atlas and putting it on the map; if you pick the tile at <2,1> on the tileset atlas it will just put <2,1> in the tilemap, instead of the current situation where it puts in <1,1>.

@groud do you think that would be a good way to implement without bloating the tilemap?

[groud]

@groud do you think that would be a good way to implement without bloating the tilemap?

I believe it could work, but that's probably a lot of work. The TileSet was designed around the fact that one ID = one tile, this is probably very difficult to work around in a safe way.