Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bitECS: Check if entity has networked component before taking ownership #6258

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

keianhzo
Copy link
Contributor

@keianhzo keianhzo commented Sep 14, 2023

Fixes #6254

This PR adds a check for Networked component when a Constraint is added to a body to taking ownership of entities that are not networked.

In some contexts we might want to add constratints to bodies of entities that are not networked. Inthe current use case most of the times this is the case as the only interactable entities are dropped medias but behavior graphs allow creating physics objects that can be interactable and might not necessarily need to be networked (although most of the times they will)

@keianhzo keianhzo force-pushed the bitecs-link-crash-fix branch from 1920a92 to c34178c Compare September 14, 2023 12:42
@keianhzo keianhzo requested a review from takahirox September 14, 2023 13:00
@takahirox takahirox changed the title Check if entity has networked component before taking ownership bitECS: Check if entity has networked component before taking ownership Sep 15, 2023
@takahirox takahirox added P1 Address as quickly as possible new-loader labels Sep 15, 2023
@takahirox
Copy link
Contributor

takahirox commented Sep 15, 2023

Haven't looked into the details yet but let me share my impression for now.

The expected behavior seems to do nothing when the preview image is clicked. So the fact that clicking it causes crash is weird to me because that means something interaction processing runs when it's clicked. I was thinking the root fix was not to run such interaction processing when the preview image is clicked. Anyways, I will look into details next week.

To call takeOwnership only for networked entity itself looks the right direction tho.

Copy link
Contributor

@takahirox takahirox left a comment

Choose a reason for hiding this comment

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

Looking good to me

@keianhzo keianhzo merged commit 234216a into master Sep 22, 2023
9 of 11 checks passed
@keianhzo keianhzo deleted the bitecs-link-crash-fix branch September 22, 2023 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-loader P1 Address as quickly as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bitECS: Link embedded in scene crashes Hubs
2 participants