Add solver_priority to Jolt constraint priority#119976
Conversation
jrouwe
left a comment
There was a problem hiding this comment.
Note that according to the documentation, Jolt's priority works opposite to Godot's priority. Godot:
Vs Jolt:
/// Priority of the constraint when solving. Higher numbers are more likely to be solved correctly.
Looking at the code for GodotPhysics I think the tooltip doesn't describe what actually happens:
godot/modules/godot_physics_3d/godot_step_3d.cpp
Lines 136 to 158 in 417cd33
If I read this code then it will first call solve() iterations times on each constraint in the current island. Then it will loop and remove all constraints with priority 1. After that it will call solve() iterations times on the remaining constraints. Then it will loop and remove all constraints with priority 2. etc. This happens until there are no constraints left. So if you set the constraint priority to 1000, then that constraint will actually be solved 1000 * iterations times, which will be insanely slow. Note that the UI limits the max value to 8, but I think you can set it to any value if you script it. Waht this boils down to is that joints with higher priorities will be solved more accurately.
Jolt on the other hand uses the priority only to sort the constraints, so all constraints will be solved iterations times, but the highest priority joints are solved last which means that they will be solved more accurately (the first constraints can be violated when solving the last constraints).
I think the tool tip needs to be adjusted to:
- Reflect that higher values have a higher priority.
- Remove the note that Jolt doesn't support it.
As you can pass an int to the set_solver_priority function and it is cast to an unsigned integer, I think the value set should be max(0, value).
Wow... so basically if someone assumes the sorting strategy (which is how the concept of "priority" should be handled), and increments their joints as they create them, switching to Godot physics would probably instantly crash any project with more than 10 constraints.
Yes, I agree. I forgot to update the documentation.
Initially when I was using priorities, I had the idea to start at I saw that Jolt runs with uints, and I didn't confirm it, but I was running with "negative" priorities assuming the cast will underflow. It probably wasn't though. Whatever way, the doc should mention what happens with negative values, as normal ints in GDScript are always signed. |
2209ce8 to
3f71b7d
Compare
- Jolt does have solver priorities, now you can set them
3f71b7d to
c52897b
Compare
What problem(s) does this PR solve?
Additional information
Jolt has constraint priorities which fulfills the purpose of ordering joint constraints in the step process. Right now Godot prints an error and claims that
solver_priorityis not supported in Jolt, but it is!