feat: modifiable map

[Schnurber]

The map should be more flexible. For some game concepts, it could be useful to change the map at runtime.
In the proposed version, you can show and hide layers.

var layer = tiles.tileMap.map.layers[1] as TileLayer;
layer.visible = false;

In addition, you can change the tile data and the cache can be updated afterwards.

layer.tileData![3,2] = const Gid(0, Flips.defaults());
tiles.tileMap.update();

See small example

[spydon]

Looks good to me, just a few fixes that needs to be done.

[spydon]

Looks good! But you haven't run flutter format on it.

[spydon]

@lfraker could you review this small change too? :)

[Schnurber]

OK!

[spydon]

It would be good to have a function that updates the visibilty of a layer and then calls refreshCache, because right now it will be a bit confusing for the user that doesn't know that updateCache needs to be called after a change to the map.

And some more dartdocs on the refreshCache function about when it should be used would be good too.

[Schnurber]

Well, I also added convenience functions to change the tileData and read all values: setLayerVisibility, getLayerVisibility, setTileData, getTileData

[st-pasha]

How do you know that this particular Layer is a TileLayer?

[Schnurber]

Better now?

[st-pasha]

It looks to me that all 4 new methods operate on a layer, selected by its layerId. In this case it is better to have one method in RenderableTileMap to retrieve a layer with the given id, and then all other methods would belong to that layer class. Thus, instead of .setLayerVisibility(layerId, value), we'd have .layer(id).visibility = value;.

[Schnurber]

It looks to me that all 4 new methods operate on a layer, selected by its layerId. In this case it is better to have one method in RenderableTileMap to retrieve a layer with the given id, and then all other methods would belong to that layer class. Thus, instead of .setLayerVisibility(layerId, value), we'd have .layer(id).visibility = value;.

Layer is from the tiled package and has no reference to Flame. Thus, the cache cannot be refreshed from there.

[st-pasha]

Unless you wrap it into your own Layer object (similarly how RenderedTiledMap wraps TiledMap).

[spydon]

It looks to me that all 4 new methods operate on a layer, selected by its layerId. In this case it is better to have one method in RenderableTileMap to retrieve a layer with the given id, and then all other methods would belong to that layer class. Thus, instead of .setLayerVisibility(layerId, value), we'd have .layer(id).visibility = value;.

How do you propose this should be done without creating new objects, an extension? We also need a reference to the RenderableTileMap in there so that the cache can be updated after a change. I agree that they all could be grouped as a layer, but I think the current solution works too.

[Schnurber]

@st-pasha I agree with @spydon That would be difficult. I would prefer a simple solution.
The situation is as follows,

In package tiled there is:

abstract class Layer {
	...
	bool visible;
	...
}

class TileLayer extends Layer {...}

class TiledMap {
...
	List<Tileset> tilesets;
	List<Layer> layers;
...
}

and in flame:

class RenderableTiledMap {
...
	final TiledMap map;

	void refreshCache() {...}

	void setLayerVisibility(int layerId, bool visibility) {
		...
		refreshCache();
		... 
	}
 	
	void setTileData( ... ) { ... }
...
}

Unfortunately, it is not possible to work with extension methods, as the references to RenderableTileMap are not possible this way.
You would have to create subclasses RenderableLayer and RenderableTilset in flame, change RenderableTiledMap and define it as a subclass of TiledMap. It would then be complicated because you would have to convert the nested list entries List<Tileset> and List<Layer> layers into lists filled with subclasses of type RenderableTilset and RenderableLayer. You would actually have to generate everything new to provide it with references to the RenderableTileMap.

[spydon]

@Schnurber did you see my response on the suggestion with the null check?

[erickzanardo]

LGTM