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

Phaser "body.removeShape" triggers error of undefined #208

Closed
wants to merge 2 commits into from
Closed

Phaser "body.removeShape" triggers error of undefined #208

wants to merge 2 commits into from

Conversation

Weedshaker
Copy link

This is just a fast fix but fixed my issue of a shape which gets temporarely added to a body. I guess it triggers this issue (si.position of undefined) when removed but still in loop. Anyhow, there probably is a much better fix but I don't have time to dig deeper. Hope you can have a look at this issue!

Thanks

This is just a fast fix but fixed my issue of a shape which gets temporarely added to a body. I guess it triggers this issue (si.position of undefined) when removed but still in loop. Anyhow, there probably is a much better fix but I don't have time to dig deeper. Hope you can have a look at this issue!

Thanks
@schteppe
Copy link
Owner

Why would this happen in the first place? Can you provide some (phaser) code?

@Weedshaker
Copy link
Author

I do add a shape to the body as followed:

this.hitShape = this.entity.body.addCircle(this.hitSize[0], this.entity.scale.x * this.hitSize[1], this.hitSize[2]);

and then by occasion remove it:

this.entity.body.removeShape(this.hitShape);

the error is triggered when any shape of the body gets a collision while the above shape has been being removed.

Must have missed this obvious part last night.
@schteppe
Copy link
Owner

Okay.. But I still don't get why it would happen. The removeBody method in Phaser seems legit. There must be some other piece of code modifying the .shapes array directly, somewhere...

@Weedshaker
Copy link
Author

nope (not modifying any in the shapes array)! I would give you my code but its just spread through too many files and objects that it would give a good example, I may should reproduce the issue with a simple example. Though can't find the time yet. But all I do is to add that shape and remove the shape. I use a 100% the phaser api and don't hack around inside p2.js directly.

@schteppe
Copy link
Owner

If you could provide a simple example it would be super awesome

@Weedshaker
Copy link
Author

I looked deeper into the issue and tried to fix it inside my code (which worked with a tiny timeout on removing the shape). Essential to trigger the error is that the main body removes the shape (triggered by onBeginContact / Narrowphase) at the same frame as the object impact occurs. Below a screen shot regarding the issue:

screenshot from 2016-02-12 10 47 32
[fyi => the outer red circle is a sensor]

  1. Here you can see the main body (boy), swinging the axe (holding axe), which gets added as an extra shape (applied to the boy's body right seen as a greenish circle).

  2. This constellation has an impact body (flying axe) or as well as the explosion. Ether one of this outer body impacts trigger the error. You are having both in the screen shot above, but the error occurs on ether one of them combined or separate.

  3. On flying axe impact or explosion impact the boy removes the holding axe shape.

Hope this helps.

@schteppe
Copy link
Owner

Thanks. Will dig deeper into this when I find some time.

One idea: You remove the shape in the beginContact event? Try not doing that... Because the Narrowphase or World may be in the middle of processing the shapes. Instead, you can make a queue of shapes that should be removed in the next update() of your game loop.
See https://github.com/schteppe/p2.js/wiki#events-fired-during-step

@Weedshaker
Copy link
Author

World.prototype.removeBody = function(body){
    if(this.stepping){
        this.bodiesToBeRemoved.push(body);
    } else {
        body.world = null;
        var idx = this.bodies.indexOf(body);
        if(idx!==-1){
            Utils.splice(this.bodies,idx,1);
            this.removeBodyEvent.body = body;
            body.resetConstraintVelocity();
            this.emit(this.removeBodyEvent);
            this.removeBodyEvent.body = null;
        }
    }
};

Seems as Phaser does this correct for Bodies (bodiesToBeRemoved - array). But not for shapes... as far as I have seen on first glance. I am gonna dig a bit deeper to make sure, if there isn't something like a "shapesToBeRemoved" array. Anyway, this looks like I have to open a Pull request at Phaser. Sorry, for wasting your time!!!

Thanks for your kind support and hunting down my issue. I really should have red through the p2.js wiki and not only rely on Phasers api.

@schteppe
Copy link
Owner

Oh, yeah. Shapes to be removed... Hm. If we keep allowing things to be removed during a physics tick, then we need to add arrays for constraints, springs and everything in the World. And yea, we should add arrays for not only removal, but adding as well. Ideally, we would have a queue for all methods that shouldn't be used during the loop as well... I'm starting to see why Bullet don't allow removal at all, during the step.

@Weedshaker
Copy link
Author

I got it fixed properly on my side:

this.game.time.events.add(10, () => {
   this.entity.body.removeShape(this.hitShape);
   this.hitShape = null;
});

I just didn't feel well with hacking around inside p2.js. Please, drop the PR... also, I don't think that you should add those arrays you mentioned above, this would just take more cpu juice. I may look into phaser to add this change but most likely lazer. LOL!

P2.js - ROCKS!!!

Again, thank you! Danke schön!

@schteppe
Copy link
Owner

Thanks for using the lib and for your help :D

@schteppe schteppe closed this Feb 13, 2016
@Weedshaker
Copy link
Author

I was thinking that it may would be nice making a p2.js branch for beginners. Which handles the above mentioned constraint issue, the issue of this pull request, etc...

Then people can decide on their own, if they prefer more performance but errors thrown if they are not tidy (with the existing master branch) or an error proof "dummy" branch with a bit less performance, since making a few extra checks.

Just an idea... but I guess it could boost popularity with making the entry to p2.js easier.
Cheers

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.

None yet

2 participants