-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Make reflect_partial_eq return more accurate results #5204
Copy link
Copy link
Closed
Labels
A-ReflectionRuntime information about typesRuntime information about typesC-UsabilityA targeted quality-of-life change that makes Bevy easier to useA targeted quality-of-life change that makes Bevy easier to useD-TrivialNice and easy! A great choice to get started with BevyNice and easy! A great choice to get started with Bevy
Metadata
Metadata
Assignees
Labels
A-ReflectionRuntime information about typesRuntime information about typesC-UsabilityA targeted quality-of-life change that makes Bevy easier to useA targeted quality-of-life change that makes Bevy easier to useD-TrivialNice and easy! A great choice to get started with BevyNice and easy! A great choice to get started with Bevy
What problem does this solve or what need does it fill?
Currently, all
***_partial_eqhelper methods do something like:bevy/crates/bevy_reflect/src/struct_trait.rs
Lines 455 to 457 in dfe9690
If
field_value.reflect_partial_eq(value)returnedNone, it makes more sense for the entire function to returnNoneas this would suggest to the user that the comparison didn't just fail, but couldn't even be performed.However, this is not currently done and instead the functions just return
Some(false).What solution would you like?
Noneon a matchedNone:bevy/crates/bevy_reflect/src/struct_trait.rs
Lines 455 to 457 in dfe9690
bevy/crates/bevy_reflect/src/tuple_struct.rs
Lines 366 to 368 in dfe9690
bevy/crates/bevy_reflect/src/tuple.rs
Line 386 in dfe9690
bevy/crates/bevy_reflect/src/array.rs
Lines 335 to 337 in dfe9690
bevy/crates/bevy_reflect/src/list.rs
Lines 309 to 311 in dfe9690
bevy/crates/bevy_reflect/src/map.rs
Lines 361 to 363 in dfe9690
Nonevalue might be returned)What alternative(s) have you considered?
Not doing this. Most usages of these functions and
reflect_partial_eqitself useunwrap_or_default()anyways, so they would not be affected by such a change. However, adding this in allows us to give a bit more info to the users who want it.Additional context
Originally brought to attention by @nicopap in #4761 (comment).