Skip to content

fix: Add per-tick callback, fix Rapier rollback index, and physics server state access - #640

Open
realpoke wants to merge 6 commits into
foxssake:mainfrom
realpoke:physics-fixes
Open

fix: Add per-tick callback, fix Rapier rollback index, and physics server state access#640
realpoke wants to merge 6 commits into
foxssake:mainfrom
realpoke:physics-fixes

Conversation

@realpoke

@realpoke realpoke commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Hey! I ran into a few edge cases while testing my project with Rapier at low network tick rates and a 1-2 physics factor. I tracked the jitter, broken physics and crashes down to a few issues.

This PR refactors NetworkRigidBody rollback stepping, fixes state retrieval bugs in the Rapier physics drivers, and improves NetworkTime clock stretch precision.

1. Per-tick callback on NetworkRigidBody (feat)

  • _physics_rollback_tick() runs once per sub-step (physics_factor times per network tick), which is required for continuous forces (apply_central_force) since Godot resets forces every step.
  • Adds _before_physics_rollback_tick(delta, tick) which runs once per network tick with the full tick delta. Use this for one-shot logic (impulses, state transitions, input consumption) to prevent impulses from running physics_factor times.
  • Updates physics guide documentation.

2. Rapier driver rollback state fix (fix)

  • StateManager::push_state_to_cache stores states oldest-first (index 0 is oldest). Derived offset math previously retrieved the wrong cached tick during rollback.
  • Replaces index calculation with tag lookup via _state.ordered_cache_tags().rfind(tick).

3. Physics server state access (refactor)

  • Refactors NetworkRigidBody2D/3D get_state() and set_state() to query PhysicsServer2D/PhysicsServer3D directly via RID.
  • Changes direct_state from a cached @onready variable to a computed property (get: return PhysicsServer...) so it cannot go stale if a node is freed.
  • Restores BODY_STATE_SLEEPING last during set_state() so transform/velocity updates don't wake the body.

4. Log-space clock stretch interpolation (fix)

  • Interpolates _clock_stretch_factor in log space (pow(clock_stretch_max, t)). Linear interpolation resulted in 1.025 stretch factor at zero clock difference, causing the clock to park ahead of reference and introduce jitter at lower tickrates.
  • Adds Vest unit test suite for NetworkTime.

5. NetworkSchemas test suite fixes (fix)

  • Fixes vec2f16 test invoking vec2f32().
  • Fixes unnormalized axis in Transform3D.rotated() in 3D transform tests.
  • Adds tolerance comparison for lossy half-precision float roundtrips on Godot 4.4+.

6. Clean up dead code (chore)

  • Removes unused _get_earliest_for() in NetworkHistoryServer.

Tests

  • Tested against Godot 4.7.1 and godot-rapier 0.8.40.
  • Ran test suite via sh/test.sh: 325 passed, 0 failed.

Let me know if anything needs adjustments. Thanks!

@albertok

albertok commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

PhysicsDriver: Moved _physics_rollback_tick() outside the physics_factor sub-stepping loop. It now properly executes once per tick using the full delta rather than executing multiple times per sub-step.

This would break sub stepping. If the game tick rate is running at 30htz and you need physics to run at 60htz this is no longer possible.

See Physics Factor in https://foxssake.github.io/netfox/latest/netfox.extras/guides/physics/

NetworkRigidBody3D: Added null checks for direct_state to prevent null dereference errors when accessed outside direct state integration. Also ensured we properly restore the historical sleeping state rather than forcing the body awake unconditionally.

I believe the root cause here was recently fixed in #633 and we now deregister when the node is gone.

NetworkTime: Fixed the clock stretch interpolation so that a clock_diff of zero correctly evaluates to a neutral stretch factor of 1.0.

This looks ok, some unit tests proving it with scenarios would be awesome.

NetworkHistoryServer: Fixed a small typo in _get_earliest_for where earliest was assigning to itself instead of subject_latest.

I think this is dead code, we don't actually use it ...

realpoke added 6 commits August 8, 2026 12:31
@realpoke
fix: Neutral clock stretch factor at zero clock difference
cc5e40c 
`_clock_stretch_factor` is calculated by lerping linearly between
`1.0 / clock_stretch_max` and `clock_stretch_max`. Stretch factors are speed
multipliers, so they compose geometrically - the neutral point is the geometric
mean, not the arithmetic one.

With the default `clock_stretch_max` of 1.25, a `clock_diff` of zero yields
`lerpf(0.8, 1.25, 0.5)` = 1.025, so the simulation clock runs 2.5% fast even
when perfectly in sync.

The controller is still stable, but it settles at a nonzero error: the clock
parks permanently ahead of the reference clock by roughly `-0.111 * ticktime`.
That scales with tick time, so it is negligible at high tick rates and grows
noticeable at low ones:

| Tickrate | Steady-state clock offset |
| --- | --- |
| 60 Hz | ~1.9 ms |
| 30 Hz | ~3.7 ms |
| 10 Hz | ~11.1 ms |

At low tick rates that constant offset is large enough to intermittently tip
`_get_ticks_in_loop()` over into running an extra tick or skipping one, which
shows up as jitter.

Interpolate in log space instead: `pow(clock_stretch_max, t)` where `t` is the
clock difference in ticks, clamped to `[-1, 1]`.

- `t = 0` gives exactly `1.0`
- `t = 1` gives `clock_stretch_max`
- `t = -1` gives `1.0 / clock_stretch_max`

Bounds and the "reach max at one tick of difference" behaviour are unchanged.
Speeding up and slowing down by the same amount are now exact inverses, which
was the property the old code violated.

The calculation is extracted into a pure static method so it can be tested
without a running clock. Added a Vest suite covering the neutral point, both
bounds, clamping, symmetry, and direction.
@realpoke
fix: Rapier drivers restore the wrong physics state on rollback
b871d23 
Two bugs in `_rollback_space()` compound.

1. `_stored_states` is never incremented. It starts at 0 and is only
incremented after the guard that reads it, which always returns first (`offset`
is always >= 1 during resimulation). `load_cached_state()` is therefore never
called - with the Rapier drivers, `rollback_physics_space` silently does nothing,
while rollback still re-steps the space forward.

2. The index assumes the cache is newest-first. It isn't. StateManager's
`push_state_to_cache` appends and evicts from the front, so index 0 is the
oldest cached state. Verified against godot-rapier 0.8.40 on Godot 4.7.1:

    ordered_cache_tags()         -> [100, 101, 102, 103, 104]
    load_cached_state(space, 0)  -> restores tick 100  (oldest)

Fixing (1) alone would be worse than the old behaviour: it would activate
`load_cached_state()` with an index derived from the wrong ordering, restoring an
unrelated tick on every rollback. Measured with a cache of 8 and ticks 0-19
snapshotted, every lookup lands on the wrong state:

    want tick 19 -> index 1 -> got 13
    want tick 17 -> index 3 -> got 15
    want tick 14 -> index 6 -> got 18

Drop `_stored_states` and look the tick up by its cache tag. netfox already
passes the tick as the tag to `cache_state()`, and `ordered_cache_tags()` exposes
them in cache order.

`rfind()` rather than `find()`, because a tick is cached more than once while
history is rewritten during rollback and the most recent entry is the one we
want. A negative result means the tick has aged out, and rollback is correctly
skipped.
@realpoke
feat: Add per-tick callback to NetworkRigidBody
9519f80 
`_physics_rollback_tick()` is called once per physics sub-step but handed a
network tick number. With `physics_factor > 1` it runs multiple times for the
same tick, which is correct for continuous forces (cleared after each step)
and wrong for one-off impulses/input consumption (applied multiple times).

Measured with godot-rapier 0.8.40 on Godot 4.7.1, `physics_factor = 2`, 30 Hz
tick, 1 kg body, no gravity:

| | applied once per tick | applied per sub-step |
| --- | --- | --- |
| `apply_central_force(10 N)` | vx = 0.16611 | vx = 0.33250 |
| `apply_central_impulse(10)` | vx = 9.96675 | vx = 19.95011 |

Add `_before_physics_rollback_tick(delta, tick)`, called once per network tick
with the full tick delta, before any sub-step. `_physics_rollback_tick()` keeps
its per-sub-step behaviour, preserving backward compatibility.
@realpoke
refactor: Read and write NetworkRigidBody state through the physics s…
c0a0300 
…erver

`get_state()` / `set_state()` go through `body_get_state()` / `body_set_state()`
on the RID rather than a handle cached at `_ready`. These are the same server
calls the direct state object forwards to, and match the pattern `godot_driver_3d.gd`
uses for snapshotting.

Benefits:
- No cached handle to go stale after a body is freed
- On a freed RID, `body_get_state()` returns nil rather than erroring
- Verified working with Rapier 0.8.40

`direct_state` is kept as a computed property rather than removed, so existing
user code still works - it is fetched on access now instead of cached.

Also makes sleeping state restore explicitly last, since setting transform and
velocities wakes the body, and fixes the 2D placeholder array which was seeded
with 3D types.
@realpoke
chore: Remove unused _get_earliest_for()
63f0ddf 
`NetworkHistoryServer._get_earliest_for()` has no callers, and contains a typo
that makes it always return -1 regardless of input:

    if earliest < 0:
        earliest = earliest   # never leaves -1

Since nothing uses it, removing it is cleaner than maintaining dead code.
@realpoke
fix: NetworkSchemas test failures on Godot 4.4+
27fd7cc 
The `NetworkSchemas` suite fails on Godot 4.4 and up due to three issues in
`network-schemas.test.gd`:

1. `vec2f16` case called `NetworkSchemas.vec2f32()`. Below 4.4 the expected
   size matched f32 due to the f32 fallback; on 4.4+ it expected 4 bytes while
   f32 wrote 8.
2. `normal2f16`, `quat16f` and `transform2f16` carry ~11 bits of mantissa and
   don't round-trip bit-identically. Exact `expect_equal` fails when real half-floats
   are active.
3. `Transform3D.rotated()` was called with an unnormalized axis `Vector3.ONE`,
   which caused Godot to print an error and return identity (testing identity
   components 0/1 instead of real rotation).

Fixes:
- Point `vec2f16` case at `NetworkSchemas.vec2f16()`.
- Normalize the rotation axis in `transform3f*` cases.
- Add optional per-case float component tolerance and apply `HALF_EPSILON` to
  half-precision cases.
- Fix format string placeholder count in size mismatch assertion.
@realpoke

realpoke commented Aug 8, 2026

Copy link
Copy Markdown
Author

Thanks for the feedback! You were spot on regarding continuous forces vs impulses, continuous forces apply_central_force must run per physics sub-step because Godot clears force accumulators after each step,
and one-shot logic apply_central_impulse/input consumption running per sub-step gets multiplied by physics_factor.
I've reworked the branch and replaced the previous commits with clean, distinct commits, and I will update the original PR.

@realpoke realpoke changed the title fix: Simulation edge cases with low network ticks and physics factors fix: Add per-tick callback, fix Rapier rollback index, and physics server state access Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@realpoke @albertok