Assorted cleanliness refactors #16250
Conversation
alice-i-cecile
added
C-Code-Quality
alice-i-cecile
changed the title
alice-i-cecile
added
the
S-Needs-Review
|
Please try to include a basic PR description and title: it makes this sort of thing much easier to review. |
alice-i-cecile
added
D-Macros
alice-i-cecile
changed the title
alice-i-cecile
added
A-UI
alice-i-cecile
changed the title
|
Merging. Please split these out and have a proper description next time though. |
| @@ -95,8 +95,7 @@ impl RelativeCursorPosition { | |||
| /// A helper function to check if the mouse is over the node | |||
| pub fn mouse_over(&self) -> bool { | |||
| self.normalized | |||
| .map(|position| self.normalized_visible_node_rect.contains(position)) | |||
| .unwrap_or(false) | |||
| .map_or(false, |position| self.normalized_visible_node_rect.contains(position)) | |||
There was a problem hiding this comment.
as in #16205, there's some distaste amongst some contributors (me included) with using map_or as it's harder to read because it put the "or" case first
There was a problem hiding this comment.
yes, hate it.
If map_or is used in the middle of a chain of composed functions then I can see it might be okay (maybe?). Placed at the end of a chain thouigh I very strongly prefer unwrap_or.
| fn custom_default() -> i32 { | ||
| -1 | ||
| } |
There was a problem hiding this comment.
I would prefer to keep this where it was, duplicated in both tests. this function doesn't really make sense as a "global" function, and it reduces readability by making it far away from where it's used
alice-i-cecile
added
S-Waiting-on-Author
| } | ||
| } | ||
| name.map(String::into_boxed_str) | ||
| name |
There was a problem hiding this comment.
@ickshonpe is this still relevant? https://github.com/bevyengine/bevy/pull/16205/files#r1829513146
I don't see the issue. 🤔
|
Looks like #16205 should be closed in favor of this PR. |
Small refactor for some assorted code quality issues.