-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Collision: more accurate computation with acceleration and long dtime #15408
base: master
Are you sure you want to change the base?
Conversation
3ea6af2
to
217fd70
Compare
217fd70
to
517c191
Compare
You can rebase now. |
517c191
to
52534f3
Compare
bd43927
to
ecca780
Compare
@appgurueu: working on taking acceleration into account directly. |
00989ce
to
890e993
Compare
@sfan5 @appgurueu this version now is probably good for reviewing and testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not doing a thorough review, but I looked into the MinGW CI failure. Please include the <algorithm>
header since you used std::sort
; it's transitively depending on an include all the way down in an irrlicht/ header, and that also makes the compiler errors much more confusing.
890e993
to
6875e45
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First review iteration. Works well from what I can see so far.
Tested:
- singleplayer: walking
- local server with
dedicated_server_step = 0.6
: falling nodes
66c8332
to
f4ace0f
Compare
e0c226b
to
eb6b3ec
Compare
eb6b3ec
to
e181488
Compare
aeb560d
to
1442a18
Compare
Giving up on the parabolic movement for now. I could not get rid of all the glitches. |
1442a18
to
1cbb84a
Compare
1cbb84a
to
2b8ea86
Compare
rebased, no changes. Ready for review. |
Slippery test Tested with this script: #15375 (comment) (N° 173 for my reference)
☑️ This is good enough. Bouncy test Physical entitiy code: diff --git a/games/devtest/mods/testentities/visuals.lua b/games/devtest/mods/testentities/visuals.lua
index 6bb1b8282..7517f7a3e 100644
--- a/games/devtest/mods/testentities/visuals.lua
+++ b/games/devtest/mods/testentities/visuals.lua
@@ -52,7 +52,11 @@ core.register_entity("testentities:mesh", {
textures = {
"testnodes_mesh_stripes2.png"
},
+ physical = true
},
+ on_activate = function(self)
+ self.object:set_acceleration({ x = 0, y = -9.81, z = 0 })
+ end,
})
core.register_entity("testentities:mesh_unshaded", { Setup: sand pillar +
|
That last test goes beyond the scope of this pull request, as it requires computing the parabolic movement. |
Fixes a regression from cb6c8eb
2b8ea86
to
afe1cad
Compare
Refreshed version of #15029 (reverted for 5.10.0 by #15400, includes #15378)
This fixes inaccuracies in the old collision code which in essence assumed a linear movement, and in case of a collision still used the final speed.
This version:
uses a parabolic computation with acceleration(postponed for now, did not get it to work perfectly)sorts node boxes to (hopefully) find collisions faster(dropped, did not improve enough)It is still an approximation to not need to iterate too much - consider a ball bouncing on a surface, and we have a long time step: the ball might do multiple bounces; this code will still only allow one per timestep: to avoid issues when standing on a floor, any collision will zero the axis velocity (unless bouncy) and zero the acceleration (always) for now.
The ultimate solution would be to make everything so fast that we can do much smaller dtime steps and all of this does not matter anymore.
The following cases should be tested: