flixel-addons
flixel-addons copied to clipboard
What happens to the old body? Overwritten, or memory leak?
https://github.com/HaxeFlixel/flixel-addons/blob/dev/flixel/addons/nape/FlxNapeSprite.hx#L253
Not sure about this, but maybe
FlxNapeSpace.bodies.remove(this.body);
should be added before the
this.body = body;
?
if anything this adds extra steps. If you look here: https://github.com/HaxeFlixel/flixel-addons/blob/dev/flixel/addons/nape/FlxNapeSprite.hx#L289 FlxNapeSprite removes the body from space if it is part of space to begin with.
Why do we have a setBody rather than just having a body property with a setter?
Good question. I would prefer that. Shall I implement it?
@thegoodideaco That doesnt remove the old body. It determines whether the current body is in the space or not. Note that it gets called after setting the body in setBody. Looks like unless you remove the old body yourself, you'll have one left hanging around in space?
@JoeCreates yes you are correct. it looks like it adds or removes it from the space depending on if physicsEnabled is true, which makes sense
Im not sure if it should automatically remove the body from space or not...
Well it does (for the new body) depending on if physicsEnabled is set to true. however if the old body is already set, and a part of space, then we immediately lose it as soon as we set this.body to another body.
I do think that this class should be looked at again. I don't like the fact that setPosition focuses on the top-left and the body is center anchored. It seems to me that the body should control all movement of the Sprite, while the sprite controls the graphics. Maybe adding getters/setters to the Sprite itself? But then again that could be a bad idea..
@thegoodideaco The point in the original post is that the original body isn't lost, because the space still has a reference to it, hence the possibility of a memory leak. Do you think it should be automatically removed or left in space?
@JoeCreates I meant the reference is lost from the FlxNapeSprite. wether we remove it from space or not, it will still be floating around not being used (unless there is some reason to use it).
Another thing to consider is that a body's space can be changed *only * if that body doesn't belong to a Compound. And you also have to consider any Constraints that it might be apart of, because a Constraint's bodies must both have the same space. setting space to null on a body with a constraint will throw an error
I don't see a reason for FlxNapeSprite to have more than one body accountable. I would probably want it destroyed since it's really only used by the helper functions for adding a box, circle or premade body. I've never swapped out a body after it's been initated but there might be some use cases I'm not aware of.
There are also other restrictions that we have as well, such as making the BodyType Dynamic right off the bat and immediately adding it to space. If I wanted it to be Kinematic, I would have to remove it from space, change the type, and add it back