Merge branch 'multi2' of https://git.offcoursegames.com/JonShard/VRyHungry1 into multi2

Containers/container.gd

@@ -141,6 +141,9 @@ func refresh_visuals(ids: Array[String]) -> void:
 
 		_make_cosmetic(visual)
 		container_root.add_child(visual)
+		# Must happen after add_child: entering the world is what puts the body
+		# into the physics space, so it can only be taken out again afterwards.
+		_remove_from_physics(visual)
 		visual.position = positions[idx].position
 		visual.rotation = positions[idx].rotation
 
@@ -152,10 +155,18 @@ func refresh_visuals(ids: Array[String]) -> void:
 func _make_cosmetic(visual: Node3D) -> void:
 	var net_pickable := visual.get_node_or_null("NetPickable")
 	if net_pickable:
-		net_pickable.queue_free()
+		# Detach and free it outright rather than queue_free(): this runs before
+		# `visual` is added to the tree, and a merely-queued node still enters the
+		# tree with its parent and runs _ready() (which starts syncing and logging)
+		# before the queued deletion lands at the end of the frame.
+		visual.remove_child(net_pickable)
+		net_pickable.free()
 	if visual is RigidBody3D:
 		visual.freeze = true
-		visual.freeze_mode = RigidBody3D.FREEZE_MODE_KINEMATIC
+		# STATIC, not KINEMATIC: a kinematic body is still driven by the physics
+		# engine (see _remove_from_physics), and "static decoration" is what this
+		# actually is.
+		visual.freeze_mode = RigidBody3D.FREEZE_MODE_STATIC
 		visual.collision_layer = 0
 		visual.collision_mask = 0
 	if visual is XRToolsPickable:
@@ -164,6 +175,20 @@ func _make_cosmetic(visual: Node3D) -> void:
 	visual.set_physics_process(false)
 
 
+# Take a display-only copy out of the physics simulation completely.
+#
+# Freezing is not enough. Under Jolt (this project's physics engine) a frozen
+# KINEMATIC RigidBody3D is still simulated: it is driven toward its target
+# transform by velocity rather than being teleported. Parented to a plate that
+# gets picked up and carried, it therefore lags behind, keeps its velocity, and
+# overshoots — so the food visibly slid off the plate and ended up metres away,
+# differently on each peer since each simulates its own copy. A body with no
+# space is never touched by the engine, so it simply follows its parent.
+func _remove_from_physics(visual: Node3D) -> void:
+	if visual is RigidBody3D:
+		PhysicsServer3D.body_set_space((visual as RigidBody3D).get_rid(), RID())
+
+
 #plate (Pickalbe)
 	#XRGrapPoints
 	#container (script) (meal positions[1], side positions[4])

Containers/plate_controller.gd

@@ -30,6 +30,13 @@ func _process(_delta: float) -> void:
 
 
 func _set_contained_ids(value: Array[String]) -> void:
+	# Only rebuild when the contents actually changed. This property is
+	# replicated in ALWAYS mode, so the synchronizer assigns it every network
+	# tick on every peer that doesn't own the plate — and refresh_visuals()
+	# frees and re-instantiates a scene per item each time. That was thousands
+	# of throwaway nodes per run (and a log line from each one's NetPickable).
+	if contained_ids == value:
+		return
 	contained_ids = value
 	# Deferred: this can be written by the replicated spawn payload before
 	# this node's own @onready vars (container) have resolved.

Net/net_pickable.gd

@@ -24,6 +24,11 @@ var _original_freeze_mode: int
 # branch of apply_held_state() clears while someone else is holding the item.
 var _original_enabled: bool
 
+# Whether the "our own hand still holds this" guard has already been logged for
+# the current grab. apply_held_state() runs every network tick, so without this
+# the guard message repeats for as long as you hold the item.
+var _grab_race_logged := false
+
 
 func _ready() -> void:
 	_pickable = get_parent() as XRToolsPickable
@@ -55,10 +60,19 @@ func _set_net_held_by(value: int) -> void:
 ## Puts the item in the right physics state for whether this peer currently
 ## owns it. Called locally after net_held_by changes, and directly by
 ## NetworkManager._set_item_authority right after an authority handoff.
+##
+## IMPORTANT: this runs on every network tick, not just on a real change.
+## net_held_by is replicated in ALWAYS mode, so the synchronizer assigns it every
+## tick on non-authority peers — unchanged value included — and that assignment
+## lands in _set_net_held_by(), which calls this. So every branch here has to be
+## idempotent and silent when there is nothing to do: otherwise each item logs a
+## line and rewrites four physics properties every tick on every peer that
+## doesn't own it.
 func apply_held_state() -> void:
 	if not _pickable:
 		return
 	if not NetworkManager.is_online() or is_multiplayer_authority():
+		_grab_race_logged = false
 		# We own this item's simulation (offline, loose+server, or currently
 		# holding it). If it's not actively in our own hand right now, make
 		# sure it isn't still left frozen/collision-less from a previous
@@ -68,7 +82,8 @@ func apply_held_state() -> void:
 		if not _pickable.is_picked_up():
 			var changed := _pickable.freeze_mode != _original_freeze_mode \
 				or _pickable.collision_mask != _pickable.original_collision_mask
-			if changed and NetworkManager.is_online():
+			if changed:
+				if NetworkManager.is_online():
 					print(
 						"%s: reclaiming ownership, restoring freeze_mode %d->%d collision_mask %d->%d" % [
 							_pickable.name, _pickable.freeze_mode, _original_freeze_mode,
@@ -101,13 +116,16 @@ func apply_held_state() -> void:
 	# hand mid-grab — only an explicit force_release_item rejection, or
 	# actually losing authority for real, should end a grab we initiated.
 	if _pickable.is_picked_up() and _pickable.get_picked_up_by() is XRToolsFunctionPickup:
-		if NetworkManager.is_online():
+		# Log once per grab, not once per tick.
+		if NetworkManager.is_online() and not _grab_race_logged:
+			_grab_race_logged = true
 			print(
 				"%s: ignoring non-authority sync (net_held_by=%d) — still actively held by our own hand (grab-race guard)" % [
 					_pickable.name, net_held_by
 				]
 			)
 		return
+	_grab_race_logged = false
 	# Someone else owns it: stop simulating locally, just follow the sync.
 	if _pickable.is_picked_up():
 		if NetworkManager.is_online():
@@ -117,12 +135,24 @@ func apply_held_state() -> void:
 				]
 			)
 		_pickable.drop()
+	# Bail out when we're already in the follow-the-sync state. Without this the
+	# writes below (and the line logged with them) repeated every tick for every
+	# item on every non-authority peer — 90% of the log, plus four redundant
+	# physics-property writes per item per tick. The comparison also means we
+	# still re-apply if something else perturbs the state (e.g. let_go()
+	# restoring the collision mask after a force-drop).
+	var want_enabled := (net_held_by == 0)
+	if _pickable.freeze \
+			and _pickable.freeze_mode == RigidBody3D.FREEZE_MODE_KINEMATIC \
+			and _pickable.collision_mask == 0 \
+			and _pickable.enabled == want_enabled:
+		return
 	if NetworkManager.is_online():
 		print("%s: freezing (non-authority, owner=peer %d)" % [_pickable.name, net_held_by])
 	_pickable.freeze = true
 	_pickable.freeze_mode = RigidBody3D.FREEZE_MODE_KINEMATIC
 	_pickable.collision_mask = 0
-	_pickable.enabled = (net_held_by == 0)
+	_pickable.enabled = want_enabled
 
 
 ## Local hand grab (not a station snap zone, which is server-only): request

recipes.yaml.uid

@@ -1 +0,0 @@
-uid://dqqu56yetl8ok

test/mp_test_driver.gd

@@ -254,7 +254,11 @@ func _cook_and_plate_round(actor: String, burger: String, buns: String, plate: S
 	# Take the finished plate away again, the way a player would carry it off to
 	# be served. Without this the counter stays occupied and the next round has
 	# nowhere to put its plate.
+	await _both("%s_food_on_plate_before_lift" % actor, "verify_plate_visuals", [plate])
 	await _step(actor, "%s_plate_off_counter2" % actor, "park", [plate, park_at])
+	# The food must still be on the plate after it has been carried off the
+	# counter and set down again.
+	await _both("%s_food_stayed_on_plate" % actor, "verify_plate_visuals", [plate])
 
 
 # Spawn a second set of ingredients through NetworkManager for the server's
@@ -292,6 +296,7 @@ func _find_client_id() -> int:
 # Run one step as `actor` (locally if that's us, over RPC if it's the client)
 # and record the verdict.
 func _step(actor: String, label: String, step: String, args: Array) -> void:
+	_current_step = label
 	_banner("STEP %d: %s  (on the %s)" % [_next_step_no(), label, actor.to_upper()])
 	var res: Dictionary
 	if actor == "server":
@@ -307,6 +312,7 @@ func _step(actor: String, label: String, step: String, args: Array) -> void:
 # Run the same check on both peers - the server's state and the client's must
 # agree, which is the whole point of the exercise.
 func _both(label: String, step: String, args: Array) -> void:
+	_current_step = label
 	_banner("STEP %d: %s  (checked on BOTH peers)" % [_next_step_no(), label])
 	_record(label, "server", await _run_local_step(step, args))
 	_record(label, "client", await _remote(step, args))
@@ -372,6 +378,7 @@ func _report() -> void:
 
 @rpc("authority", "reliable")
 func _cmd(step: String, args: Array) -> void:
+	_current_step = step
 	_log("<- server: %s%s" % [step, args])
 	_running = true
 	var res := await _run_local_step(step, args)
@@ -424,6 +431,8 @@ func _run_local_step(step: String, args: Array) -> Dictionary:
 			return _check_food_exists(args[0])
 		"verify_plate_contains":
 			return _check_plate_contains(args[0], args[1])
+		"verify_plate_visuals":
+			return _check_plate_visuals(args[0])
 	return {"ok": false, "detail": "unknown step %s" % step}
 
 
@@ -800,9 +809,58 @@ func _describe(item: Node3D) -> Dictionary:
 		# literally what the player sees sitting on the plate.
 		d["meals_shown"] = _visual_count(item, "Container/MealContainer")
 		d["sides_shown"] = _visual_count(item, "Container/SidesContainer")
+		# Food sitting on a plate is a cosmetic child of the plate, so it must
+		# travel with it. Keys starting with "_" are per-peer diagnostics that
+		# _compare() skips (floats won't match exactly across peers); the
+		# attached flag is asserted absolutely instead, because this can — and
+		# did — go wrong on both peers at once, which a diff would miss.
+		var off := _max_visual_offset(item)
+		d["_visual_offset"] = off
+		d["visuals_attached"] = off <= MAX_VISUAL_OFFSET
 	return d
 
 
+## How far a cosmetic item on a plate may sit from the plate's origin. The plate
+## is ~0.4m across and the furthest slot is ~0.12m out, so anything past this has
+## come off the plate.
+const MAX_VISUAL_OFFSET := 0.3
+
+
+# Largest distance from the plate's origin to any of the cosmetic items it is
+# displaying. -1 when the plate is showing nothing.
+func _max_visual_offset(item: Node3D) -> float:
+	var worst := -1.0
+	for path in ["Container/MealContainer", "Container/SidesContainer"]:
+		var root := item.get_node_or_null(path)
+		if not root:
+			continue
+		for c in root.get_children():
+			if c is Node3D and not c.is_queued_for_deletion():
+				worst = maxf(worst, item.global_position.distance_to((c as Node3D).global_position))
+	return snappedf(worst, 0.001)
+
+
+# What the cosmetic children look like, for diagnosing why they moved.
+func _visual_diag(item: Node3D) -> String:
+	var parts: Array[String] = []
+	for path in ["Container/MealContainer", "Container/SidesContainer"]:
+		var root := item.get_node_or_null(path)
+		if not root:
+			continue
+		for c in root.get_children():
+			var s := "%s global=%s local=%s parent=%s (%.3fm from plate at %s)" % [
+				c.name, (c as Node3D).global_position, (c as Node3D).position,
+				c.get_parent().name,
+				item.global_position.distance_to((c as Node3D).global_position),
+				item.global_position]
+			if c is RigidBody3D:
+				s += " [RigidBody3D freeze=%s freeze_mode=%d layer=%d top_level=%s queued=%s]" % [
+					c.freeze, c.freeze_mode, c.collision_layer, c.top_level,
+					c.is_queued_for_deletion()]
+			parts.append(s)
+	return "; ".join(parts) if parts else "(nothing on the plate)"
+
+
 func _visual_count(item: Node3D, path: String) -> int:
 	var n := item.get_node_or_null(path)
 	if not n:
@@ -864,10 +922,20 @@ func _compare(server: Dictionary, client: Dictionary) -> Array[String]:
 		if dist > SYNC_POS_TOLERANCE:
 			problems.append("%s is %.3fm apart (server %s vs client %s)" % [name, dist, s["pos"], c["pos"]])
 		for key in s:
-			if key == "pos":
+			# "_" keys are per-peer diagnostics, not things that must match.
+			if key == "pos" or key.begins_with("_"):
 				continue
 			if s[key] != c[key]:
 				problems.append("%s.%s: server=%s client=%s" % [name, key, s[key], c[key]])
+	# Absolute invariants, checked per peer. A cross-peer diff can't catch a
+	# fault that happens identically on both sides.
+	for peer_name in ["server", "client"]:
+		var snap: Dictionary = server if peer_name == "server" else client
+		for name in snap:
+			var d: Dictionary = snap[name]
+			if d.has("visuals_attached") and not d["visuals_attached"]:
+				problems.append("on the %s, %s's food has come off the plate (%.3fm from it, limit %.2f)"
+					% [peer_name, name, d.get("_visual_offset", -1.0), MAX_VISUAL_OFFSET])
 	return problems
 
 
@@ -893,6 +961,76 @@ func _audit_sync(label: String) -> void:
 	_record("sync_after_" + label, "both", res)
 
 
+# --- Live watch on plate visuals ------------------------------------------
+#
+# The per-step checks tell us the food ended up off the plate, but not when or
+# why. This watches every frame and reports the first frame a cosmetic item
+# departs from its slot, along with what was happening to the plate at the time.
+
+## Local offset from its slot at which a cosmetic item counts as having moved.
+const VISUAL_DRIFT_EPSILON := 0.05
+
+var _current_step := "(before any step)"
+var _drift_reported := {}
+
+
+func _process(_delta: float) -> void:
+	if not _world:
+		return
+	for root in [_world, _world.get_node_or_null("WorldContent")]:
+		if not root:
+			continue
+		for item in root.get_children():
+			if not (item is Node3D) or not item.get_node_or_null("PlateController"):
+				continue
+			_watch_plate(item as Node3D)
+
+
+func _watch_plate(plate: Node3D) -> void:
+	for path in ["Container/MealContainer", "Container/SidesContainer"]:
+		var holder := plate.get_node_or_null(path)
+		if not holder:
+			continue
+		for c in holder.get_children():
+			if not (c is Node3D) or c.is_queued_for_deletion():
+				continue
+			var drift: float = (c as Node3D).position.length()
+			var key := c.get_instance_id()
+			if drift <= VISUAL_DRIFT_EPSILON:
+				_drift_reported.erase(key)
+				continue
+			if _drift_reported.has(key):
+				continue
+			_drift_reported[key] = true
+			_log("VISUAL DRIFT: %s on %s moved to local %s (%.3f from its slot) during '%s'" % [
+				c.name, plate.name, (c as Node3D).position, drift, _current_step])
+			_log("  plate: pos=%s freeze=%s held_by=%s authority=%d" % [
+				plate.global_position, plate.freeze if plate is RigidBody3D else "-",
+				plate.get_picked_up_by() if plate.has_method("get_picked_up_by") else "-",
+				plate.get_multiplayer_authority()])
+			if c is RigidBody3D:
+				_log("  visual: freeze=%s mode=%d layer=%d top_level=%s sleeping=%s lin_vel=%s" % [
+					c.freeze, c.freeze_mode, c.collision_layer, c.top_level,
+					c.sleeping, c.linear_velocity])
+
+
+# Food shown on a plate is a cosmetic child of that plate, so it has to stay put
+# when the plate is picked up and carried around. If it drifts away, the player
+# sees the burger fly off the plate.
+func _check_plate_visuals(plate_name: String) -> Dictionary:
+	var plate := _find(plate_name)
+	if not plate:
+		return {"ok": false, "detail": "'%s' does not exist on this peer" % plate_name}
+	var off := _max_visual_offset(plate)
+	if off < 0.0:
+		return {"ok": false, "detail": "%s is not showing any food to check" % plate_name}
+	if off > MAX_VISUAL_OFFSET:
+		return {"ok": false, "detail": "%s's food has come off the plate: %.3fm away (limit %.2f). %s"
+			% [plate_name, off, MAX_VISUAL_OFFSET, _visual_diag(plate)]}
+	return {"ok": true, "detail": "%s's food is still on it (%.3fm from centre). %s"
+		% [plate_name, off, _visual_diag(plate)]}
+
+
 # --- Diagnostics -----------------------------------------------------------
 
 func _diag(item: Node3D) -> String:

test/multiPlayerTest.tscn

@@ -108,5 +108,5 @@ transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -0.40037438, 1.0094403, 0.341
 [node name="Counter" parent="." unique_id=1487893288 instance=ExtResource("12_j5uvh")]
 transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -1, 0.5, 0)
 
-[node name="Counter2" parent="." instance=ExtResource("12_j5uvh")]
+[node name="Counter2" parent="." unique_id=860178119 instance=ExtResource("12_j5uvh")]
 transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -2, 0.5, 0)