expressing context menu as an FSM instead

ezgui/src/input.rs

@@ -10,31 +10,47 @@ pub struct UserInput {
     event_consumed: bool,
     unimportant_actions: Vec<String>,
     important_actions: Vec<String>,
-
-    // While this is present, UserInput lies about anything happening.
-    // TODO Needed?
-    pub(crate) context_menu: Option<ContextMenu>,
-
     // If two different callers both expect the same key, there's likely an unintentional conflict.
     reserved_keys: HashMap<Key, String>,
+
+    // When this is active, most methods lie about having input.
+    // TODO This is hacky, but if we consume_event in things like get_moved_mouse, then canvas
+    // dragging and UI mouseover become mutex. :\
+    pub(crate) context_menu: ContextMenu,
 }
 
-pub struct ContextMenu {
+pub enum ContextMenu {
-    // We don't really need these once the Menu is present, but eh.
+    Inactive,
-    // TODO Maybe express this as a 3-state enum.
+    Building(Pt2D, BTreeMap<Key, String>),
-    pub actions: BTreeMap<Key, String>,
+    Displaying(Menu<Key>),
-    pub origin: Pt2D,
+    Clicked(Key),
+}
 
-    pub menu: Option<Menu<Key>>,
+impl ContextMenu {
-    clicked: Option<Key>,
+    pub fn maybe_build(self, canvas: &Canvas) -> ContextMenu {
+        match self {
+            ContextMenu::Building(origin, actions) => {
+                if actions.is_empty() {
+                    ContextMenu::Inactive
+                } else {
+                    ContextMenu::Displaying(Menu::new(
+                        None,
+                        actions
+                            .into_iter()
+                            .map(|(hotkey, action)| (hotkey, action, hotkey))
+                            .collect(),
+                        origin,
+                        canvas,
+                    ))
+                }
+            }
+            _ => self,
+        }
+    }
 }
 
 impl UserInput {
-    pub(crate) fn new(
+    pub(crate) fn new(event: Event, context_menu: ContextMenu, canvas: &Canvas) -> UserInput {
-        event: Event,
-        context_menu: Option<ContextMenu>,
-        canvas: &Canvas,
-    ) -> UserInput {
         let mut input = UserInput {
             event,
             event_consumed: false,
@@ -47,23 +63,26 @@ impl UserInput {
         // Create the context menu here, even if one already existed.
         if input.right_mouse_button_pressed() {
             input.event_consumed = true;
-            input.context_menu = Some(ContextMenu {
+            input.context_menu =
-                actions: BTreeMap::new(),
+                ContextMenu::Building(canvas.get_cursor_in_map_space(), BTreeMap::new());
-                origin: canvas.get_cursor_in_map_space(),
+        } else {
-                menu: None,
+            match input.context_menu {
-                clicked: None,
+                ContextMenu::Inactive => {}
-            });
+                ContextMenu::Displaying(ref mut menu) => {
-        } else if let Some(ref mut menu) = input.context_menu {
+                    // Can't call consume_event() because context_menu is borrowed.
-            // Can't call consume_event() because context_menu is borrowed.
+                    input.event_consumed = true;
-            input.event_consumed = true;
+                    match menu.event(input.event, canvas) {
-
+                        InputResult::Canceled => {
-            match menu.menu.as_mut().unwrap().event(input.event, canvas) {
+                            input.context_menu = ContextMenu::Inactive;
-                InputResult::Canceled => {
+                        }
-                    input.context_menu = None;
+                        InputResult::StillActive => {}
+                        InputResult::Done(_, hotkey) => {
+                            input.context_menu = ContextMenu::Clicked(hotkey);
+                        }
+                    }
                 }
-                InputResult::StillActive => {}
+                ContextMenu::Building(_, _) | ContextMenu::Clicked(_) => {
-                InputResult::Done(_, hotkey) => {
+                    panic!("UserInput::new given a ContextMenu in an impossible state");
-                    menu.clicked = Some(hotkey);
                 }
             }
         }
@@ -74,7 +93,7 @@ impl UserInput {
     pub fn number_chosen(&mut self, num_options: usize, action: &str) -> Option<usize> {
         assert!(num_options >= 1 && num_options <= 9);
 
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return None;
         }
 
@@ -139,7 +158,7 @@ impl UserInput {
     }
 
     pub fn key_pressed(&mut self, key: Key, action: &str) -> bool {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return false;
         }
 
@@ -159,41 +178,44 @@ impl UserInput {
     }
 
     pub fn contextual_action(&mut self, hotkey: Key, action: &str) -> bool {
-        if let Some(ref mut menu) = self.context_menu.as_mut() {
+        match self.context_menu {
-            // When the context menu is active, the event is always consumed so nothing else
+            ContextMenu::Inactive => {
-            // touches it. So don't consume or check consumption right here.
+                // If the menu's not active (the user hasn't right-clicked yet), then still allow the
-            if menu.clicked == Some(hotkey) {
+                // legacy behavior of just pressing the hotkey.
-                self.context_menu = None;
+                return self.key_pressed(hotkey, &format!("CONTEXTUAL: {}", action));
-                return true;
             }
-
+            ContextMenu::Building(_, ref mut actions) => {
-            // We could be initially populating the menu because the user just right-clicked, or
+                // The event this round was the right click, so don't check if the right keypress
-            // this could be a later round.
+                // happened.
-            if let Some(prev_action) = menu.actions.get(&hotkey) {
+                if let Some(prev_action) = actions.get(&hotkey) {
-                if prev_action != action {
+                    if prev_action != action {
-                    panic!(
+                        panic!(
-                        "Context menu uses hotkey {:?} for both {} and {}",
+                            "Context menu uses hotkey {:?} for both {} and {}",
-                        hotkey, prev_action, action
+                            hotkey, prev_action, action
-                    );
+                        );
+                    }
+                } else {
+                    actions.insert(hotkey, action.to_string());
                 }
-            } else {
-                menu.actions.insert(hotkey, action.to_string());
             }
-
+            ContextMenu::Displaying(_) => {
-            if self.event == Event::KeyPress(hotkey) {
+                if self.event == Event::KeyPress(hotkey) {
-                self.context_menu = None;
+                    self.context_menu = ContextMenu::Inactive;
-                return true;
+                    return true;
+                }
+            }
+            ContextMenu::Clicked(key) => {
+                if key == hotkey {
+                    self.context_menu = ContextMenu::Inactive;
+                    return true;
+                }
             }
-            false
-        } else {
-            // If the menu's not active (the user hasn't right-clicked yet), then still allow the
-            // legacy behavior of just pressing the hotkey.
-            self.key_pressed(hotkey, &format!("CONTEXTUAL: {}", action))
         }
+        false
     }
 
     pub fn unimportant_key_pressed(&mut self, key: Key, action: &str) -> bool {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return false;
         }
 
@@ -213,7 +235,7 @@ impl UserInput {
     }
 
     pub fn key_released(&mut self, key: Key) -> bool {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return false;
         }
 
@@ -230,26 +252,26 @@ impl UserInput {
 
     // No consuming for these?
     pub(crate) fn left_mouse_button_pressed(&mut self) -> bool {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return false;
         }
         self.event == Event::LeftMouseButtonDown
     }
     pub(crate) fn left_mouse_button_released(&mut self) -> bool {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return false;
         }
         self.event == Event::LeftMouseButtonUp
     }
     pub(crate) fn right_mouse_button_pressed(&mut self) -> bool {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return false;
         }
         self.event == Event::RightMouseButtonDown
     }
 
     pub fn get_moved_mouse(&self) -> Option<(f64, f64)> {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return None;
         }
 
@@ -260,7 +282,7 @@ impl UserInput {
     }
 
     pub(crate) fn get_mouse_scroll(&self) -> Option<f64> {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return None;
         }
 
@@ -271,7 +293,7 @@ impl UserInput {
     }
 
     pub fn is_update_event(&mut self) -> bool {
-        if self.context_menu.is_some() {
+        if self.context_menu_active() {
             return false;
         }
 
@@ -318,4 +340,11 @@ impl UserInput {
         }
         self.reserved_keys.insert(key, action.to_string());
     }
+
+    fn context_menu_active(&self) -> bool {
+        match self.context_menu {
+            ContextMenu::Inactive => false,
+            _ => true,
+        }
+    }
 }

ezgui/src/runner.rs

@@ -1,5 +1,4 @@
 use crate::input::ContextMenu;
-use crate::menu::Menu;
 use crate::{Canvas, Event, GfxCtx, UserInput};
 use glutin_window::GlutinWindow;
 use opengl_graphics::{GlGraphics, OpenGL};
@@ -34,7 +33,7 @@ pub fn run<T, G: GUI<T>>(mut gui: G, window_title: &str, initial_width: u32, ini
     let mut gl = GlGraphics::new(opengl);
 
     let mut last_event_mode = EventLoopMode::InputOnly;
-    let mut context_menu: Option<ContextMenu> = None;
+    let mut context_menu = ContextMenu::Inactive;
     let mut last_data: Option<T> = None;
     while let Some(ev) = events.next(&mut window) {
         use piston::input::RenderEvent;
@@ -54,11 +53,8 @@ pub fn run<T, G: GUI<T>>(mut gui: G, window_title: &str, initial_width: u32, ini
                     }
 
                     // Always draw the context-menu last.
-                    if let Some(ref menu) = context_menu {
+                    if let ContextMenu::Displaying(ref menu) = context_menu {
-                        menu.menu
+                        menu.draw(&mut g, gui.get_mut_canvas());
-                            .as_ref()
-                            .unwrap()
-                            .draw(&mut g, gui.get_mut_canvas());
                     }
                 });
             }
@@ -94,24 +90,7 @@ pub fn run<T, G: GUI<T>>(mut gui: G, window_title: &str, initial_width: u32, ini
                     }
                 };
             last_data = Some(data);
-            context_menu = input.context_menu;
+            context_menu = input.context_menu.maybe_build(gui.get_mut_canvas());
-            if let Some(ref mut menu) = context_menu {
-                if menu.menu.is_none() {
-                    if menu.actions.is_empty() {
-                        context_menu = None;
-                    } else {
-                        menu.menu = Some(Menu::new(
-                            None,
-                            menu.actions
-                                .iter()
-                                .map(|(hotkey, action)| (*hotkey, action.clone(), *hotkey))
-                                .collect(),
-                            menu.origin,
-                            gui.get_mut_canvas(),
-                        ));
-                    }
-                }
-            }
 
             // Don't constantly reset the events struct -- only when laziness changes.
             if new_event_mode != last_event_mode {