From 825fb0cc7e5b30911e17a2075e28f74c8d69b593 Mon Sep 17 00:00:00 2001
From: Lukas Klingsbo <lukas.klingsbo@gmail.com>
Date: Sat, 15 Jan 2022 15:40:40 +0100
Subject: [PATCH] fix: Properly dispose images when cache is cleared (#1312)

* fix: Properly dispose images when cache is cleared

* Add test for clear and clearCache

* Update docs for Images cache
---
 doc/images.md                              |  6 ++++
 packages/flame/lib/src/assets/images.dart  | 29 +++++++++++----
 packages/flame/test/images_cache_test.dart | 41 ++++++++++++++++++++++
 3 files changed, 69 insertions(+), 7 deletions(-)
 create mode 100644 packages/flame/test/images_cache_test.dart

diff --git a/doc/images.md b/doc/images.md
index 70b5a4587f9..aada87b36ab 100644
--- a/doc/images.md
+++ b/doc/images.md
@@ -29,6 +29,12 @@ They return a `Future` for the loaded Image.
 To synchronously retrieve a previously cached image, the `fromCache` method can be used. If an image
 with that key was not previously loaded, it will throw an exception.
 
+To add an already loaded image to the cache, the `add` method can be used and you can set the key
+that the image should have in the cache.
+
+For `clear` and `clearCache`, do note that `dispose` is called for each removed image from the
+cache, so make sure that you don't use the image afterwards.
+
 ### Standalone usage
 
 It can manually be used by instantiating it:
diff --git a/packages/flame/lib/src/assets/images.dart b/packages/flame/lib/src/assets/images.dart
index ef6b118ab87..f08ead36a52 100644
--- a/packages/flame/lib/src/assets/images.dart
+++ b/packages/flame/lib/src/assets/images.dart
@@ -15,22 +15,37 @@ class Images {
 
   Images({this.prefix = 'assets/images/'});
 
-  /// Remove the image with the specified [fileName] from the cache.
-  void clear(String fileName) {
-    _loadedFiles.remove(fileName);
+  /// Adds an [image] to the cache that can be fetched with with the specified
+  /// [name].
+  void add(String name, Image image) {
+    _loadedFiles[name] = _ImageAssetLoader(Future.value(image))
+      ..loadedImage = image;
+  }
+
+  /// Remove the image with the specified [name] from the cache.
+  /// This calls [Image.dispose], so make sure that you don't use the previously
+  /// cached image once it is cleared (removed) from the cache.
+  void clear(String name) {
+    _loadedFiles.remove(name)?.loadedImage?.dispose();
   }
 
   /// Clear all cached images.
+  /// This calls [Image.dispose] for all images in the cache, so make sure that
+  /// you don't use any of the previously cached images once [clearCache] has
+  /// been called.
   void clearCache() {
+    _loadedFiles.forEach((_, imageAssetLoader) {
+      imageAssetLoader.loadedImage?.dispose();
+    });
     _loadedFiles.clear();
   }
 
-  /// Gets the specified image with [fileName] from the cache.
-  Image fromCache(String fileName) {
-    final image = _loadedFiles[fileName];
+  /// Gets the specified image with [name] from the cache.
+  Image fromCache(String name) {
+    final image = _loadedFiles[name];
     assert(
       image?.loadedImage != null,
-      'Tried to access an inexistent entry on cache "$fileName", make sure to '
+      'Tried to access an inexistent entry on cache "$name", make sure to '
       'use the load method before accessing a file on the cache',
     );
     return image!.loadedImage!;
diff --git a/packages/flame/test/images_cache_test.dart b/packages/flame/test/images_cache_test.dart
new file mode 100644
index 00000000000..302cef56b77
--- /dev/null
+++ b/packages/flame/test/images_cache_test.dart
@@ -0,0 +1,41 @@
+import 'dart:ui';
+
+import 'package:flame/assets.dart';
+import 'package:mocktail/mocktail.dart';
+import 'package:test/test.dart';
+
+class MockImage extends Mock implements Image {
+  int disposedCount = 0;
+
+  @override
+  void dispose() {
+    disposedCount++;
+  }
+}
+
+void main() {
+  group('ImagesCache', () {
+    test('clear', () {
+      final cache = Images();
+      final image = MockImage();
+      cache.add('test', image);
+      expect(image.disposedCount, 0);
+      cache.clear('test');
+      expect(image.disposedCount, 1);
+    });
+
+    test('clearCache', () {
+      final cache = Images();
+      final images = List.generate(10, (_) => MockImage());
+      for (var i = 0; i < images.length; i++) {
+        cache.add(i.toString(), images[i]);
+      }
+      expect(images.fold<int>(0, (agg, image) => agg + image.disposedCount), 0);
+      cache.clearCache();
+      expect(
+        images.fold<int>(0, (agg, image) => agg + image.disposedCount),
+        images.length,
+      );
+    });
+  });
+}