deltaluca / nape

Haxe/AS3 Physics Engine
http://napephys.com
Other
542 stars 77 forks source link

getting the body mass value before setting it causes unexpected behavior #56

Closed melnikov-s closed 10 years ago

melnikov-s commented 11 years ago

Seems like the reading of the body.mass value causes side effects in certain situations. In my testing seems to happen only before you set a custom mass value (when the value is pre-computed). From some quick debugging the issue arises because zpp_inner.zip_mass gets set to false when reading the value.

Full code: taken from the simple simulation sample, scroll down to the setUp method and uncomment "scenario 2" to see the bug in action.

package;

import flash.display.Sprite;
import flash.events.Event;
import flash.events.KeyboardEvent;

import nape.geom.Vec2;
import nape.phys.Body;
import nape.phys.BodyType;
import nape.shape.Circle;
import nape.shape.Polygon;
import nape.space.Space;
import nape.util.BitmapDebug;
import nape.util.Debug;

class Main extends Sprite {

    var space:Space;
    var debug:Debug;

    function new() {
        super();

        if (stage != null) {
            initialise(null);
        }
        else {
            addEventListener(Event.ADDED_TO_STAGE, initialise);
        }
    }

    function initialise(ev:Event):Void {
        if (ev != null) {
            removeEventListener(Event.ADDED_TO_STAGE, initialise);
        }

        var gravity = Vec2.weak(0, 600);
        space = new Space(gravity);
        debug = new BitmapDebug(stage.stageWidth, stage.stageHeight, stage.color);
        addChild(debug.display);

        setUp();

        stage.addEventListener(KeyboardEvent.KEY_DOWN, keyDownHandler);
        stage.addEventListener(Event.ENTER_FRAME, enterFrameHandler);
    }

    function setUp() {
        var w = stage.stageWidth;
        var h = stage.stageHeight;

        var floor = new Body(BodyType.STATIC);
        floor.shapes.add(new Polygon(Polygon.rect(50, (h - 50), (w - 100), 1)));
        floor.space = space;
        for (i in 0...16) {
            var box = new Body(BodyType.DYNAMIC);
            box.shapes.add(new Polygon(Polygon.box(16, 32)));
            box.position.setxy((w / 2), ((h - 50) - 32 * (i + 0.5)));

            //Scenerios uncomment to reproduce

            /* Scenerio 1: mass gets applied as expected
            box.mass = 2000;
            */

            /* Scenerio 2: either mass does not get applied or box exhibits strange behaviours
            trace(box.mass);
            box.mass = 2000;
            */

            /* Scenerio 3 : mass gets applied as expected
            trace(box.mass);
            box.zpp_inner.zip_mass = true;
            box.mass = 2000;
            */

            box.space = space;
        }

        var ball = new Body(BodyType.DYNAMIC);
        ball.shapes.add(new Circle(50));
        ball.position.setxy(50, h / 2);
        ball.angularVel = 10;
        ball.space = space;
    }

    function enterFrameHandler(ev:Event):Void {
        space.step(1 / stage.frameRate);
        debug.clear();
        debug.draw(space);
        debug.flush();
    }

    function keyDownHandler(ev:KeyboardEvent):Void {
        if (ev.keyCode == 82) { // 'R'
            space.clear();

            setUp();
        }
    }

    static function main() {
        flash.Lib.current.addChild(new Main());
    }
}
deltaluca commented 11 years ago

Having trouble figuring this out as the haxe nightlies are having issues with breaking napes code completely in some cases right now, so I can't tell myself if its a probelm in my code, or haxe's :P

melnikov-s commented 11 years ago

Sorry, should have mentioned that I'm using haxe 2.10 release. You should be able to reproduce with that.

With both nape 2.0.3 and 2.0.4

deltaluca commented 11 years ago

... sadly it seems this issue is effecting even 2.10 release, you can try this yourself and see if you get the same behaviour (haxe 2.10 + nape 2.0.4)

Main.hx:

class Main { static function main() { var b = new nape.phys.Body(); b.shapes.add(new nape.shape.Circle(10)); trace(b.mass *= 10); } }

compiling

haxe -main Main -swf main.swf -lib nape -D NAPE_RELEASE_BUILD

with --no-inline it traces (correctly) pi without --no-inline it traces 0

melnikov-s commented 11 years ago

Yup, traces 0 or pi (with --no-inline) for haxe 2.10 and nape 2.0.4

deltaluca commented 11 years ago

It'd be great if you could come up with a minimal example (not requiring nape) that shows this behaviour, I've so far failed, and we can pass it to haxe dev team:

https://code.google.com/p/haxe/issues/detail?id=1652&sort=-id&colspec=ID%20Status%20Milestone%20Priority%20Platform%20Owner%20Summary

melnikov-s commented 11 years ago

I'll see what I can do, but that might be a different issue entirely! I've tried compiling the code in my first post with --no-inline and the bug is still present. Removing the trace fixes the issue.

Now the really weird part is that when I do trace(box.mass *= 2000) (which should be 2048 mass) and compile without --no-inline I get a different outcome, with --no-inline i can reproduce the original bug.

So to clarify, there are 3 cases here:

//Case 1: bug is present with both --no-inline and without.
trace(box.mass);
box.mass = 2048; 

//case 2: mass is reported to be 0 and shapes do not appear
trace(box.mass*= 2000);

//Case 3: with --no-inline same as Case 1
trace(box.mass*= 2000);
deltaluca commented 11 years ago

Okay, I think I've found the bug easily enough now and have made a fix on haxelib for it. Since you're using Haxe I'll not bother with an as3 release for now since no-one else has come across this just now.

On an unrelated note, you'll find that directly setting 'just' the mass like this will make things behave really weirdly anyways since the moment of inertia will still be very small; if you want to set a mass/inertia directly then you're free to do so, but if you simply want to make the object heavier, you should either increase the density of the material, or ensure that you scale up the mass/inertia by the same factor.

I think that the inlining bug in haxe is only in such situations where an implicit call to the getter is supposed to be made, such as in the mass *= _; situation, so hopefully you don't have to use --no-inline !

melnikov-s commented 11 years ago

Thanks for the tip and the quick fix! Cheers.

deltaluca commented 11 years ago

I figured what haxe is doing wrong and made a test case on the issue page:

https://code.google.com/p/haxe/issues/detail?can=2&start=0&num=100&q=&colspec=ID%20Status%20Milestone%20Priority%20Platform%20Owner%20Summary&groupby=&sort=-id&id=1652

deltaluca commented 11 years ago

Seems this was not a haxe bug after all, haxe does not guarantee order of execution for inlined function parameters.

In this case the problem is that doing body.mass += 10; the 'getter' for mass is invoked after the 'setter' has already began execution and its messing up the invalidation system as the 'setter' is doing something similar to locking a mutex on the body if you will which causes the getter to fail.

There is a workaround that I can make to permit this sort of usage, which is to forcebly assign inline function params to local variables at start of function... which is kind of horrible so I won't implement that 'right' now. I'll likely be using a macro to do it automaticly when the time comes but I must be careful to keep it haxe 2.0 compatible also.

deltaluca commented 10 years ago

Ehhh, unless someone really needs this done, I'll consider it 'not to be done' task for now.