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

Add PhysicsServer2/3D::space_step() to step physics simulation manually #76462

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Daylily-Zeleen
Copy link
Contributor

@Daylily-Zeleen Daylily-Zeleen commented Apr 26, 2023

Implement #1373
In #2821, it seems like the topic is quite big, but I think this pr can solve it.

Here is a simple demonstrate video of rollbackable/recordable physics simulation create by this pr:

rollbackable_simulation.mp4

But this pr is not perfect, there has two points which confuse me (I'm not familiar with multi-threads and GodotPhysics), and I mark them at below.

Here is rollback demo:
physics_rollback_test.zip

@Daylily-Zeleen Daylily-Zeleen changed the title Add space_step() to step physics simulate manually Add PhysicsSrver2/3D::space_step() to step physics simulation manually Apr 26, 2023
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/physics_space_step branch 2 times, most recently from b769061 to 2419c05 Compare April 26, 2023 05:47
void GodotPhysicsServer3D::space_flush_queries(RID p_space) {
// TODO::
// Like _update_shapes(), to provide controllability for developers, flushing_queries flag should active as a member of space and check it for each space.
// But I'm not sure about that, I am not familiar with multi-threads and the architecture of GodotPhysics.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second point which confuse me.

// TODO::
// May be let pending_shape_update_list as a member of GodotSpaces3D and update shapes by themselves.
// To avoid effecting Spces which are handled by developer (for lockstep/rollback netcode, it is particularly sensitive).
// If it is unecessary, call _update_shapes() directly.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first point which confuse me.

@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/physics_space_step branch from 2419c05 to 48553ed Compare April 26, 2023 05:55
@Daylily-Zeleen Daylily-Zeleen marked this pull request as ready for review April 26, 2023 05:56
@Daylily-Zeleen Daylily-Zeleen requested review from a team as code owners April 26, 2023 05:56
@Chaosus Chaosus added this to the 4.1 milestone Apr 26, 2023
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/physics_space_step branch 2 times, most recently from ffbacd8 to e6573c3 Compare April 28, 2023 08:56
@Lcbx
Copy link
Contributor

Lcbx commented May 14, 2023

Hello @Daylily-Zeleen,
for thread-safety, you might want to call the functions sync() and end_sync() of the PhysicsServer classes
an example of usage is in the function Main::iteration() in file main/main.cpp

Saw your PR when making mine ; yours has more general usage but might need some tweaks.

@Daylily-Zeleen
Copy link
Contributor Author

for thread-safety, you might want to call the functions sync() and end_sync() of the PhysicsServer classes
an example of usage is in the function Main::iteration() in file main/main.cpp

In my opinion, the PhysicsSpace which be stepped by step() is handled by developers, to run in which thread, sync or not, are controled by developers themselves.

Instead of call sync() and end_sync() in step(), expose spcas_sync(RID p_space) and space_end_sync(RID p_space) may be better.

I need people, who familiar with godot physics and server design, to instruct me.

@akien-mga akien-mga changed the title Add PhysicsSrver2/3D::space_step() to step physics simulation manually Add PhysicsServer2/3D::space_step() to step physics simulation manually Jun 19, 2023
@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 19, 2023
@Az-qianchen
Copy link

When will this feature be merged? I need this functionality to assist me in implementing my network synchronization.

@AThousandShips
Copy link
Member

AThousandShips commented Sep 13, 2023

@Az-qianchen it hasn't been reviewed yet, so there's no real timeframe for this being merged at the moment, the questions of the OP about some details would need to be resolved as well by someone experienced with the physics

If you can test it and give your results and comments it would help the process

servers/physics_2d/godot_physics_server_2d.cpp Outdated Show resolved Hide resolved
servers/physics_3d/godot_physics_server_3d.cpp Outdated Show resolved Hide resolved
servers/physics_3d/godot_physics_server_3d.cpp Outdated Show resolved Hide resolved
servers/physics_3d/godot_physics_server_3d.cpp Outdated Show resolved Hide resolved
servers/physics_2d/godot_physics_server_2d.cpp Outdated Show resolved Hide resolved
servers/physics_2d/godot_physics_server_2d.cpp Outdated Show resolved Hide resolved
@Az-qianchen
Copy link

2023-09-13.180402.mp4

I am not sure if the way I use is correct, but when I simulation is positive, I can get the correct result. When the value is negative, the result seems to have an error.

@Daylily-Zeleen
Copy link
Contributor Author

Daylily-Zeleen commented Sep 13, 2023

When the value is negative, the result seems to have an error.

How to step a physics space is not this pr's work.

I guess your purpose of passing a negative time to space_step() is to roll the physics space back to the previous state (for example, 0.1 second before).
This cannot be achieved through space_step(), this method is to simulate a physical space forward, you can passing a negative delta time to simulate, but it just simulate forward by a negative delta time.

The right way to implement rollback feature is store your physics objects' state, and just restore them when you want to roll back.
Typically, you should create a independent physics space to realize your physics simulation instead of using default space, then use space_step() to setp this space, to make your physics simulation under your control fully.

@Az-qianchen
Copy link

Az-qianchen commented Sep 14, 2023

you should create an independent physics space to realize

I encountered some obstacles when creating a physical space. Do I need to copy all PhysicsBody3D objects to the new physical space, and all objects must be StaticBody3D to perform space_step() on a single object and obtain the correct collision?
Is there other shortcuts?

@Daylily-Zeleen
Copy link
Contributor Author

Daylily-Zeleen commented Sep 14, 2023

Do I need to copy all PhysicsBody3D objects to the new physical space,

If your physics object are created as a node (CollisionObject2/3D or Joint2/3D) and want to move them from default space to your own space, please use PhysicsServer2/3D.xxx_set_space(), you don't need to copy them.

and all objects must be StaticBody3D to perform space_step() on a single object and obtain the correct collision?

There has not limit the type of physics objet.


I uploaded a testing project for reference.

Here is a rollback demo:
physics_rollback_test.zip

@Az-qianchen
Copy link

There has not limit the type of physics objet.

I may not be very clear. In fact, I hope to make a Step Physics Simulation in a certain object in the scene, but in order to get the correct interaction collision, I seem to need to copy a static scene in the simulation space.

Or turn PhysicsBody into staticbody in the original scene to prevent being simulated together

The following is the code that I tried to transfer the copy to the new physical space, but I still did not think about how to deal with the problem of retaining PhysicsBody and avoiding being simulated together.

class_name SubSpace3D
extends Node

@export var node_mirror: Node

var space_rid: RID
var space_rid_new: RID

func _ready() -> void:
# Body is to obtain the default space RID,maybe there is a better way?
	var Body = StaticBody3D.new()
	add_child(Body)
	space_rid = PhysicsServer3D.body_get_space(Body.get_rid())
	space_rid_new = PhysicsServer3D.space_create()

# Configure is a singleton to facilitate data calls from other locations
	Configure.space_rid = space_rid
	Configure.space_rid_new = space_rid_new

# Copy the basic data of the scene
	create(space_rid, space_rid_new)
	PhysicsServer3D.space_set_active(space_rid_new, true)
	Body.free()

# Nodes that need to be copied
	_update_space(node_mirror)

func _update_space(node:Node):
	var node_array: Array[Node]

	if node is RigidBody3D:
		node_array.push_back(node)

	for child in node.get_children():
		_update_space(child)

	for collisionobject in node_array:
		var node_new := collisionobject.duplicate()
		node.add_child(node_new)
		PhysicsServer3D.body_set_space(node_new.get_rid(),space_rid_new)

func _enter_tree():
	get_tree().connect("node_added",_node_added)
func _exit_tree():
	get_tree().disconnect("node_added",_node_added)

func _node_added(node:Node):
	await ready
	if is_ancestor_of(node) and node is CollisionObject3D:
		PhysicsServer3D.body_set_space(node.get_rid(),space_rid_new)

func set_gravity(space, new_space):
	var gravity: float = PhysicsServer3D.area_get_param(space, PhysicsServer3D.AREA_PARAM_GRAVITY)
	PhysicsServer3D.area_set_param(new_space, PhysicsServer3D.AREA_PARAM_GRAVITY, gravity)

func set_gravity_vector(space, new_space):
	var gravity_vector: Vector3 = PhysicsServer3D.area_get_param(space, PhysicsServer3D.AREA_PARAM_GRAVITY_VECTOR)
	PhysicsServer3D.area_set_param(new_space, PhysicsServer3D.AREA_PARAM_GRAVITY_VECTOR, gravity_vector)

func set_linear_damp(space, new_space):
	var linear_damp: float = PhysicsServer3D.area_get_param(space, PhysicsServer3D.AREA_PARAM_LINEAR_DAMP)
	PhysicsServer3D.area_set_param(new_space, PhysicsServer3D.AREA_PARAM_LINEAR_DAMP, linear_damp)

func set_angular_damp(space, new_space):
	var angular_damp:float = PhysicsServer3D.area_get_param(space, PhysicsServer3D.AREA_PARAM_ANGULAR_DAMP)
	PhysicsServer3D.area_set_param(new_space, PhysicsServer3D.AREA_PARAM_ANGULAR_DAMP, angular_damp)

func get_direct_space_state(_active):
	return PhysicsServer3D.space_get_direct_state(_active)

func create(space, new_space):
	set_gravity(space, new_space)
	set_gravity_vector(space, new_space)
	set_linear_damp(space, new_space)
	set_angular_damp(space, new_space)

@Daylily-Zeleen
Copy link
Contributor Author

@Az-qianchen Sorry, I don't think I can help you solve this specific application scenarios.

After reading your code, I found that you set the space_rid_new to actived. I'm not sure about your situation, but I think I need to clearify something here:
A actived physics space will be stepped automatically by the main loop, if you want to control by yourself completely, you should ensure the physics space is Inactivated.

@monitz87
Copy link

  • Events may not correctly trigger again when resimulating (i.e. things like area triggers may not work more than once). Untested. I'm sure I saw someone else point this out but I can't seem to find it so maybe it was found to not be the case.

Emit signals /events are implemented in CollisionObject nodes, it should be handled in another pr.

To provide a way to resimulate CollisionObject nodes, I think we can add two virtual method virtual CollisionStateXD CollisionObjectXD::get_state() const; and virtual void CollisionObjectXD::set_state(const Ref<CollisionStateXD>& p_state) const, and override them in child classes.

Assuming these methods would get/set both the internal state of the Node and the related object in the physics server, this would be a good way of handling the matter.

It wouldn't be complete without adding the corresponding PhysicsServer2D low level methods to get/set state that is currently not available (like the list of areas and bodies that entered a specific area for example)

@monitz87
Copy link

Implement #1373 In #2821, it seems like the topic is quite big, but I think this pr can solve it.
Here is a simple demonstrate video of rollbackable/recordable physics simulation create by this pr:
rollbackable_simulation.mp4
But this pr is not perfect, there has two points which confuse me (I'm not familiar with multi-threads and GodotPhysics), and I mark them at below.
Here is rollback demo: physics_rollback_test.zip

Here is a C# version of the GD script using List<> instead of Godot.Collections.Array<> https://pastebin.com/uxkxR9Ph Seems to be working quite well. If you want a steam key for my commercial game that'll be using this, tell me what your discord is.

I have high hopes now that it's working in my networked project even though my netcode needs touching up.

EDIT: Ignore the Godot.Collections.Array<> that I didn't convert to List<> haha

Don't want to beat a dead horse here, but I strongly recommend that you read this comment if you haven't. I'm not sure about your specific netcode implementation, but if you're doing rollback, you're most likely going to have those issues with this PR.

The comment mentions signals, but even if you're not using them, polling for overlapping areas/bodies will still fail to resimulate in certain scenarios. Other things like move_and_slide are also prone to produce different results on resimulation due to these limitations.

Once again, this is not to say that this PR isn't useful, or that it shouldn't be merged. Quite the contrary. It's just not sufficient for "full" rollback (i.e. being able to go back to a previous state and reproducing the exact same results in every scenario)

@hislittlecuzin
Copy link

Don't want to beat a dead horse here, but I strongly recommend that you read this comment if you haven't. I'm not sure about your specific netcode implementation, but if you're doing rollback, you're most likely going to have those issues with this PR.

The comment mentions signals, but even if you're not using them, polling for overlapping areas/bodies will still fail to resimulate in certain scenarios. Other things like move_and_slide are also prone to produce different results on resimulation due to these limitations.

Once again, this is not to say that this PR isn't useful, or that it shouldn't be merged. Quite the contrary. It's just not sufficient for "full" rollback (i.e. being able to go back to a previous state and reproducing the exact same results in every scenario)

huh...
short of writing my own physics engine which I contemplated and deemed not going to happen, maybe someone will come up with a solution.

@monitz87
Copy link

Don't want to beat a dead horse here, but I strongly recommend that you read this comment if you haven't. I'm not sure about your specific netcode implementation, but if you're doing rollback, you're most likely going to have those issues with this PR.
The comment mentions signals, but even if you're not using them, polling for overlapping areas/bodies will still fail to resimulate in certain scenarios. Other things like move_and_slide are also prone to produce different results on resimulation due to these limitations.
Once again, this is not to say that this PR isn't useful, or that it shouldn't be merged. Quite the contrary. It's just not sufficient for "full" rollback (i.e. being able to go back to a previous state and reproducing the exact same results in every scenario)

huh... short of writing my own physics engine which I contemplated and deemed not going to happen, maybe someone will come up with a solution.

Well, if your game is 2D, you can use sg-physics-2d, a GDextension that implements an alternative "physics engine". It uses fixed point math under the hood and it's fully deterministic. I use quotation marks because it doesn't replace Godot physics, but rather offers its own set of nodes and a similar API for collision detection and movement.

You can check it out here if you're interested. The README does a good job of explaining how it works, what it can do and what it can't.

@hislittlecuzin
Copy link

Don't want to beat a dead horse here, but I strongly recommend that you read this comment if you haven't. I'm not sure about your specific netcode implementation, but if you're doing rollback, you're most likely going to have those issues with this PR.
The comment mentions signals, but even if you're not using them, polling for overlapping areas/bodies will still fail to resimulate in certain scenarios. Other things like move_and_slide are also prone to produce different results on resimulation due to these limitations.
Once again, this is not to say that this PR isn't useful, or that it shouldn't be merged. Quite the contrary. It's just not sufficient for "full" rollback (i.e. being able to go back to a previous state and reproducing the exact same results in every scenario)

huh... short of writing my own physics engine which I contemplated and deemed not going to happen, maybe someone will come up with a solution.

Well, if your game is 2D, you can use sg-physics-2d, a GDextension that implements an alternative "physics engine". It uses fixed point math under the hood and it's fully deterministic. I use quotation marks because it doesn't replace Godot physics, but rather offers its own set of nodes and a similar API for collision detection and movement.

You can check it out here if you're interested. The README does a good job of explaining how it works, what it can do and what it can't.

Unfortunately my project is 3D.

Looks like CharacterBody3D does not interface with PhysicsServer3D.BodyGetState(body, (PhysicsServer3D.BodyState)body_state);

I hope this thread has an answer or else I'm going to write some C++ haha.

@AThousandShips AThousandShips modified the milestones: 4.3, 4.4, 4.x Jul 25, 2024
@TheYellowArchitect
Copy link

TheYellowArchitect commented Aug 11, 2024

Current Status of this PR:

Rollback is mostly implemented, what remains is updating the code of signals to work with rollback

  • Events may not correctly trigger again when resimulating (i.e. things like area triggers may not work more than once). Untested. I'm sure I saw someone else point this out but I can't seem to find it so maybe it was found to not be the case.

Emit signals /events are implemented in CollisionObject nodes, it should be handled in another pr.

To provide a way to resimulate CollisionObject nodes, I think we can add two virtual method virtual CollisionStateXD CollisionObjectXD::get_state() const; and virtual void CollisionObjectXD::set_state(const Ref<CollisionStateXD>& p_state) const, and override them in child classes.

See godotengine/godot-proposals#2821 (comment) for details on how signals should work.

Also there are some spooky //TODO: comments which ask for clarity from someone experienced in PhysicsServer.

It is fair to say OP has neglected this pull request, and anyone can pick this up and hopefully finish it.

I am only bumping this to synopsize this PR's status and previous comments, because I am making an online game with rollback which requires physics rollback. And like me, every online action game developer who finishes rollback movement and is about to implement rollback physics, must pause development until a PR such as this merges :(

@Ughuuu
Copy link
Contributor

Ughuuu commented Aug 14, 2024

I'm not sure it's fair to say OP neglected the PR. He received multiple times feedback on PR, he was waited a very long time for a review by a "physics person" from godot. If they take their time he should be able to take his time too. If this was important to Godot team they would help more and review better/faster and offer more support.

It's still not clear what the scope of this PR is, even though the OP clearly wrote he wants to be able to step() the physics server, everyone talks about rollback, which is so out of the scope of this.

To me, even if OP doesn't neglect this PR, something that might speed up this PR getting approved would be a more focused approach to this. This PR wants to add step function, and manual stepping. It could be exposed as experimental, and if there are issues fixed after. However it seems Godot team wants to be extra careful, so thats why this might take a very long time (until every possible bug can be caught and fixed).

@grazianobolla
Copy link

Rollback is not that far away from this, depending on your definition of rollback (which there are many)... You save the world state and at any point you go back to it and then use this step() function to advance it again, that's rollback, and this PR is a must.

But yeah, we should stop talking about rollback and netcode when this implements something else (although related).

But I can't stress how important this is for multiplayer games, specially when making the jump from P2P games to more serious/competitive games, you NEED to be able to step the physics manually in some way or another, this would MASSIVELY improve what can be achieved with Godot regarding multiplayer games, and it is slowing down/deterring some people from utilizing the engine.

@Clonkex
Copy link

Clonkex commented Aug 14, 2024

It's still not clear what the scope of this PR is, even though the OP clearly wrote he wants to be able to step() the physics server, everyone talks about rollback, which is so out of the scope of this.

I beg to differ. This PR is very obviously intended to support rollback. I see the word "rollback" four times in the OP, and both of the linked proposal issues are also specifically discussing networking and rollback. While I do agree that this PR could already be merged if it had a narrower scope (of being able to manually step the physics engine, which is what the changes allow), that narrower scope results in a very niche feature that is not useful to most people. The primary use case for manually stepping the physics engine is for networking support, which we can't do unless we can also roll back the world state and have it step correctly.

@monitz87
Copy link

It's still not clear what the scope of this PR is, even though the OP clearly wrote he wants to be able to step() the physics server, everyone talks about rollback, which is so out of the scope of this.

I beg to differ. This PR is very obviously intended to support rollback. I see the word "rollback" four times in the OP, and both of the linked proposal issues are also specifically discussing networking and rollback. While I do agree that this PR could already be merged if it had a narrower scope (of being able to manually step the physics engine, which is what the changes allow), that narrower scope results in a very niche feature that is not useful to most people. The primary use case for manually stepping the physics engine is for networking support, which we can't do unless we can also roll back the world state and have it step correctly.

I have to disagree here. Full rollback support is a huge feature by itself, and there are independent use cases for both manually stepping the physics engine and for being able to load/save the internal state of the physics server. Furthermore, allowing saving/loading will probably require quite a bit of work on godot physics and possibly breaking changes in the physics server API (which would affect existing alternative physics engines and everyone who is using them). Considering the current workload of the core team and how much time it has taken for this to be reviewed, I personally think it's better to be able to merge at least part of the work to get the ball rolling instead of waiting for a full rollback implementation to be executed, reviewed, and merged.

@hudmarc
Copy link

hudmarc commented Aug 15, 2024

It's still not clear what the scope of this PR is, even though the OP clearly wrote he wants to be able to step() the physics server, everyone talks about rollback, which is so out of the scope of this.

I beg to differ. This PR is very obviously intended to support rollback. I see the word "rollback" four times in the OP, and both of the linked proposal issues are also specifically discussing networking and rollback. While I do agree that this PR could already be merged if it had a narrower scope (of being able to manually step the physics engine, which is what the changes allow), that narrower scope results in a very niche feature that is not useful to most people. The primary use case for manually stepping the physics engine is for networking support, which we can't do unless we can also roll back the world state and have it step correctly.

I have to disagree here. Full rollback support is a huge feature by itself, and there are independent use cases for both manually stepping the physics engine and for being able to load/save the internal state of the physics server. Furthermore, allowing saving/loading will probably require quite a bit of work on godot physics and possibly breaking changes in the physics server API (which would affect existing alternative physics engines and everyone who is using them). Considering the current workload of the core team and how much time it has taken for this to be reviewed, I personally think it's better to be able to merge at least part of the work to get the ball rolling instead of waiting for a full rollback implementation to be executed, reviewed, and merged.

To add onto this, if the blocker is mainly the rollback aspect, it makes sense to split this into two separate PR's. One to focus on adding the manual step() function (this PR) and the other to add a more sophisticated 'snapshot' system so the physics state can be easily rolled back to a previous point in time.

I come from a Unity background and as far as I know that engine only has a manual step() function without any way to truly roll back to previous states, and that's worked fine for the many networking libraries there, since they all manually implement their own circular buffers for keeping track of past states.

Of course a built-in way to get a snapshot of a particular previous physics world state would be nice but I imagine adding that feature and properly testing it would add a ton of overhead to this issue.

@Clonkex
Copy link

Clonkex commented Aug 15, 2024

To add onto this, if the blocker is mainly the rollback aspect, it makes sense to split this into two separate PR's. One to focus on adding the manual step() function (this PR) and the other to add a more sophisticated 'snapshot' system so the physics state can be easily rolled back to a previous point in time.

I'm definitely not advocating for a built-in snapshot buffer or automatic rollback system. That would be drastically more complex and way out of scope. My view is that this PR should solve the problem of events not firing correctly in the case that a user builds their own rollback system and attempts to resimulate the world after setting its state back to a previous snapshot. Otherwise, there will be subtle and extremely confusing bugs for anyone attempting to make use of the physics stepping feature without prior knowledge of this Github issue.

The point is, we should be able to update transforms and velocities on rigidbodies at any point and then step the physics engine and get a correct result, regardless of what happened the last time the engine was stepped. How would this work internally (for instance, a body is inside a trigger volume on frame 5, then is moved back outside the volume to its position as it was on frame 3, but on the next step it enters the volume again and now the engine doesn't know it should trigger the event)? No clue. I don't know if it's feasible and I'm not sure how other engines handle it. Maybe it would be possible for the user to always roll back 1 frame further than intended and step again from there... but that has its own potential issues 😕 Maybe it would be easiest to provide a way to retrieve a snapshot of all internal physics engine state for the user to store however they think best, then apply that to revert the world state.

IMO networking is already difficult enough without merging a known-broken PR and letting people figure out that it's broken for themselves, which is why I'm advocating for figuring out the known issues before merging.

@grazianobolla
Copy link

grazianobolla commented Aug 15, 2024

We need to get all aligned then: This PR only implements a method to step the physics forward (manually), with all that comes with that like triggering events.

Let's start testing it and see if we can figure out how to resimulate events/triggers/signals so this PR is working correctly and can be merged.

@npinsker
Copy link

npinsker commented Oct 3, 2024

Need to rebase on top of 7c4c4b9, just need to update physics_server_2d_dummy.h etc with dummy methods --

virtual void space_step(RID p_space, real_t p_delta) override {}
virtual void space_flush_queries(RID p_space) override {}
[...]
virtual int space_get_last_process_info(RID p_space, ProcessInfo p_info) override { return 0; }

@Daylily-Zeleen
Copy link
Contributor Author

@npinsker this pr is deferred from 4.1 to 4.3 and 4.x at last.
To be honest, I don't want to make further changes to this PR unless they have a genuine intention to merge this pr (because it should have been merged and maintained along with other changes a long time ago).

@npinsker
Copy link

npinsker commented Oct 3, 2024

Completely fair (and I agree). I'm mostly leaving the comment for future possible users since I'm building on top of this PR in my own project

@Ughuuu
Copy link
Contributor

Ughuuu commented Oct 3, 2024

For anyone looking at this PR, know that space_step is not going to be enough, as others said. It is a start, but there also needs to be state serialization (for rollback). And that in itself will be a hard challenge, because RID's, which are used a lot in code, are random. (I know because I implemented this in Godot Rapier Physics plugin, total plug but related to this, so if anyone wants to give it a try there, they can. This was possible since Rust language has serialization for free, unlike C++)

I believe blocking this PR was not the best move, as @Daylily-Zeleen said, but rather making multiple smaller commits as things develop (instead of waiting for everything to be ready). Also, the community really wants this, as you can see by the support on the PR, but the godot team isn't as interested to offer much help, sadly.

Copy link
Contributor

@RadiantUwU RadiantUwU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, has been thoroughly tested and been working for months in my custom fork of godot.

@@ -935,13 +935,28 @@
Creates a 2D space in the physics server, and returns the [RID] that identifies it. A space contains bodies and areas, and controls the stepping of the physics simulation of the objects in it.
</description>
</method>
<method name="space_flush_queries" is_experimental="true">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need these to be experimental?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has not been extensively tested, and I would rather mark all added APIs as experimental to reserve the right to break the compatibility by revoking.

@Brzusko
Copy link

Brzusko commented Oct 3, 2024

@npinsker this pr is deferred from 4.1 to 4.3 and 4.x at last. To be honest, I don't want to make further changes to this PR unless they have a genuine intention to merge this pr (because it should have been merged and maintained along with other changes a long time ago).

I hope that PR will be merged soon, a lot of people are waiting for review from maintainers. Good work my friend.

@scgm0
Copy link
Contributor

scgm0 commented Oct 3, 2024

So what is the reason that hinders this PR merger?

@RadiantUwU
Copy link
Contributor

So what is the reason that hinders this PR merger?

They want to hire someone to work on physics first so they review it.

@scgm0
Copy link
Contributor

scgm0 commented Oct 3, 2024

So what is the reason that hinders this PR merger?

They want to hire someone to work on physics first so they review it.

Haven't hired someone yet to take over the physics part? Didn't the foundation say a few days ago that it was not short of money? . .

@RadiantUwU
Copy link
Contributor

Haven't hired someone yet to take over the physics part? Didn't the foundation say a few days ago that it was not short of money? . .

I dont even know who the lead of physics is, i'll ask akien whether they know anything about when this PR is gonna be reviewed.

@grazianobolla
Copy link

This is currently the most discussed PR on the physics topic, why we haven't heard from anyone with the authority to review it? This just implements a function to step the engine physics, it has been tested individually by many people over the last almost 2 years! It is a simple intervention...

@Ughuuu
Copy link
Contributor

Ughuuu commented Oct 28, 2024

Just so it's clear, if you really want this change, you can most definitely build yourself a custom engine and try it out, or use an addon that might expose this (shameless plug of godot-rapier that exposes this).
This isn't reviewed as there isn't a physics lead maintainer at the moment, as it was said above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.