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

Box2DWorld is a Node2D but has undefined behavior when its transform changes #8

Closed
briansemrau opened this issue Dec 6, 2020 · 26 comments
Assignees
Labels
bug Something isn't working

Comments

@briansemrau
Copy link
Owner

This can be resolved in a few ways:

Solution 1.
Make Box2DWorld inherit Node. This fixes undefined behavior but isn't fun.

Solution 2.
Moving the node utilizes b2World.ShiftOrigin and all bodies stay in the same screen-space coordinates.
Perhaps there could be an option/property to either A. lock bodies to screen-space, or B. inherit Box2DWorld's translation changes. This might be stupid, I don't know.

@briansemrau briansemrau added bug Something isn't working PRs welcome labels Dec 6, 2020
@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Whats are the cons to 1?

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Does solving in the Box2dWorld node solve it everywhere? Does any transform in the scene tree (above or below the Box2DWorld node) have undefined behavoir?

@briansemrau
Copy link
Owner Author

What are the cons to 1?

If there's any opportunity to incorporate the functionality of b2World.ShiftOrigin in an intuitive way, I think this is it.

The first solution fixes the bug in the short term, but I'm not sure it'll be the final design.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

what about scale and rotation?

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Node2d at the root has a scale X of 2.

Screen Shot 2020-12-06 at 9 16 28 PM

Screenshot editor

Screen Shot 2020-12-06 at 9 16 45 PM

Screenshot play

Sprite under box2d doesn't scale X

@briansemrau
Copy link
Owner Author

Godot doesn't support scale for collision shapes, so maybe it's not unreasonable to leave out support for those types of transformations and just document it.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Sorry, my screenshots may be confusing. i'm not scaling anything of the collision shapes. I'm scaling the root node in the scene tree (Node2D). (Which makes everything look stretched). But when I play the scene, the sprite attached to the box2d body isn't stretched.

The sprites 'ball' and 'ball1' are the same sprite, and don't have any transformation directly on them. But you'll notice in my screencap one is just under the scene root, and one is attached to the b2Body node. When i play the scene, one transformation is respected, the other isn't.

Likewise for rotation. If I rotate any node in the scene tree above the Box2DWorld node

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Screen Shot 2020-12-06 at 9 28 13 PM

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

forgive me I'm still getting up to speed with box2d_physics_body.cpp

I'm just looking through code there now and seeing how global transforms are done. I may try and tweak a little.

Do we want to be doing anything with global_transforms? Why not base everything local? Or at lease relative to the Box2dWorld node?

@briansemrau
Copy link
Owner Author

briansemrau commented Dec 7, 2020

Sorry, I didn't see the screenshots when I replied from my phone. I think I understand now.

At the moment, everything is using global transforms because local transforms make the following hierarchy impossible:

Box2DWorld
└─ (Node2D) PileOfBricks
      └─ (several Box2DPhysicsBody nodes)

I currently aim to use neither of these transforms in the future and instead use "relative to Box2DWorld transform", though I don't know how that may interact with scaling and rotation. Unless there's a good use-case for rotating and scaling the world, I think it might be a bit complex to figure out.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Great. That's what I was hoping for, as I think the easiest would be relative to box2dworld transform.

I mean, i still want to be able to 'zoom' in on my world from above the Box2DWorld node. I definitley do this all the time. The World node itself isn't scaled, but say a node above it.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

SomeViewNode2D
└─ Box2DWorld
      └─ (several Box2DPhysicsBody nodes)

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

It would be great to support local transforms in the tree under Box2DWorld, although I'm thinking how it would even work. It would need to adjust shapes, and I'm considering the ramifications of that, wrecking contacts, etc. Even too many b2Fixture::Synchronize will call performance problems. Still would be cool though if there's an elegant solution. I can't think of one right now though, that doesn't involve changing fixtures though.

@briansemrau
Copy link
Owner Author

Yeah, changing transforms would have potentially drastic implications. If I scale my PileOfBricks example mid-simulation, what happens? I'd have to see what Godot supports, and likely follow that. Though, I don't expect this is something it allows.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Exactly. Ya i mean, godot doesn't support it. It's just that it's apparently been a source of confusion for new developers in godot.

I mean the only saving grace, is that any modification would be outside a world->step(). I've thought about this a few times, and it continually boils down to moving shape vertices (or parameters for shapes like circle). (or removing and re-adding fixtures with new adjusted vertices or parameters). But at that point contacts are going to be blown out and collisions are going to be not great weird.

But i guess, even circle doesn't work, because with a scaleX anywhere it ceases to stay a circle..

Maybe the editor should just NOT allow any Node2D directly under the Box2DWorld (or at least show a warning or something). If you want to group Box2DPhysicsBody nodes together, it could be done with a simple Node under the Box2DWorld.

@briansemrau
Copy link
Owner Author

So Node2D is a necessity so that you can create instanced that can be placed throughout the world, but scaling should definitely not be supported.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Is that a real workflow people have? I actually have no clue. So you can 'group' Box2DPhysicsBody's together under a Node2D and move them all around in the editor? Then you still have the issue of potentially moving that Node2D as well mid-simulation no?

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Maybe there's a possibility to detect Node2D's between the Box2DWorld and Box2DPhysicsBody's and disable scale even in the editor, or even show a warning on that property or something.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

I've modified the rigid bodies to use 'relative to world' transforms, instead of global. This project just has a ball bouncing on a box. In this video, I've scaled the world node, and I'm rotating it every frame in a script. Transforms from the WorldNode and up the scene tree not longer have an impact on the transforms of the physics nodes. And because the transform is relative to world, it still supports putting 'Node2D's in-between the Box2dWorldNode and Box2dPhysicsBody's. Everything kinda works as expected.

See here:

https://youtu.be/PcussH-2q3c

Here is what it looks like un-scaled, and not rotating:
Screen Shot 2020-12-07 at 4 19 24 PM

Thoughts @briansemrau ?

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

Here is again, the same project, but I am scaling the world node as well:

https://youtu.be/jKf4eTU42Xs

@briansemrau
Copy link
Owner Author

Am I correct in understanding that these different transforms are leaving the simulation untouched, but is still applying the scale/rotation/offset to the visual elements?

Also, does this lead to any visual bugs with the fixture shape editor handles?

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

It's a wip, and I'll clean it up and I'll verify the editor tools are working correctly next, but they will just work if I swap any set/get global xform funcs, to the world xform func. But basically think of it this way. Everytime you move from b2d to godot, instead of going to the global space, we go to the world space. And vice versa.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

So take any transformation from the Box2dWorld node and above in the scene tree (inclusive), and it won't change any actual placement of anything in the box2d simulation.

@jordo
Copy link
Collaborator

jordo commented Dec 7, 2020

The editor tools should 'just work'. Even if you are working in a node, and a root node higher in the tree is 'scaled' or anything, you will actually used the tools in the editor under a 'scaled' view. But when we actually set the properties of the b2Body (pos, rotation), or shape vertices, they are always in coordinates in the world space. So the skewed view at the editor level doesn't really change any values.

Will clean up, and submit something tomorrow 👍

@briansemrau
Copy link
Owner Author

Alright, sounds good. Thanks for figuring this one out!

@jordo
Copy link
Collaborator

jordo commented Dec 8, 2020

Can probably close with with #20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants