UnderwaterApps / overlap2d-runtime-libgdx

Overlap2D - UI and Level Editor libgdx runtime
http://overlap2d.com
Other
170 stars 96 forks source link

Fix origin and rotation of physics objects #87

Closed leruaa closed 8 years ago

leruaa commented 8 years ago

When using images with physics component and try to apply forces on theirs bodies, I found that center of mass and rotation are not correct, as you can see below :

bug

I set the center of mass in the editor to be at the center of the balloon, but the force is applied at the left bottom corner.

With the fix it looks better :

fix

azakhary commented 8 years ago

this seems right, going to test/merge tomorrow.

olipoh commented 8 years ago

Hi,

no it doesn't work. Here are 2 videos:

PR69 which fixes the rotation problem of images without physics but doesn't fix the center of mass problem.

PR87 which is your fix. As you can see, it just cancels the PR69 fix. PR69-87.zip

I just use simple images, no composites.

EDIT : I think I got lost in all the source versions I have. I had a lot to do these last weeks and I am not sure I'm using the right sources. If you have a way to get your sources it would be fine. Otherwise I have to find by myself and I won't have time before a few weeks... unfortunately.

EDIT 2 : finally I started from the last runtime sources and I have the same result. Strange as I can see that it works for you...

leruaa commented 8 years ago

@olipoh Yes, you are right, rotations set with the editor wasn't applied on physics object. I've just pushed a fix.

leruaa commented 8 years ago

I realize this PR is all wrong. I confused box2d origin and box2d center of mass.

In the editor, rotations seems to be centered on the object origin (size divided by 2) while in a overlap game (before my PR), origin of physics object were at the bottom left corner (this is easily noticable in @olipoh first video).

All I have to do is make origin of physics object at runtime match the object size divided by 2. Center of mass is a different feature that has nothing to do with all this !

I will fix this PR with another push soon.

leruaa commented 8 years ago

Ok, this should work fine now.

olipoh commented 8 years ago

Yes !!!

I made tests with different scenes, and the problems are solved. I'm impressed :-).

But actually, there is a slight difference. The objects are slower with your fix, as if the gravity were not the same. Maybe the problem was before your fix, I don't know.

If you want videos before and after the fix, just tell me.

PS : are you french ?

leruaa commented 8 years ago

Yes the objects are slower now, there was a bug I fixed in the commit a35751648449a78d768814de3c3619a7405dc629.

PS : Yes, I'm french !

olipoh commented 8 years ago

Alright. It works, it's wonderful. Thank you.

PS : would it be possible to speak privately ? olivier.hopp@gmail.com (french too).

azakhary commented 8 years ago

big thanks for taking on this guys! is it test-worthy at this point? also can you clamp the commits?

olipoh commented 8 years ago

As for me, the tests are conclusive, but I didn't test everything (and I won't have time next days).

As I'm a bit new on git, I don't understand "clamp the commits"...

leruaa commented 8 years ago

@azakhary Yes, I think it is ok for testing.

I don't understand "clamp the commits" either, do you want I merge all the commits in one ?

azakhary commented 8 years ago

Sorry I mean "squash" not clamp. As PR is done by @acatinon same use has to squash all 5 commits into one commit so we can merge. There is bunch of stack-overflow answers on how to squash many commits in one. This is just to keep repo "clean"

leruaa commented 8 years ago

Squash done !

azakhary commented 8 years ago

Thanks! we'll get back to this at the morning, it's a bit sleepy time here :))

olipoh commented 8 years ago

I have one question. When you merge a PR, what should I do in gradle ? Give another overlap runtime version name ? If yes, which one ? (ok it's more than one question).

azakhary commented 8 years ago

nope, it's going to be part of 0.1.2-SNAPSHOT when I merge, I will then build a new jar to maven central, and a gradle refresh will do it. (sometimes you need to clear caches)

olipoh commented 8 years ago

Don't you think it may be a problem for bug that had a work around ? Just an example : if you merge PR 87, the gravity will be managed differently (as it seems there was a problem before). So people won't have the choice to keep their runtime, they will have to change the gravity value to have the same results / behaviors. Of course this is just an example with easy mandatory tuning. But it could be tricky for some other fixes... I'm just asking, not trying to teach you anything :-) I'm learning...

datscharf commented 8 years ago

Hi there,

I am very excited about these fixes good job to you all! Hope you don't think I am too impatient :-) but when will these changes become available in a new 0.1.2-SNAPSHOT?

Best regards Martin

azakhary commented 8 years ago

I am horrible person I know :)))

Okay, done, tested, merged, and snapshot baking initiated. It'll be there in 10 minutes or so.

leruaa commented 8 years ago

Thanks!

datscharf commented 8 years ago

Damn that was quick! Thanks a lot!! :D