Gamua / Starling-Framework

The Cross Platform Game Engine
http://www.starling-framework.org
Other
2.82k stars 821 forks source link

Image.bindScale9GridToTexture overrides automateSetupForTexture #1004

Closed brighttank closed 6 years ago

brighttank commented 6 years ago

For a particular texture I wanted to both set the scale9grid and set the textureSmoothing to NONE however the following did not work:

Image.bindScale9GridToTexture(myTexture, new flash.geom.Rectangle(10, 10, 20, 20));
Image.automateSetupForTexture(myTexture, function(image:Image) : void {
    image.textureSmoothing = TextureSmoothing.NONE;
});

Looking closer at the implementation in the Image class I realized that bindScale9GridToTexture was calling automateSetupForTexture with its own functions which were then overriden by my subsequent call to automateSetupForTexture.

Of course the fix is easy enough:

Image.automateSetupForTexture(myTexture, function(image:Image) : void {
    image.textureSmoothing = TextureSmoothing.NONE;
    image.scale9grid = new flash.geom.Rectangle(10, 10, 20, 20);
}, function(image:Image):void {
    image.scale9grid = null;
});

However, I suggest that the documentation be updated to reflect the fact that these methods (and bindPivotPointToTexture) are mutually exclusive.

Alternately the implementation could be updated to support multiple assignments to automateSetupForTexture.

PrimaryFeather commented 6 years ago

You're right, that definitely needed clarification! Thanks a lot for the heads-up!

I just updated the inline API reference to make this clearer. I also considered supporting multiple assignments — but decided against it, at least for now. This would also mean adding more granular methods to manage the setup (e.g. allowing to remove a specific pair of methods, etc.) — while most of the time, just knowing that you need to combine everything in one method pair will prevent any issues.

I can think of scenarios where it would be useful, though, so I might come back to this in the future. :wink:

PrimaryFeather commented 6 years ago

Haha, actually "the future" arrived rather quickly — because I already changed my mind and implemented support for multiple setup/release methods per texture.

In other words, your original code should now work just as fine, too. :smile: Please try it out and let me know if it works for you, @brighttank!

Cheers!

brighttank commented 6 years ago

Wow, that's some fast turn-around Daniel! Works as expected.

The advantage of having this ability is that my AssetManager can now have a list of textures to apply scale9grid's to and another list that need TextureSmoothing disabled. Much cleaner!

Thanks for all your great work.

PrimaryFeather commented 6 years ago

You're very welcome! Thanks for trying it out!

Yes, that was also the main reason why I thought it made sense: you can now separate responsibilities better. E.g. one could subclass "TextureAtlas" so that it adds pivot information, and at some other place in the game's code change texture smoothing. That was difficult to do with the previous solution.

So thanks again for making me aware of this! :smile: