From 7abb9a28140617a39c502ca89fbad0030e3a183e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 3 Nov 2020 22:00:56 +0100 Subject: [PATCH] Improve Id generation, with more aggressive name clash warnings --- egui/src/containers/area.rs | 2 +- egui/src/containers/collapsing_header.rs | 19 +++--- egui/src/containers/combo_box.rs | 2 +- egui/src/containers/resize.rs | 5 +- egui/src/containers/scroll_area.rs | 27 +++----- egui/src/containers/window.rs | 4 +- egui/src/context.rs | 83 +++++++++--------------- egui/src/demos/demo_window.rs | 27 -------- egui/src/demos/demo_windows.rs | 3 +- egui/src/demos/mod.rs | 3 +- egui/src/demos/tests.rs | 51 +++++++++++++++ egui/src/id.rs | 6 +- egui/src/ui.rs | 52 ++++----------- egui/src/widgets/mod.rs | 9 +-- egui/src/widgets/slider.rs | 2 +- egui/src/widgets/text_edit.rs | 2 +- 16 files changed, 134 insertions(+), 163 deletions(-) create mode 100644 egui/src/demos/tests.rs diff --git a/egui/src/containers/area.rs b/egui/src/containers/area.rs index e686f23e..b2500bbc 100644 --- a/egui/src/containers/area.rs +++ b/egui/src/containers/area.rs @@ -113,7 +113,7 @@ impl Area { fixed_pos, } = self; - let layer_id = LayerId { order, id }; + let layer_id = LayerId::new(order, id); let state = ctx.memory().areas.get(id).cloned(); let mut state = state.unwrap_or_else(|| State { diff --git a/egui/src/containers/collapsing_header.rs b/egui/src/containers/collapsing_header.rs index dedfccc5..997307bd 100644 --- a/egui/src/containers/collapsing_header.rs +++ b/egui/src/containers/collapsing_header.rs @@ -129,17 +129,19 @@ pub fn paint_icon(ui: &mut Ui, openness: f32, response: &Response) { pub struct CollapsingHeader { label: Label, default_open: bool, - id_source: Option, + id_source: Id, } impl CollapsingHeader { pub fn new(label: impl Into) -> Self { + let label = Label::new(label) + .text_style(TextStyle::Button) + .multiline(false); + let id_source = Id::new(label.text()); Self { - label: Label::new(label) - .text_style(TextStyle::Button) - .multiline(false), + label, default_open: false, - id_source: None, + id_source, } } @@ -149,9 +151,9 @@ impl CollapsingHeader { } /// Explicitly set the source of the `Id` of this widget, instead of using title label. - /// This is useful if the title label is dynamics. + /// This is useful if the title label is dynamic or not unique. pub fn id_source(mut self, id_source: impl Hash) -> Self { - self.id_source = Some(Id::new(id_source)); + self.id_source = Id::new(id_source); self } } @@ -176,8 +178,7 @@ impl CollapsingHeader { // TODO: horizontal layout, with icon and text as labels. Insert background behind using Frame. - let title = label.text(); - let id = ui.make_unique_child_id_full(id_source, Some(title)); + let id = ui.make_persistent_id(id_source); let available = ui.available_finite(); let text_pos = available.min + vec2(ui.style().spacing.indent, 0.0); diff --git a/egui/src/containers/combo_box.rs b/egui/src/containers/combo_box.rs index b9f608a8..8c894150 100644 --- a/egui/src/containers/combo_box.rs +++ b/egui/src/containers/combo_box.rs @@ -7,7 +7,7 @@ pub fn combo_box_with_label( menu_contents: impl FnOnce(&mut Ui), ) -> Response { let label = label.into(); - let button_id = ui.make_unique_child_id(label.text()); + let button_id = ui.make_persistent_id(label.text()); ui.horizontal(|ui| { let mut response = combo_box(ui, button_id, selected, menu_contents); diff --git a/egui/src/containers/resize.rs b/egui/src/containers/resize.rs index 63232b7d..ce0118f7 100644 --- a/egui/src/containers/resize.rs +++ b/egui/src/containers/resize.rs @@ -145,7 +145,8 @@ struct Prepared { impl Resize { fn begin(&mut self, ui: &mut Ui) -> Prepared { - let id = self.id.unwrap_or_else(|| ui.make_child_id("resize")); + let position = ui.available().min; + let id = self.id.unwrap_or_else(|| ui.make_persistent_id("resize")); let mut state = ui.memory().resize.get(&id).cloned().unwrap_or_else(|| { ui.ctx().request_repaint(); // counter frame delay @@ -167,8 +168,6 @@ impl Resize { .at_least(self.min_size) .at_most(self.max_size); - let position = ui.available().min; - let corner_response = if self.resizable { // Resize-corner: let corner_size = Vec2::splat(ui.style().visuals.resize_corner_size); diff --git a/egui/src/containers/scroll_area.rs b/egui/src/containers/scroll_area.rs index 884bd7af..92a645aa 100644 --- a/egui/src/containers/scroll_area.rs +++ b/egui/src/containers/scroll_area.rs @@ -72,7 +72,7 @@ impl ScrollArea { let ctx = ui.ctx().clone(); - let id = ui.make_child_id("scroll_area"); + let id = ui.make_persistent_id("scroll_area"); let state = ctx .memory() .scroll_areas @@ -221,28 +221,21 @@ impl Prepared { pos2(right, from_content(state.offset.y + inner_rect.height())), ); - // intentionally use same id for inside and outside of handle let interact_id = id.with("vertical"); - let mut response = ui.interact(handle_rect, interact_id, Sense::click_and_drag()); + let response = ui.interact(outer_scroll_rect, interact_id, Sense::click_and_drag()); - if let Some(mouse_pos) = ui.input().mouse.pos { - if response.active { - if inner_rect.top() <= mouse_pos.y && mouse_pos.y <= inner_rect.bottom() { - state.offset.y += - ui.input().mouse.delta.y * content_size.y / inner_rect.height(); - } - } else { - // Check for mouse down outside handle: - let scroll_bg_response = - ui.interact(outer_scroll_rect, interact_id, Sense::click_and_drag()); - - if scroll_bg_response.active { + if response.active { + if let Some(mouse_pos) = ui.input().mouse.pos { + if handle_rect.contains(mouse_pos) { + if inner_rect.top() <= mouse_pos.y && mouse_pos.y <= inner_rect.bottom() { + state.offset.y += + ui.input().mouse.delta.y * content_size.y / inner_rect.height(); + } + } else { // Center scroll at mouse pos: let mpos_top = mouse_pos.y - handle_rect.height() / 2.0; state.offset.y = remap(mpos_top, top..=bottom, 0.0..=content_size.y); } - - response = response.union(scroll_bg_response); } } diff --git a/egui/src/containers/window.rs b/egui/src/containers/window.rs index f44792ae..24768017 100644 --- a/egui/src/containers/window.rs +++ b/egui/src/containers/window.rs @@ -691,7 +691,7 @@ impl TitleBar { ); } - let title_bar_id = ui.make_child_id("title_bar"); + let title_bar_id = ui.make_position_id().with("title_bar"); if ui .interact(self.rect, title_bar_id, Sense::click()) .double_clicked @@ -716,7 +716,7 @@ impl TitleBar { } fn close_button(ui: &mut Ui, rect: Rect) -> Response { - let close_id = ui.make_child_id("window_close_button"); + let close_id = ui.make_position_id().with("window_close_button"); let response = ui.interact(rect, close_id, Sense::click()); ui.expand_to_include_rect(response.rect); diff --git a/egui/src/context.rs b/egui/src/context.rs index 785dd123..cbfd47c9 100644 --- a/egui/src/context.rs +++ b/egui/src/context.rs @@ -51,7 +51,7 @@ pub struct Context { // The output of a frame: graphics: Mutex, output: Mutex, - /// Used to debug name clashes of e.g. windows + /// Used to debug `Id` clashes of widgets. used_ids: Mutex>, paint_stats: Mutex, @@ -336,49 +336,26 @@ impl Context { // --------------------------------------------------------------------- - /// Generate a id from the given source. - /// If it is not unique, an error will be printed at the given position. - pub fn make_unique_id(self: &Arc, source: IdSource, pos: Pos2) -> Id - where - IdSource: std::hash::Hash + std::fmt::Debug + Copy, - { - self.register_unique_id(Id::new(source), source, pos) - } - - pub fn is_unique_id(&self, id: Id) -> bool { - !self.used_ids.lock().contains_key(&id) - } - - /// If the given Id is not unique, an error will be printed at the given position. - pub fn register_unique_id( - self: &Arc, - id: Id, - source_name: impl std::fmt::Debug, - pos: Pos2, - ) -> Id { - if let Some(clash_pos) = self.used_ids.lock().insert(id, pos) { - let painter = self.debug_painter(); - if clash_pos.distance(pos) < 4.0 { - painter.error( - pos, - &format!("use of non-unique ID {:?} (name clash?)", source_name), - ); - } else { - painter.error( - clash_pos, - &format!("first use of non-unique ID {:?} (name clash?)", source_name), - ); - painter.error( - pos, - &format!( - "second use of non-unique ID {:?} (name clash?)", - source_name - ), - ); + /// If the given `Id` is not unique, an error will be printed at the given position. + /// Call this for `Id`:s that need interaction or persistence. + pub(crate) fn register_unique_id(self: &Arc, id: Id, new_pos: Pos2) { + if let Some(prev_pos) = self.used_ids.lock().insert(id, new_pos) { + if prev_pos == new_pos { + // Likely same Widget being interacted with twice, which is fine. + return; } - id - } else { - id + + let id_str = id.short_debug_format(); + + let painter = self.debug_painter(); + if prev_pos.distance(new_pos) < 4.0 { + painter.error(new_pos, format!("Double use of ID {}", id_str)); + } else { + painter.error(prev_pos, format!("First use of ID {}", id_str)); + painter.error(new_pos, format!("Second use of ID {}", id_str)); + } + + // TODO: a tooltip explaining this. } } @@ -449,16 +426,14 @@ impl Context { layer_id: LayerId, clip_rect: Rect, rect: Rect, - interaction_id: Option, + id: Option, sense: Sense, ) -> Response { let interact_rect = rect.expand2(0.5 * self.style().spacing.item_spacing); // make it easier to click. TODO: nice way to do this let hovered = self.contains_mouse(layer_id, clip_rect, interact_rect); - let has_kb_focus = interaction_id - .map(|id| self.memory().has_kb_focus(id)) - .unwrap_or(false); + let has_kb_focus = id.map(|id| self.memory().has_kb_focus(id)).unwrap_or(false); - if interaction_id.is_none() || sense == Sense::nothing() || !layer_id.allow_interaction() { + if id.is_none() || sense == Sense::nothing() || !layer_id.allow_interaction() { // Not interested or allowed input: return Response { ctx: self.clone(), @@ -471,15 +446,17 @@ impl Context { has_kb_focus, }; } - let interaction_id = interaction_id.unwrap(); + let id = id.unwrap(); + + self.register_unique_id(id, rect.min); let mut memory = self.memory(); memory.interaction.click_interest |= hovered && sense.click; memory.interaction.drag_interest |= hovered && sense.drag; - let active = memory.interaction.click_id == Some(interaction_id) - || memory.interaction.drag_id == Some(interaction_id); + let active = + memory.interaction.click_id == Some(id) || memory.interaction.drag_id == Some(id); if self.input.mouse.pressed { if hovered { @@ -496,7 +473,7 @@ impl Context { if sense.click && memory.interaction.click_id.is_none() { // start of a click - memory.interaction.click_id = Some(interaction_id); + memory.interaction.click_id = Some(id); response.active = true; } @@ -504,7 +481,7 @@ impl Context { && (memory.interaction.drag_id.is_none() || memory.interaction.drag_is_window) { // start of a drag - memory.interaction.drag_id = Some(interaction_id); + memory.interaction.drag_id = Some(id); memory.interaction.drag_is_window = false; memory.window_interaction = None; // HACK: stop moving windows (if any) response.active = true; diff --git a/egui/src/demos/demo_window.rs b/egui/src/demos/demo_window.rs index c167dab4..4bbb01de 100644 --- a/egui/src/demos/demo_window.rs +++ b/egui/src/demos/demo_window.rs @@ -105,33 +105,6 @@ impl DemoWindow { painter.line_segment([c, c + r * Vec2::angled(TAU * 3.0 / 8.0)], stroke); }); }); - - if false { - // TODO: either show actual name clash, or remove this example - ui.collapsing("Name clash demo", |ui| { - ui.label("\ - Widgets that store state require unique identifiers so we can track their state between frames. \ - Identifiers are normally derived from the titles of the widget."); - - ui.label("\ - For instance, collapsable headers needs to store wether or not they are open. \ - If you fail to give them unique names then clicking one will open both. \ - To help you debug this, an error message is printed on screen:"); - - ui.collapsing("Collapsing header", |ui| { - ui.label("Contents of first foldable ui"); - }); - ui.collapsing("Collapsing header", |ui| { - ui.label("Contents of second foldable ui"); - }); - - ui.label("\ - Most widgets don't need unique names, but are tracked \ - based on their position on screen. For instance, buttons:"); - let _ = ui.button("Button"); - let _ = ui.button("Button"); - }); - } } } diff --git a/egui/src/demos/demo_windows.rs b/egui/src/demos/demo_windows.rs index 504d46e7..0557ad23 100644 --- a/egui/src/demos/demo_windows.rs +++ b/egui/src/demos/demo_windows.rs @@ -39,6 +39,7 @@ impl Default for Demos { demos: vec![ (false, Box::new(crate::demos::DancingStrings::default())), (false, Box::new(crate::demos::DragAndDropDemo::default())), + (false, Box::new(crate::demos::Tests::default())), ], } } @@ -323,7 +324,7 @@ fn show_menu_bar(ui: &mut Ui, windows: &mut OpenWindows, seconds_since_midnight: } if ui .button("Clear entire Egui memory") - .on_hover_text("Forget scroll, collapsibles etc") + .on_hover_text("Forget scroll, collapsing headers etc") .clicked { *ui.ctx().memory() = Default::default(); diff --git a/egui/src/demos/mod.rs b/egui/src/demos/mod.rs index be40f938..a293af93 100644 --- a/egui/src/demos/mod.rs +++ b/egui/src/demos/mod.rs @@ -9,12 +9,13 @@ mod demo_windows; mod drag_and_drop; mod fractal_clock; mod sliders; +mod tests; pub mod toggle_switch; mod widgets; pub use { app::*, color_test::ColorTest, dancing_strings::DancingStrings, demo_window::DemoWindow, - demo_windows::*, drag_and_drop::*, fractal_clock::FractalClock, sliders::Sliders, + demo_windows::*, drag_and_drop::*, fractal_clock::FractalClock, sliders::Sliders, tests::Tests, widgets::Widgets, }; diff --git a/egui/src/demos/tests.rs b/egui/src/demos/tests.rs new file mode 100644 index 00000000..09d2de36 --- /dev/null +++ b/egui/src/demos/tests.rs @@ -0,0 +1,51 @@ +use crate::*; + +#[derive(Default)] +pub struct Tests {} + +impl demos::Demo for Tests { + fn name(&self) -> &str { + "Tests" + } + + fn show(&mut self, ctx: &std::sync::Arc, open: &mut bool) { + Window::new(self.name()).open(open).show(ctx, |ui| { + use demos::View; + self.ui(ui); + }); + } +} + +impl demos::View for Tests { + fn ui(&mut self, ui: &mut Ui) { + ui.heading("Name collision example"); + + ui.label("\ + Widgets that store state require unique and persisting identifiers so we can track their state between frames.\n\ + For instance, collapsable headers needs to store wether or not they are open. \ + Their Id:s are derived from their names. \ + If you fail to give them unique names then clicking one will open both. \ + To help you debug this, an error message is printed on screen:"); + + ui.collapsing("Collapsing header", |ui| { + ui.label("Contents of first foldable ui"); + }); + ui.collapsing("Collapsing header", |ui| { + ui.label("Contents of second foldable ui"); + }); + + ui.label("\ + Any widget that can be interacted with also need a unique Id. \ + For most widgets the Id is generated by a running counter. \ + As long as elements are not added or removed, the Id stays the same. \ + This is fine, because during interaction (i.e. while dragging a slider), \ + the number of widgets previously in the same window is most likely not changing \ + (and if it is, the window will have a new layout, and the slider will endup somewhere else, and so aborthing the interaction probably makes sense)."); + + ui.label("So these buttons have automatic Id:s, and therefore there is no name clash:"); + let _ = ui.button("Button"); + let _ = ui.button("Button"); + + ui.add(__egui_github_link_file!()); + } +} diff --git a/egui/src/id.rs b/egui/src/id.rs index c0ae75d4..d4402ae1 100644 --- a/egui/src/id.rs +++ b/egui/src/id.rs @@ -12,7 +12,7 @@ use std::hash::Hash; /// For some widgets `Id`s are also used to persist some state about the /// widgets, such as Window position or wether not a collapsing header region is open. /// -/// This implies that the `Id`s must be unqiue. +/// This implies that the `Id`s must be unique. /// /// For simple things like sliders and buttons that don't have any memory and /// doesn't move we can use the location of the widget as a source of identity. @@ -54,4 +54,8 @@ impl Id { child.hash(&mut hasher); Id(hasher.finish()) } + + pub(crate) fn short_debug_format(&self) -> String { + format!("{:04X}", self.0 as u16) + } } diff --git a/egui/src/ui.rs b/egui/src/ui.rs index e6f15f94..28137223 100644 --- a/egui/src/ui.rs +++ b/egui/src/ui.rs @@ -85,8 +85,8 @@ impl Ui { } pub fn child_ui(&mut self, max_rect: Rect, layout: Layout) -> Self { - let id = self.make_position_id(); // TODO: is this a good idea? self.child_count += 1; + let id = self.make_position_id(); // TODO: ui.id should be persistent and not position based. let cursor = layout.initial_cursor(max_rect); let min_size = Vec2::zero(); // TODO: From Style @@ -365,52 +365,22 @@ impl Ui { /// # `Id` creation impl Ui { - /// Will warn if the returned id is not guaranteed unique. - /// Use this to generate widget ids for widgets that have persistent state in Memory. - /// If the `id_source` is not unique within this ui - /// then an error will be printed at the current cursor position. - pub fn make_unique_child_id(&self, id_source: IdSource) -> Id + /// Use this to generate widget ids for widgets that have persistent state in `Memory`. + pub fn make_persistent_id(&self, id_source: IdSource) -> Id where IdSource: Hash + std::fmt::Debug, { - let id = self.id.with(&id_source); - // TODO: clip name clash error messages to clip rect - self.ctx().register_unique_id(id, id_source, self.cursor) - } - - /// Ideally, all widgets should use this. TODO - /// Widgets can set an explicit id source (user picked, e.g. some loop index), - /// and a default id source (e.g. label). - /// If they fail to be unique, a positional id will be used instead. - pub fn make_unique_child_id_full( - &mut self, - explicit_id_source: Option, - default_id_source: Option<&str>, - ) -> Id { - let id = if let Some(explicit_id_source) = explicit_id_source { - self.id.with(&explicit_id_source) - } else { - let id = self.id.with(default_id_source); - if self.ctx().is_unique_id(id) { - id - } else { - self.make_position_id() - } - }; - self.ctx() - .register_unique_id(id, default_id_source.unwrap_or_default(), self.cursor) + self.id.with(&id_source) } /// Make an Id that is unique to this position. /// Can be used for widgets that do NOT persist state in Memory /// but you still need to interact with (e.g. buttons, sliders). + /// Call AFTER allocating new space for your widget. + // TODO: return from `allocate_space` ? pub fn make_position_id(&self) -> Id { self.id.with(self.child_count) } - - pub fn make_child_id(&self, id_seed: impl Hash) -> Id { - self.id.with(id_seed) - } } /// # Interaction @@ -716,7 +686,7 @@ impl Ui { ); let indent = vec2(self.style().spacing.indent, 0.0); let child_rect = Rect::from_min_max(self.cursor + indent, self.max_rect.right_bottom()); // TODO: wrong for reversed layouts - let mut child_ui = Ui { + let mut child_ui = Self { id: self.id.with(id_source), ..self.child_ui(child_rect, self.layout) }; @@ -736,20 +706,20 @@ impl Ui { (ret, self.interact_hover(rect)) } - pub fn left_column(&mut self, width: f32) -> Ui { + pub fn left_column(&mut self, width: f32) -> Self { self.column(Align::Min, width) } - pub fn centered_column(&mut self, width: f32) -> Ui { + pub fn centered_column(&mut self, width: f32) -> Self { self.column(Align::Center, width) } - pub fn right_column(&mut self, width: f32) -> Ui { + pub fn right_column(&mut self, width: f32) -> Self { self.column(Align::Max, width) } /// A column ui with a given width. - pub fn column(&mut self, column_position: Align, width: f32) -> Ui { + pub fn column(&mut self, column_position: Align, width: f32) -> Self { let x = match column_position { Align::Min => 0.0, Align::Center => self.available().width() / 2.0 - width / 2.0, diff --git a/egui/src/widgets/mod.rs b/egui/src/widgets/mod.rs index a592766e..0bac8eb2 100644 --- a/egui/src/widgets/mod.rs +++ b/egui/src/widgets/mod.rs @@ -205,10 +205,11 @@ impl Widget for Hyperlink { } = self; let color = color::LIGHT_BLUE; let text_style = text_style.unwrap_or_else(|| ui.style().body_text_style); - let id = ui.make_child_id(&url); let font = &ui.fonts()[text_style]; let galley = font.layout_multiline(text, ui.available().width()); let rect = ui.allocate_space(galley.size); + + let id = ui.make_position_id(); let response = ui.interact(rect, id, Sense::click()); if response.hovered { ui.ctx().output().cursor_icon = CursorIcon::PointingHand; @@ -313,13 +314,13 @@ impl Widget for Button { let button_padding = ui.style().spacing.button_padding; - let id = ui.make_position_id(); let font = &ui.fonts()[text_style]; let galley = font.layout_multiline(text, ui.available().width()); let mut desired_size = galley.size + 2.0 * button_padding; desired_size = desired_size.at_least(ui.style().spacing.interact_size); let rect = ui.allocate_space(desired_size); + let id = ui.make_position_id(); let response = ui.interact(rect, id, sense); let visuals = ui.style().interact(&response); // let text_cursor = response.rect.center() - 0.5 * galley.size; // centered-centered (looks bad for justified drop-down menus @@ -377,7 +378,6 @@ impl<'a> Widget for Checkbox<'a> { text_color, } = self; - let id = ui.make_position_id(); let text_style = TextStyle::Button; let font = &ui.fonts()[text_style]; let galley = font.layout_single_line(text); @@ -392,6 +392,7 @@ impl<'a> Widget for Checkbox<'a> { desired_size.y = desired_size.y.max(icon_width); let rect = ui.allocate_space(desired_size); + let id = ui.make_position_id(); let response = ui.interact(rect, id, Sense::click()); if response.clicked { *checked = !*checked; @@ -463,7 +464,6 @@ impl Widget for RadioButton { text, text_color, } = self; - let id = ui.make_position_id(); let text_style = TextStyle::Button; let font = &ui.fonts()[text_style]; let galley = font.layout_multiline(text, ui.available().width()); @@ -477,6 +477,7 @@ impl Widget for RadioButton { desired_size.y = desired_size.y.max(icon_width); let rect = ui.allocate_space(desired_size); + let id = ui.make_position_id(); let response = ui.interact(rect, id, Sense::click()); let text_cursor = pos2( diff --git a/egui/src/widgets/slider.rs b/egui/src/widgets/slider.rs index 38f5dccd..a55d5e82 100644 --- a/egui/src/widgets/slider.rs +++ b/egui/src/widgets/slider.rs @@ -227,9 +227,9 @@ fn x_range(rect: &Rect) -> RangeInclusive { impl<'a> Slider<'a> { /// Just the slider, no text fn allocate_slide_space(&self, ui: &mut Ui, height: f32) -> Response { - let id = ui.make_position_id(); let desired_size = vec2(ui.style().spacing.slider_width, height); let rect = ui.allocate_space(desired_size); + let id = ui.make_position_id(); ui.interact(rect, id, Sense::click_and_drag()) } diff --git a/egui/src/widgets/text_edit.rs b/egui/src/widgets/text_edit.rs index db415583..a4d50fda 100644 --- a/egui/src/widgets/text_edit.rs +++ b/egui/src/widgets/text_edit.rs @@ -105,7 +105,7 @@ impl<'t> Widget for TextEdit<'t> { let desired_width = desired_width.unwrap_or_else(|| ui.style().spacing.text_edit_width); - let id = id.unwrap_or_else(|| ui.make_child_id(id_source)); + let id = id.unwrap_or_else(|| ui.make_persistent_id(id_source)); let mut state = ui.memory().text_edit.get(&id).cloned().unwrap_or_default();