Fix five multiplayer sync bugs, add automated two-instance test
Adds test/multiPlayerTest.tscn plus a driver that runs the kitchen flow across two game instances: grab/drop, dirt station, sink washing, hob cooking, counter combining and plating. Runs headless (run_mp_test.ps1), in two visible windows (run_mp_test_windowed.ps1), or by hand with keyboard controls (play_mp_test.ps1). 108 checks, exits non-zero on failure. After every step both peers snapshot every item's position and rendered state and the server diffs them. Targeted assertions only look at the thing a step touched, which misses desyncs elsewhere - that audit is what caught the last bug below. Bugs found and fixed: - net_pickable: apply_held_state() only wrote `enabled` in its non-authority branch, so once a client grabbed an item every other peer set enabled=false and regaining authority never restored it. The server could then never pick that item up again, and a station would "snap" it (emitting has_picked_up, so a plate still got marked dirty) while pick_up() bailed out on the disabled item - leaving the zone holding an item with no grab driver. - network_manager: station gating only happened in the spawn path, so stations baked into a scene file kept running their snap zones on clients and grabbed items straight out of the local hand. Added gate_existing_stations(). - network_manager: despawn_item() only freed the server's copy. Items baked into a scene aren't tracked by the MultiplayerSpawner, so consuming one left a ghost on every client, which then blocked the station it sat in and got grabbed instead of its replacement. - network_manager: the snap-into-station decision read the server's own copy of the item position, but the reliable release RPC routinely overtakes the synchronizer's unordered position updates - so it acted on a stale position and teleported items back into the station they had just been carried away from. The releasing peer now sends its final transform and the server adopts it first. - container: contained_ids.append()/erase() mutate the array in place, which never fires the setter that rebuilds the plate's visuals. The peer that put food on a plate was the only peer that never redrew it; remote peers looked right because the synchronizer assigns there. Also null-guards XRServer.get_tracker() in the vendored xr-tools hand grab point, which threw on every successful grab without an XR runtime, and adds multiplayer_world.populate_from_layout so debug scenes can bake their own content instead of spawning the whole kitchen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+25
-1
@@ -20,6 +20,10 @@ var _pickable: XRToolsPickable
|
||||
# apply_held_state() last forced it to while non-authority.
|
||||
var _original_freeze_mode: int
|
||||
|
||||
# Same idea for the pickable's authored `enabled` flag, which the non-authority
|
||||
# branch of apply_held_state() clears while someone else is holding the item.
|
||||
var _original_enabled: bool
|
||||
|
||||
|
||||
func _ready() -> void:
|
||||
_pickable = get_parent() as XRToolsPickable
|
||||
@@ -27,6 +31,7 @@ func _ready() -> void:
|
||||
push_error("NetPickable must be a child of an XRToolsPickable")
|
||||
return
|
||||
_original_freeze_mode = _pickable.freeze_mode
|
||||
_original_enabled = _pickable.enabled
|
||||
_pickable.picked_up.connect(_on_picked_up)
|
||||
_pickable.dropped.connect(_on_dropped)
|
||||
# Deferred: the pickable root captures its own original_collision_mask/
|
||||
@@ -72,6 +77,22 @@ func apply_held_state() -> void:
|
||||
)
|
||||
_pickable.freeze_mode = _original_freeze_mode
|
||||
_pickable.collision_mask = _pickable.original_collision_mask
|
||||
# Unlike freeze/collision (which XRToolsPickable manages itself while
|
||||
# held), `enabled` is only ever written by the non-authority branch
|
||||
# below, so it must be restored here or it stays false forever: once a
|
||||
# client grabbed this item, every other peer set enabled=false, and
|
||||
# regaining authority left it that way. On the server that silently
|
||||
# broke everything downstream — hands couldn't pick the item up again,
|
||||
# and a station snap zone would "snap" it (emitting has_picked_up, so
|
||||
# e.g. a plate still got marked dirty) while pick_up() bailed out on
|
||||
# the disabled item, leaving the zone holding an item with no grab
|
||||
# driver that then fell out of the station.
|
||||
if _pickable.enabled != _original_enabled:
|
||||
if NetworkManager.is_online():
|
||||
print("%s: reclaiming ownership, restoring enabled %s->%s" % [
|
||||
_pickable.name, _pickable.enabled, _original_enabled
|
||||
])
|
||||
_pickable.enabled = _original_enabled
|
||||
return
|
||||
# A net_held_by/position sync update can race ahead of the
|
||||
# authority-handoff RPC that's about to confirm a grab we just made
|
||||
@@ -132,8 +153,11 @@ func _on_dropped(_p) -> void:
|
||||
print("%s dropped, reporting release to server (lin=%s ang=%s)" % [
|
||||
_pickable.name, _pickable.linear_velocity, _pickable.angular_velocity
|
||||
])
|
||||
# Send our own final transform too: we were the authority until now, and the
|
||||
# server's copy may not have received our last position sync yet.
|
||||
NetworkManager.release_item_authority_from(
|
||||
_pickable.get_path(), _pickable.linear_velocity, _pickable.angular_velocity
|
||||
_pickable.get_path(), _pickable.linear_velocity, _pickable.angular_velocity,
|
||||
_pickable.global_transform
|
||||
)
|
||||
|
||||
|
||||
|
||||
+52
-8
@@ -121,8 +121,31 @@ func spawn_item(scene_path: String, xform: Transform3D, node_name: String = "",
|
||||
## every peer when a tracked node exits the tree on the authority, so this is
|
||||
## the single seam for destroying spawned items (works offline too).
|
||||
func despawn_item(node: Node) -> void:
|
||||
if owns_world() and is_instance_valid(node):
|
||||
log_line("despawn_item: %s" % node.name)
|
||||
if not owns_world() or not is_instance_valid(node):
|
||||
return
|
||||
log_line("despawn_item: %s" % node.name)
|
||||
# Items that came from the ItemsSpawner are despawned on every peer
|
||||
# automatically when they leave the tree here. Items baked into a scene file
|
||||
# are unknown to the spawner, so their removal has to be broadcast
|
||||
# explicitly — otherwise every client keeps a ghost copy of an item the
|
||||
# server has consumed, which then blocks the station it was sitting in and
|
||||
# gets grabbed instead of the real item that replaced it.
|
||||
if is_online() and not _is_spawner_tracked(node):
|
||||
_despawn_static_item.rpc(node.get_path())
|
||||
node.queue_free()
|
||||
|
||||
|
||||
# Items the ItemsSpawner replicates live under its spawn path; anything else was
|
||||
# baked into the scene file and the spawner knows nothing about it.
|
||||
func _is_spawner_tracked(node: Node) -> bool:
|
||||
return _content_root != null and _content_root.is_ancestor_of(node)
|
||||
|
||||
|
||||
@rpc("authority", "call_remote", "reliable")
|
||||
func _despawn_static_item(path: NodePath) -> void:
|
||||
var node := get_node_or_null(path)
|
||||
if node:
|
||||
log_line("despawn_static_item: freeing %s (the server consumed it)" % node.name)
|
||||
node.queue_free()
|
||||
|
||||
|
||||
@@ -163,6 +186,19 @@ func _gate_station(node: Node) -> void:
|
||||
log_line("gated station (non-owner peer): %s" % node.name)
|
||||
|
||||
|
||||
## Gate every station already sitting in the scene tree, for peers that don't
|
||||
## own world logic. Stations that arrive through spawn_item() are gated as they
|
||||
## are built (see _spawn_item_from_data), but ones baked into a scene file never
|
||||
## pass through there — leaving a client running its own snap zones, which then
|
||||
## grab items straight out of the local hand and fight the server's
|
||||
## authoritative placement. Idempotent, so it's safe on every session start.
|
||||
func gate_existing_stations() -> void:
|
||||
if owns_world():
|
||||
return
|
||||
for station in get_tree().get_nodes_in_group("station"):
|
||||
_gate_station(station)
|
||||
|
||||
|
||||
# --- Item grab-authority transfer -----------------------------------------
|
||||
|
||||
## Called by NetPickable when this peer grabs an item by hand. Godot rejects
|
||||
@@ -209,27 +245,35 @@ func _grant_or_reject_item_authority(item_path: NodePath, sender: int) -> void:
|
||||
## Called by NetPickable when this peer releases an item, forwarding its throw
|
||||
## velocity so the server can resume simulating it. Same self-RPC issue as
|
||||
## above: runs directly if we're the server.
|
||||
func release_item_authority_from(item_path: NodePath, lin: Vector3, ang: Vector3) -> void:
|
||||
func release_item_authority_from(item_path: NodePath, lin: Vector3, ang: Vector3, xform: Transform3D) -> void:
|
||||
if is_server():
|
||||
_do_release_item_authority(item_path, lin, ang, multiplayer.get_unique_id())
|
||||
_do_release_item_authority(item_path, lin, ang, xform, multiplayer.get_unique_id())
|
||||
else:
|
||||
_release_item_authority_rpc.rpc_id(1, item_path, lin, ang)
|
||||
_release_item_authority_rpc.rpc_id(1, item_path, lin, ang, xform)
|
||||
|
||||
|
||||
@rpc("any_peer", "reliable")
|
||||
func _release_item_authority_rpc(item_path: NodePath, lin: Vector3, ang: Vector3) -> void:
|
||||
func _release_item_authority_rpc(item_path: NodePath, lin: Vector3, ang: Vector3, xform: Transform3D) -> void:
|
||||
if not is_server():
|
||||
return
|
||||
_do_release_item_authority(item_path, lin, ang, multiplayer.get_remote_sender_id())
|
||||
_do_release_item_authority(item_path, lin, ang, xform, multiplayer.get_remote_sender_id())
|
||||
|
||||
|
||||
## Runs on the server. If released next to a station, the server snaps it in
|
||||
## (server-authoritative placement).
|
||||
func _do_release_item_authority(item_path: NodePath, lin: Vector3, ang: Vector3, sender: int) -> void:
|
||||
func _do_release_item_authority(item_path: NodePath, lin: Vector3, ang: Vector3, xform: Transform3D, sender: int) -> void:
|
||||
log_line("release_item_authority: %s released by peer %d" % [str(item_path), sender])
|
||||
_set_item_authority.rpc(item_path, 1)
|
||||
var item := get_node_or_null(item_path)
|
||||
if item is RigidBody3D:
|
||||
# Adopt the releasing peer's own final transform rather than trusting our
|
||||
# copy's. That peer was the item's authority right up to this moment, and
|
||||
# its position updates travel on the synchronizer's separate, unordered
|
||||
# channel — this reliable RPC routinely overtakes them, leaving our copy
|
||||
# still sitting where the item was BEFORE the peer carried it away. The
|
||||
# snap decision below then reads that stale position and teleports the
|
||||
# item straight back into the station it was just picked up from.
|
||||
item.global_transform = xform
|
||||
item.freeze = false
|
||||
item.linear_velocity = lin
|
||||
item.angular_velocity = ang
|
||||
|
||||
Reference in New Issue
Block a user