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

Don't panic in scene::replicate for non-reflected components #327

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

Shatur
Copy link
Contributor

@Shatur Shatur commented Sep 13, 2024

It's useful to remove some components that are replicated from scene serialization.

@Shatur Shatur requested a review from UkoeHB September 13, 2024 15:29
Comment on lines +86 to +92
debug!("ignoring `{type_name}` because it's not registered");
continue;
};
let Some(reflect_component) = registration.data::<ReflectComponent>() else {
debug!("ignoring `{type_name}` because it's missing `#[reflect(Component)]`");
continue;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it right to use debug messages here? Often you don't have debug logs enables, which mean things silently fail. I'd go with warning if this is always a user error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just could be the indented by the user. For example, in my game I replicate calculated navigation paths from server, but I don't want to serialize them as part of the scene. So I just removed the reflection for them.

Copy link

codecov bot commented Sep 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.78%. Comparing base (c740527) to head (9880317).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #327   +/-   ##
=======================================
  Coverage   89.77%   89.78%           
=======================================
  Files          37       37           
  Lines        2318     2320    +2     
=======================================
+ Hits         2081     2083    +2     
  Misses        237      237           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Shatur Shatur merged commit 1cd4a0a into master Sep 13, 2024
6 checks passed
@Shatur Shatur deleted the scene-replication-ignore branch September 13, 2024 15:57
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.

2 participants