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

Expose allow_sleep and awake Box2D parameters #5405

Closed
totebo opened this issue Dec 14, 2020 · 31 comments · Fixed by #5848 or #8712
Closed

Expose allow_sleep and awake Box2D parameters #5405

totebo opened this issue Dec 14, 2020 · 31 comments · Fixed by #5848 or #8712
Labels
engine Issues related to the Defold engine feature request A suggestion for a new feature good first issue? The issue might be good for beginners wanting to work on the engine or the editor physics Issues related to physics (2D and/or 3D)

Comments

@totebo
Copy link
Sponsor Contributor

totebo commented Dec 14, 2020

Is your feature request related to a problem? Please describe (REQUIRED):
Dynamic bodies are deactivated from the physics simulation if no forces have been affecting them for a certain time.

In my zero gravity game the player starts the game by connecting the player body to a different body. If the player body has gone to sleep before creating the joint, the joint doesn't work - it connects but is passive. This means physics.set_joint_properties() no longer has any effect.

Describe the solution you'd like (REQUIRED):
Expose the allow_sleep flag in Box2D so that that a body can be set to never go to sleep. Also add an physics.is_awake() to check if a body is sleeping, if using allow_sleep is not desired.

Describe alternatives you've considered (REQUIRED):
Applying a small impulse every frame to keep the body awake.

Additional context (OPTIONAL):

b2BodyDef bodyDef;
bodyDef.allowSleep = true;
bodyDef.awake = true;

https://box2d.org/documentation/md__d_1__git_hub_box2d_docs_dynamics.html

@totebo totebo added the feature request A suggestion for a new feature label Dec 14, 2020
@totebo totebo changed the title Expose allow_sleep Box2D parameter Expose allow_sleep and awake Box2D parameters Dec 14, 2020
@britzl britzl added engine Issues related to the Defold engine physics Issues related to physics (2D and/or 3D) labels Dec 14, 2020
@britzl britzl added the good first issue? The issue might be good for beginners wanting to work on the engine or the editor label Apr 12, 2021
@britzl
Copy link
Contributor

britzl commented Apr 12, 2021

Also add an physics.is_awake() to check if a body is sleeping, if using allow_sleep is not desired.

In which scenarios is allow_sleep not desired?

@totebo
Copy link
Sponsor Contributor Author

totebo commented Apr 12, 2021

Bodies that never sleep potentially consume more resources, because they are never excluded from the simulation.

If they're allowed to sleep it's handy to know wether they are asleep or not, for example to end the level in Angry Birds.

@otsakir
Copy link
Contributor

otsakir commented Apr 17, 2021

So, @britzl , @totebo , this is about adding read/write methods for two Box2D properties. Right ?

  1. allowSleep that controls whether a box2d body will fall asleep if inert for a long time.
  2. awake that determines the sleep status of a body and allows to change it.

Any second thoughts on that ?

I'm not an expert in physics code, but the way i see it, if this is just about being able to run physics.set_joint_properties() seamlessly, why not just do that ? We could make sure that set_joint_properties() checks first that the object is awake using the respective Box2d api. If it isn't wake it up and then modify the joint properties. This logic would be part of the internal physics implementation. Nothing will change in the public lua api.

On the other hand, if we really need to extend the public physics api, we could indeed expose allowSleep and awake b2Body properties in the lua api.

https://github.com/defold/defold/blob/dev/engine/physics/src/box2d/Box2D/Dynamics/b2Body.h#L315-L327

Thoughts ?

@totebo
Copy link
Sponsor Contributor Author

totebo commented Apr 17, 2021

I think the best approach would be to support the standard Box2D API for body sleep functionality. This would solve the joint issue and also allow other uses to do with sleeping bodies.

@britzl
Copy link
Contributor

britzl commented Apr 18, 2021

this is just about being able to run physics.set_joint_properties() seamlessly

Doesn't that function only operate on joints/constraints? Aren't the sleep and awake properties on physics bodies?

And we should probably access them through go.get() and go.set() on the collision component.

@totebo
Copy link
Sponsor Contributor Author

totebo commented Apr 18, 2021

Doesn't that function only operate on joints/constraints?

Yeah, I don't think physics.set_joint_properties() is relevant in this case.

we should probably access them through go.get() and go.set() on the collision component.

Sounds perfect to me!

@otsakir
Copy link
Contributor

otsakir commented Apr 18, 2021

@britzl , let me try to draft the implementation.

  1. Introduce new allow_sleep property to the collision-object component. Both get/set operations will be possible essentially wrapping Box2D's b2body::IsSleepingAllowed() and b2body::SetSleepingAllowed().
  2. Introduce new awake property to the collision-object component. get/set operations will be possible and wrap Box2D's b2body::IsAwake() and b2body::SetAwake().

Example use - wake up a collision object if sleeping

if not go.get("#collisionobject1", "awake") then
    go.set("#collisionobject1","awake", true)
end

Example use - disable sleeping preemptively

go.set("#collisionobject2", "allow_sleep", false)

Editor

Do we need support in editor for these properties ? If we do, should this be implemented as part of this issue ?

@britzl
Copy link
Contributor

britzl commented Apr 19, 2021

draft the implementation

This looks good. Keep in mind that we have 2 physics engines (one for 2D and one for 3D). Does Bullet have similar properties?

Do we need support in editor for these properties ? If we do, should this be implemented as part of this issue ?

We do not need this, but it is a nice thing to have. And it would be for the new "allow_sleep" property if we decide to add it.

@JCash
Copy link
Contributor

JCash commented Apr 19, 2021

Also note that another approach is to wake up the body when we "touch" it. I.e. when connecting two bodies with a new joint constraint.

I don't mean to say that the setawake() functionality isn't desired, but I don't think it's the best solution to this particular problem.

@otsakir
Copy link
Contributor

otsakir commented May 2, 2021

@britzl I skimmed through bullet's docs (those i could find :-). Sharing some thoughts...

Although there seems to be a similar behaviour in bullet that deactivates a body if it comes to rest, we're still talking about a different API (from Box2D's). Both naming and semantics should be considered in order to have neutral api and not a Box2D biased feature.

For semantics, bullet has the following definitions:

#define ACTIVE_TAG 1
#define ISLAND_SLEEPING 2
#define WANTS_DEACTIVATION 3
#define DISABLE_DEACTIVATION 4
#define DISABLE_SIMULATION 5

ACTIVE_TAG and ISLAND_SLEEPING seem to express status while the rest probably indicate transition.

Copied them from this - https://pybullet.org/Bullet/BulletFull/btCollisionObject_8h_source.html

Here follow the respective box2d and bullet apis in theory. I checked the docs and went through the code. I didn't run any code.

Check if awake

box2D - b2Body::IsAwake()
bullet - body.getActivationState() == ACTIVE_TAG

I'm not sure about putting explicitly to sleep. I'd stick to read-only behaviour.

Prevent sleeping

box2d - b2Body::SetSleepingAllowed(false)
bullet - maybe body.setActivationState(DISABLE_DEACTIVATION)

Trying to move the discussion forward, i think i'll agree with @JCash. Waking up a body in certain occasions behind the scenes seems an easier, quicker and safer fix for the initial use case reported by @totebo.

On the other hand we could proceed with implementing only the operation of checking-if-awake() for both box2d/bullet which seems clear and safe if read-only if it is useful.

Thoughts people ?

@britzl
Copy link
Contributor

britzl commented May 2, 2021

Both naming and semantics should be considered in order to have neutral api and not a Box2D biased feature.

Yes, I agree!

I'm not sure about putting explicitly to sleep. I'd stick to read-only behaviour.

Sounds reasonable to keep these read-only!

we could proceed with implementing only the operation of checking-if-awake() for both box2d/bullet which seems clear and safe if read-only if it is useful

Yes, the ability to check if objects are sleeping is useful. See the example given by @totebo regarding Angry Birds levels!

Waking up a body in certain occasions behind the scenes seems an easier, quicker and safer fix for the initial use case reported by @totebo

How would this work?

@otsakir
Copy link
Contributor

otsakir commented May 3, 2021

Waking up a body in certain occasions behind the scenes seems an easier, quicker and safer fix for the initial use case reported by @totebo

How would this work?

To address the initial use case reported we could hook physics.set_joint_properties(). Transparently awake collision objects involved there by invoking Box2D.SetAwake(). Not the best solution, a little hackish and partly solves the problem. It might make some sense though if we don't want to introduce new public properties.

As a personal preference i'd suggest moving in small steps. First implement IsAwake() which seems clearer, will give us eyes on the internal sleeping state and allow better understanding and testing.

@britzl
Copy link
Contributor

britzl commented May 3, 2021

As a personal preference i'd suggest moving in small steps. First implement IsAwake() which seems clearer, will give us eyes on the internal sleeping state and allow better understanding and testing.

Agreed!

@otsakir
Copy link
Contributor

otsakir commented May 7, 2021

Started working on this.

@otsakir
Copy link
Contributor

otsakir commented May 7, 2021

It seems that the low level C getters for the sleeping status of collision objects are already present both for box2d and bullet.

https://github.com/defold/defold/blob/dev/engine/physics/src/physics/physics_2d.cpp#L1089
https://github.com/defold/defold/blob/dev/engine/physics/src/physics/physics_3d.cpp#L1008

I didn't find any reference to them though in the engine code. Hmmm...

@JCash , any idea if they are not used because they weren't needed so far or if there is another reason to consider ?

@JCash
Copy link
Contributor

JCash commented May 8, 2021

I don't think there's a special reason other than that we don't want to add all functions and attributes, but add them as needed.

@otsakir
Copy link
Contributor

otsakir commented May 10, 2021

Dynamic bodies are deactivated from the physics simulation if no forces have been affecting them for a certain time.

@totebo , any idea how much time is that ? I'm trying to reproduce.

@totebo
Copy link
Sponsor Contributor Author

totebo commented May 11, 2021

From the manual:

When Box2D determines that a body (or group of bodies) has come to rest, the body enters a sleep state which has very little CPU overhead.

https://box2d.org/documentation/md__d_1__git_hub_box2d_docs_dynamics.html

In practice, the body appears to be stationary for some time before it sleeps, but this might have to do with precision. Which means there probably is no fixed time.

@otsakir
Copy link
Contributor

otsakir commented May 13, 2021

Sharing some findings ...

  1. In box2D falling to sleep is almost instant once a body stops moving. Thus, it makes no sense to assume an object is awake after manually setting its 'sleeping' property to false. It will fall right asleep again.
  2. A joint created between two sleeping object has no effect. If any of the objects is waken, it does become effective. Thus, there is some value in supporting 'writing' to the 'sleeping' property. Users will need to do that manually for one of the joint's collision objects i.e. go.set("/goX#collisionobject","sleeping",false).
  3. I think we should avoid messing with "sleeping-allowed" box2d property to avoid complications. It's updated here too:
    https://github.com/defold/defold/blob/dev/engine/physics/src/physics/physics_2d.cpp#L425
  4. In 3D physics, objects get asleep right away when they stop moving. Same behavior as in Box2D. Other aspects of the issue are not relevant though. Creating a fixed joint is not supported.

To cut a long story short, for this issue, i suggest read/write support for sleeping, no support for sleeping-allowed. Both for 2D and 3D.

Pending: Investigate how to awake a body in 3D.

@otsakir
Copy link
Contributor

otsakir commented May 13, 2021

One more thing:

  • Awaking a body makes sense as discussed.
  • Putting it to sleep explicitly doesn't. It will fall asleep by itself if needed. In addition, in Bullet this operation doesn't seem conventional. It looks a bit hacky if at all possible - https://gamedev.net/forums/topic/637030-deactivate-a-rigid-body-in-bullet/5019431/
  • We obviously want to use the property API for the 'sleeping' property instead of other arbitrary method names like 'awake()'.
  • We need both get() and set()

To get to the point, set('sleeping', true) makes no sense and is not clean in Bullet but we need set('sleeping', false) which works ok and cleanly in both physics backends.

I'm thinking to either ignore set('sleeping', true) in bullet backend or issue a warning that such operation is not possible/supported.

Do you see a problem with that ? Any alternatives to suggest ?

@britzl
Copy link
Contributor

britzl commented May 14, 2021

  • Putting it to sleep explicitly doesn't. It will fall asleep by itself if needed

I agree. It should be up to the physics simulation to determine when a body is asleep.

Do you see a problem with that ? Any alternatives to suggest ?

  • We obviously want to use the property API for the 'sleeping' property instead of other arbitrary method names like 'awake()'

I'm not sure. It feels a bit weird to have a go.set("sleeping", true|false) that doesn't accept both values. Would it really be weird to have an "awaken" or "wake_up" message that can be sent to collision component to wake it up?

@otsakir
Copy link
Contributor

otsakir commented May 14, 2021

Would it really be weird to have an "awaken" or "wake_up" message that can be sent to collision component to wake it up?

@britzl , note we're talking about the 'go-to-sleep' command and not the 'wake-up' operation and our obstacle is in Bullet only.

Yeah it's kind of weird if you only think in terms of API and not what underlies it. My point is that thinking semantically, our problem is that sometimes our collision object over-sleep and we had no way to wake them up + we wanted to know whether they sleep or not. Putting them explicitly to sleep was not a use case and seems a vain operation too (both engines put them to sleep again). I would even say that if the underlying layer does not have a clean way to do it, it would be misleading for Defold developer to claim that it's possible in our API.

To me sleeping seems more an optimization feature to exclude objects from the simulation to save CPU. We do take advantage of it but we may have to deal with behavior we can't fully control.

Again, to make sure we're on the same page my suggestion is to display a warning when user gives go.set("...", "sleeping", true) in Bullet, like "Putting to sleep is automatically handled by underlying physics engine".

That being said, i'll check some more about putting-to-sleep in bullet to see if I'm missing something.

@britzl
Copy link
Contributor

britzl commented May 14, 2021

Ok, so this is the suggested new API/property for both 2D and 3D physics:

go.get("#collisionobject", "sleeping") -- true or false depending on physics simulation

go.set("#collisionobject", "sleeping", "false") -- wake up

go.set("#collisionobject", "sleeping", "true") -- no effect and warning

What I'm not sure about is this:

go.set("#collisionobject", "sleeping", "true")

It feels a bit weird to not allow any value. BUT on the other hand we do have checks in place for other values such as linear_damping which must be within 0.0 and 1.0 so I guess it is ok. The alternative would be the message I wrote in my previous post, but it is more complicated.

@JCash do you have an opinion?

@otsakir
Copy link
Contributor

otsakir commented May 17, 2021

Changing course a little. Instead of setting the 'sleeping' property in the collision object, a wakeup() method will be implemented.

otsakir added a commit to otsakir/defold that referenced this issue May 18, 2021
otsakir added a commit to otsakir/defold that referenced this issue May 18, 2021
@otsakir
Copy link
Contributor

otsakir commented May 18, 2021

Added wakeup() implementation.

Also tested on sample project with physics debugging enabled. With both backends it works as expected.

box2d
  1. Create two gameobjects with collision object attached.
  2. Let them fall and rest on a base (they change color in debug mode)
  3. Create joint (a line connecting then appears but nothing else happens).
  4. Call physics.wakeup("/go1#collisionobject"). The objects change color start moving and come close to each other.
bullet

The same use case is not possible because bullet doesn't support this type of joint. However, the object's frame still changes colour for a split sec when activated.

@JCash , for testing i plan to add a new test case to test_gamesys.cpp. See TEST_F(ComponentTest, JointTest) and do sth similar. Have two resting gameobjects, create a joint between them and check that they moved.

Does this look ok to you ? Anything else to suggest ?

@JCash
Copy link
Contributor

JCash commented May 19, 2021

Sounds good to me! 👍

otsakir added a commit to otsakir/defold that referenced this issue May 24, 2021
@otsakir
Copy link
Contributor

otsakir commented Jun 2, 2021

Almost done. Still have to merge stuff from dev branch to get up to date before creating the PR.

otsakir added a commit to otsakir/defold that referenced this issue Jun 3, 2021
otsakir added a commit to otsakir/defold that referenced this issue Jun 10, 2021
JCash pushed a commit that referenced this issue Jun 14, 2021
* Implemented 'wakeup' call in 2D physics (box2d) - ref #5405

* collision object activation for bullet 3d - ref #5405

* WIP - Test code and files for #5405

* WIP on 5405

* proof of concept for custom project config in gamesys tests cases

* Issue 5405: gamesys test for waking up sleeping box2d objects - ref #5405

* Issue 5405: final review and fixed minor stuff - ref #5405

* Applied fixed requested by PR #5848 - ref #5405
@britzl britzl linked a pull request Jun 14, 2021 that will close this issue
@britzl britzl added this to Discuss in Issue Review Jul 21, 2021
@britzl britzl moved this from Discuss to Yes in Issue Review Jul 21, 2021
pull bot pushed a commit to proteanblank/defold that referenced this issue Jul 28, 2021
* Implemented 'wakeup' call in 2D physics (box2d) - ref defold#5405

* collision object activation for bullet 3d - ref defold#5405

* WIP - Test code and files for defold#5405

* WIP on 5405

* proof of concept for custom project config in gamesys tests cases

* Issue 5405: gamesys test for waking up sleeping box2d objects - ref defold#5405

* Issue 5405: final review and fixed minor stuff - ref defold#5405

* Applied fixed requested by PR defold#5848 - ref defold#5405
@subsoap
Copy link
Contributor

subsoap commented Sep 14, 2021

Being able to detect if a 3d physics is sleep would be useful. For now using average position and orientation over time to detect sleep.

@britzl
Copy link
Contributor

britzl commented Sep 14, 2021

For now using average position and orientation over time to detect sleep.

What about reading angular_velocity and linear_velocity on the object? Or are the examples of these being zero and still considered awake?

@subsoap
Copy link
Contributor

subsoap commented Sep 14, 2021

What about reading angular_velocity and linear_velocity on the object? Or are the examples of these being zero and still considered awake?

I had not realized those were an option. I'll try that!

@britzl
Copy link
Contributor

britzl commented Sep 14, 2021

I had not realized those were an option. I'll try that!

I haven't tried myself but worth checking at least!

@JCash JCash mentioned this issue Mar 25, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine Issues related to the Defold engine feature request A suggestion for a new feature good first issue? The issue might be good for beginners wanting to work on the engine or the editor physics Issues related to physics (2D and/or 3D)
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants