Fix double destroy call on layer children

[gm0nk]

Please do not update the README or Change Log, we will do this when we merge your PR.

This PR (delete as applicable)

• Fixes a bug

Describe the changes below: #6626 added layer.remove method as event listener on DESTROY event of layer's child. This causes an error to appear when game or scene is being destroyed:

GameObject.js:767 Uncaught TypeError: Cannot read properties of undefined (reading 'sys')
    at Image.removeFromDisplayList (GameObject.js:767:1)
    at Image.destroy (GameObject.js:853:1)
    at Layer.destroy (Layer.js:1056:1)
    at DisplayList.shutdown (DisplayList.js:241:1)
    at DisplayList.destroy (DisplayList.js:257:1)
    at EventEmitter.emit (index.js:202:1)
    at Systems.destroy (Systems.js:808:1)
    at SceneManager.destroy (SceneManager.js:1739:1)
    at CoreGame.runDestroy (Game.js:751:1)
    at CoreGame.step (Game.js:478:1)

The reason for this: gameObject.once(GameObjectEvents.DESTROY, this.remove, this) in layer.addChildCallback() method. So when in child.destroy() method (inherited from GameObject) we emit DESTROY event: this.emit(Events.DESTROY, this, fromScene) Child and fromScene parameters are passed to layer.remove(child, destroyChild). And fromScene is treated as destroyChild which causes the second destroy to start in the middle of the first one. Also we dont really need to add remove as a listener to DESTROY event, because destroy (inherited from GameObject) calls removeFromDisplayList() method, that does essentially the same thing.

[samme]

And fromScene is treated as destroyChild which causes the second destroy to start in the middle of the first one.

There's a similar problem with Container, I think.

[gm0nk]

There's a similar problem with Container, I think.

Yes, you're right. I guess it happens if layer contains anything that inherits GameObject.destroy() method.

[gm0nk]

Same issue #6675. @rexrainbow suggested to remove remove method from listeners when layer.destroy is called in PR #6676. As I mentioned, we dont really need to use remove as a listener when there's removeFromDisplayList in gameobject's destroy method. Also there's might be additional problem in gameobject.removeFromDisplayList: it calls displayList.remove(this, true). In case of layer's child displayList.remove is actually layer.remove. This method was added in #6626, it has signature remove(child, destsroyChild) and it overrides inherited from List method with interface remove(child, skipCallback). So when displayList.remove(this, true) is called with true meaning skipCallback, but it's actually destroyChild. Same thing was with fromScene, but this one doesnt lead to crash. At this point I think we should just reverse everything that was added in #6626.

[rexrainbow]

I try to align interface of these layer methods to container's, especially remove(child, destsroyChild) in that PR, so that user can switch between layer and container more easier.

Since you and Rich will take care about this issue, I don't have to add PR for it.

[gm0nk]

I try to align interface of these layer methods to container's, especially remove(child, destsroyChild) in that PR, so that user can switch between layer and container more easier.

Yeah, it makes sense. I guess there's intentional division between display list and container. May be @photonstorm can clarify more.

[photonstorm]

I agree, the changes made in 3.70 broke this and overly complicates the flow, so I have reverted them.