Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbeb7a4e16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while (history.firstPending < history.sentTicks.Count && | ||
| history.sentTicks[history.firstPending] <= ackedServerTick) | ||
| { | ||
| history.acknowledgedTick = history.sentTicks[history.firstPending++]; |
There was a problem hiding this comment.
When a root is in a sparse interest tier, normal frames are still sent through the unreliable SendFrameToRemote path, so a frame that included this root can be dropped while later frames still advance ackedServerTick. Advancing acknowledgedTick for every sentTick <= ackedServerTick then makes the next root update delta against a state the client never received, corrupting reconciliation under packet loss; this needs per-root delivery confirmation or an absolute/reliable resend after skipped sends.
Useful? React with 👍 / 👎.
| if (_relevance[i] != IsLocallyRelevant(i)) | ||
| continue; |
There was a problem hiding this comment.
When a locally culled subtree with LocalPhysics child rigidbodies re-enters during the rollback that applies the absolute frame, OnStartingToRollback has already forced the body kinematic, but relevance flips from false to true before this check. This branch skips restoring the saved state in exactly that re-entry case, leaving the now-active Rigidbody kinematic until some unrelated code changes it; only the true-to-false cull transition should keep the rollback freeze.
Useful? React with 👍 / 👎.
| if (_relevance[i] != IsLocallyRelevant(i)) | ||
| continue; |
There was a problem hiding this comment.
The same re-entry path affects LocalPhysics2D: if a culled subtree becomes relevant while the rollback is applying the authoritative frame, the body was already forced to Kinematic in OnStartingToRollback, and this relevance-change guard skips restoring the saved bodyType. Active Rigidbody2D children without their own prediction component can therefore remain kinematic after re-entry.
Useful? React with 👍 / 👎.
| if (i >= _systemsCount) | ||
| break; |
There was a problem hiding this comment.
When a client is missing one of the server's positional systems, this new guard stops the loop but leaves the remaining state payload unread; because positional entries do not carry per-entry lengths, the following event-handler flag/entries are then decoded from the wrong bit offset. In that mismatch case, the frame needs to be discarded or converted to an ID/length-delimited repair path before continuing to later sections.
Useful? React with 👍 / 👎.
| bool force = forceAllRelevant || | ||
| interestControls != null && interestControls.RequiresAbsolute(root); | ||
| return interest.ShouldSerializeState(player, root, localTick, force); |
There was a problem hiding this comment.
When ownership is released while the old owner's copy is already outside interest, the cull control is delivered but this path filters out the root's state because the server no longer sees system.owner == player and the resolved tier is culled. The old client never receives the owner = null metadata, so its local ownership exemption keeps the root materialized and predicted instead of applying the cull; cull transitions caused by ownership changes need to force at least the ownership/absolute state through.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db91bb6599
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| interest?.Dispose(); | ||
| interest = null; |
There was a problem hiding this comment.
If predictionLODProfile is changed or cleared while the manager is spawned, this drops the server-side interest module but leaves each existing PlayerPacker.interestControls with the tiers it previously marked delivered. On the next initialization, InitializePlayer only queues nonzero tiers, so a root that was previously delivered as culled/sparse and now resolves to tier 0 never sends the tier-0 control; affected clients keep the old local tier and can leave the root dematerialized or filtered until some later nonzero transition happens. Clear the per-client interest control state or explicitly queue reset tiers when reinitializing.
Useful? React with 👍 / 👎.
| if (changed && predictionManager && predictionManager.interest != null) | ||
| { | ||
| predictionManager.HandleLocalOwnershipChanged(this); |
There was a problem hiding this comment.
When interest has been disabled or cleared, ownership changes now skip HandleLocalOwnershipChanged, so the local owned-root cache is not invalidated. If the cache was populated before disabling interest and the profile is later re-enabled, GetLocallyOwnedRoots() can reuse the stale set and exempt the wrong roots from culling/policy overrides until another ownership change happens while interest is active; invalidate the cache on every owner change, even when no interest module is currently installed.
Useful? React with 👍 / 👎.
|
|
||
| try | ||
| { | ||
| RemovePieceSet(_localInterestRecordsScratch, _localInterestMembersScratch, true, false, false); |
There was a problem hiding this comment.
When a parent root is culled while a child root remains materialized, this local-only removal path calls RemovePieceSet, whose rescue logic detaches non-member descendants from the disappearing parent. Because the child root's hierarchy record did not change, later parent re-entry does not recreate or reattach that already-live child, leaving it unparented with the wrong transform relationship until some unrelated parent-state change occurs; restore affected child attachments when the parent materializes or avoid culling parent roots independently of live child roots.
Useful? React with 👍 / 👎.
1 participant
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.