melonjs / melonJS

a fresh, modern & lightweight HTML5 game engine
https://melonjs.org
MIT License
5.93k stars 643 forks source link

Exception if Renderable.tint is not coming from pool.pull("Color"....) #1123

Closed wpernath closed 2 years ago

wpernath commented 2 years ago

Describe the bug I just came along the "tint" feature of renderables. I now use it heavily on my players and on the BitmapText objects. I just realized that if I am using

BitmapText txt = new BitmapText(10, 15, {
  fillStyle: pool.pull("Color", 255,55,55),
   text: "Hey there"
});

this.addChild(txt);

Everything is fine. But if I am using something like

fillStyle: "#ff5555"

I get an exception like as soon as the Stage is destroyed (mostly on onDestroyEvent()).

melonjs.module.js:1958 Uncaught Error: me.pool: object #00ffa0 cannot be recycled
    at ObjectPool.push (melonjs.module.js:1958:23)
    at BitmapText.destroy (melonjs.module.js:18763:18)
    at BitmapText.destroy (melonjs.module.js:34989:15)
    at PlayerEntry.removeChildNow (melonjs.module.js:20969:31)
    at PlayerEntry.reset (melonjs.module.js:20464:22)
    at PlayerEntry.destroy (melonjs.module.js:21172:14)
    at MenuComponent.removeChildNow (melonjs.module.js:20969:31)
    at MenuComponent.reset (melonjs.module.js:20464:22)
    at MenuComponent.destroy (melonjs.module.js:21172:14)
    at World.removeChildNow (melonjs.module.js:20969:31)
push @ melonjs.module.js:1958
destroy @ melonjs.module.js:18763
destroy @ melonjs.module.js:34989
removeChildNow @ melonjs.module.js:20969
reset @ melonjs.module.js:20464
destroy @ melonjs.module.js:21172
removeChildNow @ melonjs.module.js:20969
reset @ melonjs.module.js:20464
destroy @ melonjs.module.js:21172
removeChildNow @ melonjs.module.js:20969
reset @ melonjs.module.js:20464
reset @ melonjs.module.js:21776
emit @ melonjs.module.js:4490
emit @ melonjs.module.js:5110
reset @ melonjs.module.js:21972
reset @ melonjs.module.js:22923
_switchState @ melonjs.module.js:23249
setTimeout (async)
defer @ melonjs.module.js:1781
eval @ melonjs.module.js:23691
update @ melonjs.module.js:33888
update @ melonjs.module.js:21220
update @ melonjs.module.js:21867
update @ melonjs.module.js:22939
update @ melonjs.module.js:22052
_renderFrame @ melonjs.module.js:23211
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216

melonjs.module.js:1992 Uncaught TypeError: Cannot read properties of undefined (reading 'className')
    at ObjectPool.poolable (melonjs.module.js:1992:29)
    at ObjectPool.push (melonjs.module.js:1956:19)
    at BitmapText.destroy (melonjs.module.js:34983:14)
    at PlayerEntry.removeChildNow (melonjs.module.js:20969:31)
    at PlayerEntry.reset (melonjs.module.js:20464:22)
    at PlayerEntry.destroy (melonjs.module.js:21172:14)
    at MenuComponent.removeChildNow (melonjs.module.js:20969:31)
    at MenuComponent.reset (melonjs.module.js:20464:22)
    at MenuComponent.destroy (melonjs.module.js:21172:14)
    at World.removeChildNow (melonjs.module.js:20969:31)

Live Example

Desktop (please complete the following information):

wpernath commented 2 years ago

BTW, the same is happening if my Sprite is using a tint.

obiot commented 2 years ago

I was just about to publish the 13.2 version when I saw this ticket :)

I fixed it, if you can give it a try ? you can also now use pass fillStyle: "#ff5555" which is a better way than pulling a color instance from the object pool (this is what the constructor is doing already, so you just end up wasting one instance)

for Sprite though, I'm not able to reproduce the issue (I use that heavily on my side too and never had an issue), what is your use case exactly ?

wpernath commented 2 years ago

Cool, great! Thank you. With BitmapText it's working now. With Sprites not.

My usecase with Sprites is, that I have a simple player sprite (which is mostly white). I use tinting here to have different colors for different players.

let player1 = new PlayerSprite(xxxx);
let player2 = new PlayerSprite(xxxx);
player2.tint = new Color(255,20,20);

or

player2.tint.parseCSS("#ff1414");

They all give me the following exception:

Uncaught Error: me.pool: object [object Object] cannot be recycled
    at ObjectPool.push (melonjs.module.js:1958:23)
    at PlayerEntity.destroy (melonjs.module.js:18763:18)
    at PlayerEntity.destroy (melonjs.module.js:29596:15)
    at GetReadyBack.removeChildNow (melonjs.module.js:20969:31)
    at GetReadyBack.reset (melonjs.module.js:20464:22)
    at GetReadyBack.destroy (melonjs.module.js:21172:14)
    at World.removeChildNow (melonjs.module.js:20969:31)
    at World.reset (melonjs.module.js:20464:22)
    at World.reset (melonjs.module.js:21776:15)
    at EventEmitter.emit (melonjs.module.js:4490:34)
push @ melonjs.module.js:1958
destroy @ melonjs.module.js:18763
destroy @ melonjs.module.js:29596
removeChildNow @ melonjs.module.js:20969
reset @ melonjs.module.js:20464
destroy @ melonjs.module.js:21172
removeChildNow @ melonjs.module.js:20969
reset @ melonjs.module.js:20464
reset @ melonjs.module.js:21776
emit @ melonjs.module.js:4490
emit @ melonjs.module.js:5110
reset @ melonjs.module.js:21972
reset @ melonjs.module.js:22923
_switchState @ melonjs.module.js:23249
setTimeout (async)
defer @ melonjs.module.js:1781
eval @ melonjs.module.js:23691
update @ melonjs.module.js:33888
update @ melonjs.module.js:21220
update @ melonjs.module.js:21867
update @ melonjs.module.js:22939
update @ melonjs.module.js:22052
_renderFrame @ melonjs.module.js:23211
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
melonjs.module.js:1992 Uncaught TypeError: Cannot read properties of undefined (reading 'className')
    at ObjectPool.poolable (melonjs.module.js:1992:29)
    at ObjectPool.push (melonjs.module.js:1956:19)
    at PlayerEntity.destroy (melonjs.module.js:29594:14)
    at GetReadyBack.removeChildNow (melonjs.module.js:20969:31)
    at GetReadyBack.reset (melonjs.module.js:20464:22)
    at GetReadyBack.destroy (melonjs.module.js:21172:14)
    at World.removeChildNow (melonjs.module.js:20969:31)
    at World.deferredRemove (melonjs.module.js:20306:10)
poolable @ melonjs.module.js:1992
push @ melonjs.module.js:1956
destroy @ melonjs.module.js:29594
removeChildNow @ melonjs.module.js:20969
reset @ melonjs.module.js:20464
destroy @ melonjs.module.js:21172
removeChildNow @ melonjs.module.js:20969
deferredRemove @ melonjs.module.js:20306
setTimeout (async)
defer @ melonjs.module.js:1781
removeChild @ melonjs.module.js:20936
onDestroyEvent @ get-ready.js:198
destroy @ melonjs.module.js:23008
_switchState @ melonjs.module.js:23241
setTimeout (async)
defer @ melonjs.module.js:1781
eval @ melonjs.module.js:23691
update @ melonjs.module.js:33888
update @ melonjs.module.js:21220
update @ melonjs.module.js:21867
update @ melonjs.module.js:22939
update @ melonjs.module.js:22052
_renderFrame @ melonjs.module.js:23211
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216
requestAnimationFrame (async)
_renderFrame @ melonjs.module.js:23216

Right now I am overwriting the destroy() method of my PlayerSprite to explicitly set

    destroy() {
        this.tint = undefined;
        super.destroy();
    }
obiot commented 2 years ago

I don't understand why it's not working to be honest, I found another bug through that I just fixed while testing it (I'm not sure it would fix your issue, but you can try), but in the meanwhile I have to go, so cannot investigate further now.

let me know !

wpernath commented 2 years ago

I think, I managed to understand, what is wrong:

If I do

sprite.tint = new Color(255,255,0);

There is the exception as mentioned above. If I just do what you've done in your fix, it works:

sprite.tint.copy(new Color(255,255,0));

Hmmm... not sure if this is just my fault, because I haven't carefully read any documentation about setting tints or if it is still an issue... 🙈

However: Thank you again for your help!

obiot commented 2 years ago

tint is actually already a Color instance, so you don't need to actually instantiate a new one.

you can just do sprite.tint.setColor(255, 255, 0);

is the setColor() method not clear, shall I add a setRGB one ?

wpernath commented 2 years ago

Well, I don't think, this would be necessary. But having a setter for tint would be nice. One which is taking all possible arguments (css, hex, etc.) and is associating the actual tint accordingly.

Right now, tint is just a property and one (like myself) is typically setting it like I did:

Sprite.tint = new Color(r,g,b).

This will result in that exception.

But if you'd create a setter for this, it would hide it from the user.

What du you think?

Thanks, Wanja

obiot commented 2 years ago

I guess so, all right, thanks for the idea ! will revert back shortly with some update to be tested :)

obiot commented 2 years ago

done ! if you have the time to give it a quick test (I also update all the dist files).

However you should really not do Sprite.tint = new Color(r,g,b); because you really create a Color object for nothing. with the new setter, however you can do Sprite.tint = "#rgb";

wpernath commented 2 years ago

Great! Thanks a lot. Let me check it!

wpernath commented 2 years ago

Works like a charm! Thanks a lot again for your help!

And I am using a string now for colors. :-)

obiot commented 2 years ago

Fantastic ! I will publish the 13.2 version tomorrow then :)

thanks as well for all the feedback and help !