Skip to content

Commit

Permalink
fix: TiledComponent now can be safely loaded regardless of the order (f…
Browse files Browse the repository at this point in the history
…lame-engine#2391)

Basically, the TiledAtlas must not be reused without clone() because it includes batch which is different for each TiledAtlas.
But the following snippets(before this PR) can lead to the reuse of the TiledAtlas if I have loaded the map with the same tilesets(same atlasKey).

   return atlasMap[key] ??= TiledAtlas._(...); 
Also, this PR includes the change of the TestAssetBundle.
I don't know about the exact intention of it.
But it can load only a single map and also it ignored the actual file name to the bundle's first file name.
This makes tests to be hard to debug.

This change is quite far from the original one. So please feel free to tell me to divide it to the separate PR.
  • Loading branch information
Hwan-seok authored and eukleshnin committed Mar 12, 2023
1 parent d9d54b8 commit 7bc2fdc
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 27 deletions.
9 changes: 8 additions & 1 deletion packages/flame_tiled/lib/src/tile_atlas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import 'package:meta/meta.dart';
import 'package:tiled/tiled.dart';

/// One image atlas for all Tiled image sets in a map.
///
/// Please note that [TiledAtlas] should not be reused without [clone] as it may
/// have a different [batch] instance.
class TiledAtlas {
/// Single atlas for all renders.
// Retain this as SpriteBatch can dispose of the original image for flips.
Expand Down Expand Up @@ -100,7 +103,11 @@ class TiledAtlas {
// really boring map.
final image = (await Flame.images.load(key)).clone();

return atlasMap[key] ??= TiledAtlas._(
// There could be a special case that a concurrent call to this method
// passes the check `if (atlasMap.containsKey(key))` due to the async call
// inside this block. So, instance should always be recreated within this
// block to prevent unintended reuse.
return atlasMap[key] = TiledAtlas._(
atlas: image,
offsets: {key: Offset.zero},
key: key,
Expand Down
11 changes: 11 additions & 0 deletions packages/flame_tiled/test/assets/single_tile_map_1.tmx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<map version="1.8" tiledversion="1.8.4" orientation="orthogonal" renderorder="right-down" width="1" height="1" tilewidth="16" tileheight="16" infinite="0" nextlayerid="2" nextobjectid="1">
<tileset firstgid="1" name="4_color_sprite" tilewidth="16" tileheight="16" tilecount="1" columns="1">
<image source="4_color_sprite.png" width="16" height="16"/>
</tileset>
<layer id="1" name="Tile Layer 1" width="1" height="1">
<data encoding="csv">
1
</data>
</layer>
</map>
11 changes: 11 additions & 0 deletions packages/flame_tiled/test/assets/single_tile_map_2.tmx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<map version="1.8" tiledversion="1.8.4" orientation="orthogonal" renderorder="right-down" width="1" height="1" tilewidth="16" tileheight="16" infinite="0" nextlayerid="2" nextobjectid="1">
<tileset firstgid="1" name="4_color_sprite" tilewidth="16" tileheight="16" tilecount="1" columns="1">
<image source="4_color_sprite.png" width="16" height="16"/>
</tileset>
<layer id="1" name="Tile Layer 1" width="1" height="1">
<data encoding="csv">
2147483649
</data>
</layer>
</map>
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
33 changes: 23 additions & 10 deletions packages/flame_tiled/test/test_asset_bundle.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,46 @@ import 'package:flutter/services.dart' show CachingAssetBundle;
class TestAssetBundle extends CachingAssetBundle {
TestAssetBundle({
required this.imageNames,
required this.mapPath,
required this.stringNames,
});

final List<String> imageNames;
final String mapPath;
final List<String> stringNames;

@override
Future<ByteData> load(String key) async {
final pattern = RegExp(r'assets/images/(\.\./)*');
final split = key.split('/');
final imgName = split.isNotEmpty ? key.replaceFirst(pattern, '') : key;

var toLoadName = key.replaceFirst(pattern, '');
if (!imageNames.contains(imgName) && imageNames.isNotEmpty) {
toLoadName = imageNames.first;
final toLoadName = key.replaceFirst(pattern, '');
final fileName = 'test/assets/$toLoadName';

if (!imageNames.contains(imgName)) {
throw StateError(
'No $fileName found in the TestAssetBundle. Did you forget to add it?',
);
}
return File('test/assets/$toLoadName')
return File(fileName)
.readAsBytes()
.then((bytes) => ByteData.view(Uint8List.fromList(bytes).buffer));
}

@override
Future<String> loadString(String key, {bool cache = true}) {
var toLoad = mapPath;
if (key.endsWith('tsx')) {
toLoad = key.replaceFirst('assets/tiles/', 'test/assets/');
final pattern = RegExp(r'assets/tiles/(\.\./)*');
final split = key.split('/');
final mapName = split.isNotEmpty ? key.replaceFirst(pattern, '') : key;

final toLoadName = key.replaceFirst(pattern, '');
final fileName = 'test/assets/$toLoadName';

if (!stringNames.contains(mapName)) {
throw StateError(
'No $fileName found in the TestAssetBundle. Did you forget to add it?',
);
}
return File(toLoad).readAsString();

return File(fileName).readAsString();
}
}
54 changes: 53 additions & 1 deletion packages/flame_tiled/test/tile_atlas_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ void main() {
'images/orange.png',
'images/peach.png',
],
mapPath: 'test/assets/isometric_plain.tmx',
stringNames: [
'isometric_plain.tmx',
'tiles/isometric_plain_1.tsx',
],
);
});

Expand Down Expand Up @@ -119,5 +122,54 @@ void main() {
);
});
});

group('Single tileset map', () {
setUp(() async {
TiledAtlas.atlasMap.clear();
Flame.bundle = TestAssetBundle(
imageNames: [
'4_color_sprite.png',
],
stringNames: [
'single_tile_map_1.tmx',
'single_tile_map_2.tmx',
],
);
});

test(
'''Two maps with a same tileset but different tile alignment should be rendered differently''',
() async {
final components = await Future.wait([
TiledComponent.load(
'single_tile_map_1.tmx',
Vector2(16, 16),
),
TiledComponent.load(
'single_tile_map_2.tmx',
Vector2(16, 16),
)
]);

final atlas = TiledAtlas.atlasMap.values.first;
final imageRendered_1 = renderMapToPng(components[0]);
final imageRendered_2 = renderMapToPng(components[1]);

expect(TiledAtlas.atlasMap.length, 1);
expect(
await imageToPng(atlas.atlas!),
matchesGoldenFile('goldens/single_tile_atlas.png'),
);
expect(imageRendered_1, isNot(same(imageRendered_2)));
expect(
imageRendered_1,
matchesGoldenFile('goldens/single_tile_map_1.png'),
);
expect(
imageRendered_2,
matchesGoldenFile('goldens/single_tile_map_2.png'),
);
});
});
});
}
30 changes: 15 additions & 15 deletions packages/flame_tiled/test/tiled_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ void main() {
setUp(() async {
Flame.bundle = TestAssetBundle(
imageNames: ['map-level1.png', 'image1.png'],
mapPath: 'test/assets/map.tmx',
stringNames: ['map.tmx'],
);
tiled = await TiledComponent.load('x', Vector2.all(16));
tiled = await TiledComponent.load('map.tmx', Vector2.all(16));
});

test('correct loads the file', () async {
Expand Down Expand Up @@ -80,7 +80,7 @@ void main() {
// odd errors if you're trying to debug.
Flame.bundle = TestAssetBundle(
imageNames: ['map-level1.png', 'image1.png'],
mapPath: 'test/assets/map.tmx',
stringNames: ['map.tmx', 'tiles/external_tileset_1.tsx'],
);

final tsxProvider =
Expand Down Expand Up @@ -110,7 +110,7 @@ void main() {
'green_sprite.png',
'red_sprite.png',
],
mapPath: 'test/assets/2_tiles-green_on_red.tmx',
stringNames: ['2_tiles-green_on_red.tmx'],
);
overlapMap = await RenderableTiledMap.fromFile(
'2_tiles-green_on_red.tmx',
Expand Down Expand Up @@ -207,7 +207,7 @@ void main() {
imageNames: [
'4_color_sprite.png',
],
mapPath: 'test/assets/8_tiles-flips.tmx',
stringNames: ['8_tiles-flips.tmx'],
);
overlapMap = await RenderableTiledMap.fromFile(
'8_tiles-flips.tmx',
Expand Down Expand Up @@ -300,7 +300,7 @@ void main() {
imageNames: [
'4_color_sprite.png',
],
mapPath: 'test/assets/8_tiles-flips.tmx',
stringNames: ['8_tiles-flips.tmx'],
);
final tiledComponent = TiledComponent(
await RenderableTiledMap.fromFile(
Expand Down Expand Up @@ -344,7 +344,7 @@ void main() {
setUp(() async {
Flame.bundle = TestAssetBundle(
imageNames: ['map-level1.png'],
mapPath: 'test/assets/layers_test.tmx',
stringNames: ['layers_test.tmx'],
);
_renderableTiledMap =
await RenderableTiledMap.fromFile('layers_test.tmx', Vector2.all(32));
Expand Down Expand Up @@ -396,7 +396,7 @@ void main() {
'image1.png',
'map-level1.png',
],
mapPath: 'test/assets/map.tmx',
stringNames: ['map.tmx'],
);
component = await TiledComponent.load(
'map.tmx',
Expand Down Expand Up @@ -432,7 +432,7 @@ void main() {
imageNames: [
'isometric_spritesheet.png',
],
mapPath: 'test/assets/test_isometric.tmx',
stringNames: ['test_isometric.tmx'],
);
component = await TiledComponent.load(
'test_isometric.tmx',
Expand Down Expand Up @@ -466,7 +466,7 @@ void main() {
imageNames: [
imageFile,
],
mapPath: 'test/assets/$tmxFile',
stringNames: [tmxFile],
);
return component = await TiledComponent.load(
tmxFile,
Expand Down Expand Up @@ -543,7 +543,7 @@ void main() {
imageNames: [
imageFile,
],
mapPath: 'test/assets/$tmxFile',
stringNames: [tmxFile],
);
return component = await TiledComponent.load(
tmxFile,
Expand Down Expand Up @@ -631,10 +631,10 @@ void main() {
imageNames: [
'isometric_spritesheet.png',
],
mapPath: 'test/assets/test_shifted.tmx',
stringNames: ['test_shifted.tmx'],
);
component = await TiledComponent.load(
'test_isometric.tmx',
'test_shifted.tmx',
destTileSize,
);
}
Expand Down Expand Up @@ -681,7 +681,7 @@ void main() {
imageNames: [
'isometric_spritesheet.png',
],
mapPath: 'test/assets/test_isometric.tmx',
stringNames: ['test_isometric.tmx'],
);
component = await TiledComponent.load('test_isometric.tmx', size);
});
Expand Down Expand Up @@ -747,7 +747,7 @@ void main() {
imageNames: [
'0x72_DungeonTilesetII_v1.4.png',
],
mapPath: 'test/assets/dungeon_animation_$mapType.tmx',
stringNames: ['dungeon_animation_$mapType.tmx'],
);
component = await TiledComponent.load(
'dungeon_animation_$mapType.tmx',
Expand Down

0 comments on commit 7bc2fdc

Please sign in to comment.