UpdateList bug since 3.17.0 - the object is never removed if it's in active state.

[jcyuan]

remove: function (child)
{
    // I think here should be _list.indexOf(child)? or perhaps check both just like the add method does?
    // like this: if (this._list.indexOf(child) !== -1 && this._pendingRemoval.indexOf(child) === -1)
    // {
    //     this._pendingRemoval.push(child);
    // }

    var index = this._pendingRemoval.indexOf(child); 

    // or if use _pendingRemoval to check duplications, here should be === -1?
    if (index !== -1) 
    {
        this._pendingRemoval.push(child);
    }

    return child;
}

please read the comments, so is this a mistake or I misunderstood something?
because I'm not quite clear of the purpose, so can't make a PR, sorry.

[CipSoft-Components]

I would say this is a bug. I have found the same, because of performance problems, after my game is running for a while. And in my game there are many objects created and removed again.
Can somebody please fix this? This is also in 3.18.1.

[CipSoft-Components]

@jcyuan Can you change the title to something more explaining, like "UpdateList not working", so it is better visible. And changing the description, to have everything from the create issue template, so it is processed, because if the template is not used, the issue will be ignored. Because I don't want to open a duplicate issue.

[samme]

@CipSoft-Components — Can you explain the bug, i.e. what's the defective behavior and how it can be produced?

[CipSoft-Components]

@samme Explanation:
Defective behavior: The UpdateList will not remove objects, when its remove method is called. When I understand the code correctly, the remove method only adds the element to a "ready to remove" list, which will be removed soon (next update step). But the method first looks up, if the element is already in this "ready to remove" list, to not add it multiple times. The problem now is, that the condition to check this, is wrong, and always evaluates to false, so that the element never is added to the "ready to remove" list.

How to reproduce:
Create some GameObjects, destroy them with <GameObject>.destroy() (which should remove them from the UpdateList), and then look into the elements in the UpdateList after some updates occured, The objects are remaining in the list, and also its "ready to remove" array (_pendingRemoval) is empty.

Main problem from that behavior: When you have a long running scene, with many created and destroyed objects, the performance goes down extremely after some time.

[jcyuan]

@CipSoft-Components sure, will do.

@photonstorm this should be a coding mistake for fixing #4365 which was reported by me too.

this is the existing code since 3.17.0:
https://github.com/photonstorm/phaser/blob/34bf26592e922ca2ef53d76753dfa5eccf97ac13/src/gameobjects/UpdateList.js#L217-L227

as you can see, if my object is in active state, it can't be removed at all!

really easy to fix (I patched for my own project locally)
way1: just like the way the add method does:

if (this._list.indexOf(child) !== -1 && this._pendingRemoval.indexOf(child) === -1) {
        this._pendingRemoval.push(child);
}

way2: change the condition
modify https://github.com/photonstorm/phaser/blob/34bf26592e922ca2ef53d76753dfa5eccf97ac13/src/gameobjects/UpdateList.js#L221
to

 if (index === -1) 

but personally way 1 is better and recommended.

[jcyuan]

I'm curious why people not report this problem as this topic is already here for a little while...
guys don't remove things in their game? 🤣

[CipSoft-Components]

I think it depends on the games. Perhaps, many of the games don't have long running scenes, with very much unique objects, which they destroy manually. If a scene is shutdown, everything will be cleared up. And if you have many nearly identically objects you use groups, and don't destroy so many objects manually.

[jcyuan]

yep, it does make sense, my game is a bit heavy too, so found this problem before.

[photonstorm]

Thank you for submitting this issue. We have fixed this and the fix has been pushed to the master branch. It will be part of the next release. If you get time to build and test it for yourself we would appreciate that.