diff --git a/Containers/container.gd b/Containers/container.gd index d99550a..8464f07 100644 --- a/Containers/container.gd +++ b/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]) diff --git a/Containers/plate_controller.gd b/Containers/plate_controller.gd index 3f639da..b617482 100644 --- a/Containers/plate_controller.gd +++ b/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. diff --git a/Net/net_pickable.gd b/Net/net_pickable.gd index 2aa7d30..430ccec 100644 --- a/Net/net_pickable.gd +++ b/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,15 +82,16 @@ 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(): - print( - "%s: reclaiming ownership, restoring freeze_mode %d->%d collision_mask %d->%d" % [ - _pickable.name, _pickable.freeze_mode, _original_freeze_mode, - _pickable.collision_mask, _pickable.original_collision_mask - ] - ) - _pickable.freeze_mode = _original_freeze_mode - _pickable.collision_mask = _pickable.original_collision_mask + 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, + _pickable.collision_mask, _pickable.original_collision_mask + ] + ) + _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 @@ -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 diff --git a/recipes.yaml.uid b/recipes.yaml.uid deleted file mode 100644 index d264c0c..0000000 --- a/recipes.yaml.uid +++ /dev/null @@ -1 +0,0 @@ -uid://dqqu56yetl8ok diff --git a/test/mp_test_driver.gd b/test/mp_test_driver.gd index 2756399..470a55f 100644 --- a/test/mp_test_driver.gd +++ b/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: diff --git a/test/multiPlayerTest.tscn b/test/multiPlayerTest.tscn index 9aa2c0b..b26aa39 100644 --- a/test/multiPlayerTest.tscn +++ b/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)