Skip to content

Cache deleting identities Approach #1 - #56

Draft
ashtonmeuser wants to merge 2 commits into
PurrNet:devfrom
ashtonmeuser:cache-deleting
Draft

Cache deleting identities Approach #1#56
ashtonmeuser wants to merge 2 commits into
PurrNet:devfrom
ashtonmeuser:cache-deleting

Conversation

@ashtonmeuser

@ashtonmeuser ashtonmeuser commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

This keeps a cache of deleted identities until PostSimulate has been run. This allows resolving the GameObject of PredictedIdentities deleted during the previous simulation tick.

Opening this as a draft because I'm not thrilled with the asymmetry I've introduced between PredictedComponentID.GetGameObject and PredictedComponentID.GetIdentity. I think this change should go in one of the following directions:

  1. Make it a little less impactful by exposing a PredictedComponentID.GetIdentityIncludingDeleting (or similar) method that is then called by the predicted physics system(s) when firing events.
  2. Make it a little more heavy handed and always include just-deleted PredictedIdentities in PredictionManager.GetIdentity. Happy to hear thoughts and feedback.

EDIT: I went ahead and implemented both of the above approaches. This PR contains the lighter method of only using the deleting cache in the predicted physics systems while #57 is the heavier approach. See the diff here: https://github.com/ashtonmeuser/PurrDiction/compare/cache-deleting..cache-deleting-2

I noticed this because of the following case:

  1. Have two GameObjects each with a PredictedRigidbody2D, RB#1 and RB#2.
  2. Subscribe to RB#1's onCollisionExit event which, when fired, includes the GameObject with which RB#1 is no longer colliding.
  3. Ensure bodies are in contact e.g. RB#2 sitting on top of RB#1.
  4. Delete RB#2 via PredictionManager.hierarchy.Delete(rb2).
  5. Note that the onCollisionExit event includes null as the collider GO.

This makes it very difficult to reason about RB contacts.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bf7692d2-a6be-4af4-be24-c1dae1197c10

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Applied the cached mid-delete resolution fix to the predicted physics systems
@ashtonmeuser ashtonmeuser changed the title Cache deleting identities Cache deleting identities Approach #1 Jun 12, 2026
Comment on lines +1543 to 1547
finally
{
UnregisterPooledInstance(instance);
pool.Delete(instance);
if (!isSimulating)
EndDeletingInstance(deletingIds);
}

@ashtonmeuser ashtonmeuser Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a quick test, this should never be the case (as long as users are calling PredictedHierarchy.Delete() during sim) and can therefore be removed. This entire try/finally as well as EndDeletingInstance can be removed. That said, it's not really harmful either and may help in the case of PredictedHierarchy misuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant