Bug: Assets.isLocal() should call Assets.isValidBitmapData()

#1

So I found a potential bug with Assets.isLocal(). I figured I should ask about it here first in case it’s by design before attempting a pull request on Github.

Currently, Assets.isLocal( … ) does not check if a BitmapData asset that was previously loaded asynchronously was disposed. I’ve run into the following scenario in the past:

  1. Use Assets.isLocal( … ) to see if an image is already loaded. It returns false.
  2. Use Assets.loadBitmapData( … ) to fetch the image asynchronously.
  3. The BitmapData is loaded and cached.
  4. Use Assets.isLocal( … ) to see if an image is already loaded. It returns true.
  5. Use Assets.getBitmapData( … ) to fetch the BitmapData immediately.
  6. The BitmapData is disposed at some point (!!!)
  7. Use Assets.isLocal( … ) to see if an image is already loaded. It returns true, despite it being disposed of (!!!)
  8. Use Assets.getBitmapData( … ) to fetch the BitmapData immediately.
  9. Error!

I’ve included a test case written below:

package tests;
import openfl.utils.AssetType;
import openfl.utils.Assets;
import openfl.display.BitmapData;

/**
 * ...
 * @author 
 */
class BitmapLoaderTest 
{
	private var _url:String;
	
	// -----------------------------------------------------------------------------------------------
	// -----------------------------------------------------------------------------------------------
	public function new(url:String) 
	{
		_url = url; // should be an asset not already loaded
		start();
	}
	
	// -----------------------------------------------------------------------------------------------
	private function start():Void
	{
		load( firstCallback );
	}
	
	// -----------------------------------------------------------------------------------------------
	private function firstCallback(bitmap:BitmapData):Void
	{
		trace("firstCallback -> Finished");
		load( secondCallback );
	}
	
	// -----------------------------------------------------------------------------------------------
	private function secondCallback(bitmap:BitmapData):Void
	{
		trace("secondCallback -> Finished");
		trace("secondCallback -> disposing of BitmapData");
		bitmap.dispose();
		load( secondCallback );
	}
	
	// -----------------------------------------------------------------------------------------------
	private function thirdCallback(bitmap:BitmapData):Void
	{
		trace("thirdCallback -> Finished");
		trace("Test completed");
	}
	
	// -----------------------------------------------------------------------------------------------
	private function load( callback:BitmapData->Void ):Void
	{
		if ( Assets.isLocal(_url, AssetType.IMAGE, true) ) {
			trace("Loading image synchronously");
			var bmpd:BitmapData = Assets.getBitmapData( _url );
			callback( bmpd );
		}
		else {
			trace("Loading image asynchronously");
			Assets.loadBitmapData( _url ).onComplete( callback );
		}
	}
}

It produces the following output:

BitmapLoaderTest.hx:60: Loading image asynchronously
BitmapLoaderTest.hx:31: firstCallback -> Finished
BitmapLoaderTest.hx:55: Loading image synchronously
BitmapLoaderTest.hx:38: secondCallback -> Finished
BitmapLoaderTest.hx:39: secondCallback -> disposing of BitmapData
BitmapLoaderTest.hx:55: Loading image synchronously
[Fault] exception, information=[lime.utils.Assets] ERROR: IMAGE asset “” exists, but only asynchronously

I know you can technically bypass this by passing “false” for the useCache flag, but I personally feel when using Assets.isLocal(…) for BitmapData, it should also call Assets.isValidBitmapData( … ) to check if it has been disposed.

EDIT
It occurred to me that if Assets.isValidBitmapData( … ) returns false while calling Assets.isLocal(…), it should probably also be removed from the cache at that point.
END EDIT

Any thoughts or opinions on the matter?