Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlxG.cameras: fix reset() not removing all cameras #2016

Merged
merged 2 commits into from
Dec 26, 2016

Conversation

Leonix
Copy link

@Leonix Leonix commented Dec 26, 2016

The problem is that FlxG.cameras.reset() tries to remove elements of an Array while iterating over it.
This fails to .destroy() half of all cameras, leading to the memory leak.


  • Flixel version: 4.2.0
  • OpenFL version: 3.6.1
  • Lime version: 2.9.1
  • Affected targets: at least Neko, likely more
  • Observed behavior: Process memory usage steadily goes up.
  • Expected behavior: It should not.

Code snippet reproducing the issue:

package;
import flixel.FlxG;
import flixel.FlxState;
import flixel.FlxCamera;

class PlayState extends FlxState
{
	override public function create():Void
	{
		FlxG.cameras.add(new FlxCamera());
	}

	override public function update(elapsed: Float):Void
	{
		super.update(elapsed);
		FlxG.switchState(new PlayState());
	}
}

Leonid Vakulenko and others added 2 commits December 26, 2016 16:45
when game has more than one camera added
@Gama11 Gama11 changed the title Memory leak on state change when more than one camera FlxG.cameras: fix reset() not removing all cameras Dec 26, 2016
@Gama11
Copy link
Member

Gama11 commented Dec 26, 2016

Looks like this is a regression from edf93b5.

Thanks!

@Gama11 Gama11 merged commit 1c714ba into HaxeFlixel:dev Dec 26, 2016
@Leonix
Copy link
Author

Leonix commented Dec 26, 2016

Note that unit test for this should probably test for whether FlxG.cameras.cameraRemoved signal is called for all cameras. Or whether all cameras were properly camera.remove()d. Checking only for list.length would pass on vanilla 4.2.0, too.

@Gama11
Copy link
Member

Gama11 commented Dec 26, 2016

You're right, I made a mistake when checking whether the test actually fails before your changes...

@Leonix Leonix deleted the patch-1 branch December 27, 2016 09:19
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants