fix bindings not taking into account press order, making some bindings ambiguous

CHANGELOG.md

@@ -10,6 +10,7 @@ Game store page: https://crispypin.itch.io/marble-machinations
 - keybindings activated even when typing in a text field, making especially renaming blueprints difficult
 - grid rendering broken on right edge of the screen at some zoom levels and window sizes
 - crash when saving config if no user dir exists
+- bindings did not properly take into account order of pressing, so Shift+A and A+Shift were treated as the same thing
 - after removing a binding that was a superset of another, the remaining one did not stop being blocked by the removed ones additional modifiers until another binding was added or edited
 
 ## v0.3.2 - 2025-04-14

README.md

@@ -9,7 +9,7 @@ logic mostly like https://git.crispypin.cc/CrispyPin/marble
 - engine tests
 - blag post about marble movement logic?
 ### bugs
-- Shift+A and A+Shift conflict
+
 ### features
 #### 0.3.x
 - more levels

src/config.rs

@@ -39,6 +39,7 @@ impl Config {
 			return MenuReturn::ReturnCancel;
 		}
 
+		// self.input.update(d);
 		self.input.draw_edit(d, globals, y);
 		MenuReturn::Stay
 	}

src/input.rs

@@ -94,8 +94,9 @@ impl Default for Input {
 		bind_key(ActionId::TileGroupCompare, vec![], H);
 
 		Self {
-			bindings,
+			action_bindings: bindings,
-			states: Default::default(),
+			action_states: [BindingState::Off; ActionId::SIZE],
+			key_states: [BindingState::Off; Button::SIZE],
 			editing_binding: None,
 			in_text_edit: false,
 		}
@@ -116,8 +117,9 @@ type InputMap = BTreeMap<String, Vec<Binding>>;
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(from = "InputMap", into = "InputMap")]
 pub struct Input {
-	bindings: [Vec<Binding>; ActionId::SIZE],
+	action_bindings: [Vec<Binding>; ActionId::SIZE],
-	states: [BindingState; ActionId::SIZE],
+	action_states: [BindingState; ActionId::SIZE],
+	key_states: [BindingState; Button::SIZE],
 	editing_binding: Option<(ActionId, usize, BindingEdit)>,
 	pub in_text_edit: bool,
 }
@@ -142,10 +144,13 @@ impl Input {
 		for action_index in 0..ActionId::SIZE {
 			let action = ActionId::from_usize(action_index).unwrap();
 
+			// if self.action_states[action_index] == BindingState::Pressed {
+			// 	d.draw_rectangle(200, y, 20, 20, Color::LIMEGREEN);
+			// }
 			d.draw_text(&format!("{action:?}"), 16, y, 20, Color::ORANGE);
-			for (binding_index, binding) in self.bindings[action_index].iter().enumerate() {
+			for (binding_index, binding) in self.action_bindings[action_index].iter().enumerate() {
 				if text_button(d, &globals.mouse, buttons_x, y, 80, "remove") {
-					self.bindings[action_index].remove(binding_index);
+					self.action_bindings[action_index].remove(binding_index);
 					self.update_modifier_blocks();
 					return;
 				}
@@ -162,7 +167,7 @@ impl Input {
 				d.draw_text(&modifiers, x, y + 5, 20, Color::LIGHTBLUE);
 				x += 10 + d.measure_text(&modifiers, 20);
 				//
-				let conflicts = conflicts(&self.bindings, binding, action);
+				let conflicts = conflicts(&self.action_bindings, binding, action);
 				if !conflicts.is_empty() {
 					let conflict_text = format!("also used by: {conflicts:?}");
 					d.draw_text(&conflict_text, x, y + 5, 20, Color::ORANGERED);
@@ -176,8 +181,11 @@ impl Input {
 				y += 32;
 			}
 			if text_button(d, &globals.mouse, buttons_x, y, 130, "add binding") {
-				self.editing_binding =
+				self.editing_binding = Some((
-					Some((action, self.bindings[action_index].len(), BindingEdit::Init));
+					action,
+					self.action_bindings[action_index].len(),
+					BindingEdit::Init,
+				));
 			}
 			y += 45;
 		}
@@ -253,7 +261,7 @@ impl Input {
 				let text = format!("{:?} + {:?}", b.modifiers, b.trigger);
 				d.draw_text(&text, x + 5, y + 5, 20, colour);
 
-				let conflicts = conflicts(&self.bindings, b, *action);
+				let conflicts = conflicts(&self.action_bindings, b, *action);
 				if !conflicts.is_empty() {
 					d.draw_text(
 						&format!("conflicts: {conflicts:?}"),
@@ -266,7 +274,7 @@ impl Input {
 			}
 			if text_button(d, &globals.mouse, ok_btn_x, ok_btn_y, ok_btn_width, "ok") {
 				if let BindingEdit::Releasing(binding) = edit_state {
-					let binding_list = &mut self.bindings[*action as usize];
+					let binding_list = &mut self.action_bindings[*action as usize];
 					if *binding_index < binding_list.len() {
 						binding_list[*binding_index] = binding.clone();
 					} else {
@@ -283,20 +291,10 @@ impl Input {
 	}
 
 	pub fn update(&mut self, rl: &RaylibHandle) {
-		for i in 0..ActionId::SIZE {
+		for i in 0..Button::SIZE {
-			let bindings = &self.bindings[i];
+			let button = Button::from_usize(i).unwrap();
-			let mut is_active = false;
+			let state = &mut self.key_states[i];
-			if !self.in_text_edit {
+			*state = if button.is_down(rl) {
-				for binding in bindings {
-					if binding.modifiers.iter().all(|&m| m.is_down(rl))
-						&& !binding.blocking_modifiers.iter().any(|&m| m.is_down(rl))
-					{
-						is_active |= binding.trigger.is_down(rl);
-					}
-				}
-			}
-			let state = &mut self.states[i];
-			*state = if is_active {
 				match state {
 					BindingState::Off | BindingState::Released => BindingState::Pressed,
 					BindingState::Pressed | BindingState::Held => BindingState::Held,
@@ -308,32 +306,52 @@ impl Input {
 				}
 			}
 		}
+		for i in 0..ActionId::SIZE {
+			let bindings = &self.action_bindings[i];
+			let mut is_active = BindingState::Off;
+			if !self.in_text_edit {
+				for binding in bindings {
+					if binding
+						.modifiers
+						.iter()
+						.all(|&m| self.key_states[m as usize] == BindingState::Held)
+						&& binding
+							.blocking_modifiers
+							.iter()
+							.all(|&m| self.key_states[m as usize] == BindingState::Off)
+					{
+						is_active = is_active.or(self.key_states[binding.trigger as usize]);
+					}
+				}
+			}
+			self.action_states[i] = is_active;
+		}
 		self.in_text_edit = false;
 	}
 
 	pub fn is_pressed(&self, action: ActionId) -> bool {
-		self.states[action as usize] == BindingState::Pressed
+		self.action_states[action as usize] == BindingState::Pressed
 	}
 
 	pub fn is_held(&self, action: ActionId) -> bool {
-		self.states[action as usize] == BindingState::Pressed
+		self.action_states[action as usize] == BindingState::Pressed
-			|| self.states[action as usize] == BindingState::Held
+			|| self.action_states[action as usize] == BindingState::Held
 	}
 
 	pub fn is_released(&self, action: ActionId) -> bool {
-		self.states[action as usize] == BindingState::Released
+		self.action_states[action as usize] == BindingState::Released
 	}
 
 	/// Must be called after any binding has changed.
 	/// Ensures a binding "S" is not triggered by "Ctrl+S", if "Ctrl+S" is bound to something else.
 	pub fn update_modifier_blocks(&mut self) {
 		for i in 0..ActionId::SIZE {
-			let bindings = &self.bindings[i];
+			let bindings = &self.action_bindings[i];
 			for binding_index in 0..bindings.len() {
-				let binding = &self.bindings[i][binding_index];
+				let binding = &self.action_bindings[i][binding_index];
 				let mut blocking_mods = Vec::new();
 				for i in 0..ActionId::SIZE {
-					let other_bindings = &self.bindings[i];
+					let other_bindings = &self.action_bindings[i];
 					for other_binding in other_bindings {
 						if other_binding.trigger == binding.trigger {
 							for modifier in &other_binding.modifiers {
@@ -346,7 +364,7 @@ impl Input {
 						}
 					}
 				}
-				self.bindings[i][binding_index].blocking_modifiers = blocking_mods;
+				self.action_bindings[i][binding_index].blocking_modifiers = blocking_mods;
 			}
 		}
 	}
@@ -395,13 +413,29 @@ pub struct Binding {
 	trigger: Button,
 }
 
+impl BindingState {
+	fn or(self, rhs: Self) -> Self {
+		use BindingState::*;
+		match (self, rhs) {
+			(Off, other) => other,
+			(other, Off) => other,
+			(Held, _) => Held,
+			(_, Held) => Held,
+			(Pressed, Pressed) => Pressed,
+			(Released, Released) => Released,
+			(Pressed, Released) => Held,
+			(Released, Pressed) => Held,
+		}
+	}
+}
+
 impl From<InputMap> for Input {
 	fn from(map: InputMap) -> Self {
 		let mut new = Self::default();
 		for (action, loaded_bindings) in map {
 			let temp_json = format!("\"{action}\"");
 			if let Ok(action) = serde_json::from_str::<ActionId>(&temp_json) {
-				new.bindings[action as usize] = loaded_bindings;
+				new.action_bindings[action as usize] = loaded_bindings;
 			} else {
 				println!("'{action}' is not a valid action id, bindings discarded");
 			}
@@ -413,7 +447,7 @@ impl From<InputMap> for Input {
 impl From<Input> for InputMap {
 	fn from(value: Input) -> Self {
 		value
-			.bindings
+			.action_bindings
 			.iter()
 			.enumerate()
 			.map(|(i, b)| {