Input rebind patch #247

Xaeroxe · Jun 17, 2017

I'm working on enhancements to the input system and adding support for key rebinding. I'm opening this pull request now to start gathering feedback.

~~The only file modified in any meaningful way in this PR so far is src/ecs/resources/input.rs everything else is just whitespace changes.~~

Whitespace changes have been removed.

Xaeroxe · Jun 17, 2017

~~The only file modified in any meaningful way in this PR so far is src/ecs/resources/input.rs everything else is just whitespace changes.~~

Whitespace changes have been removed.

torkleyy · Jun 17, 2017

torkleyy reviewed Jun 17, 2017

View changes

Looks nice, here just a few first thoughts.

torkleyy · Jun 17, 2017

src/ecs/resources/input.rs

@@ -62,45 +80,63 @@ impl<'a> Iterator for PressedMouseButtons<'a> {
 /// ```
 #[derive(Default)]
 pub struct InputHandler {
-    pressed_keys: HashMap<VirtualKeyCode, KeyQueryState>,
-    pressed_mouse_buttons: HashMap<MouseButton, KeyQueryState>,
+    pressed_keys: Vec<VirtualKeyCode>,


It would be great if we could instead just have a boolean Vec where the index is the key. Also see this issue.

I had similar thoughts, I initially wanted to store a fixed size array of bools as part of the struct, where each VirtualKeyCode (or MouseButton) as usize is the index into the array. This reduces the amount of times we have to follow pointers and do allocations, but determining what that fixed size should be is a bit hard as there isn't an easy way to get a count of all the variants of a VirtualKeyCode. I could just hard code a value but this makes for some brittle code if new codes are ever added. Additionally every frame the current input state needs to be copied to the previous input state as well, so if the input state structures are larger this gets more expensive. There are also limitations on how large this Vec<VirtualKeyCode> can get, most of the time it will have < 5 elements, so iterating it to determine key state isn't that expensive, especially when compared to the cost of copying an entire Vec<bool> of all keys every frame.

torkleyy · Jun 17, 2017

src/ecs/resources/input.rs

+    }
+}
+
+fn mouse_button_to_button(mb: &MouseButton) -> Button {


Could we have that as an Into implementation?

Sure! I'll make one.

It's been added!

torkleyy · Jun 17, 2017

src/ecs/resources/input.rs

+    }
+
+    /// Returns the value of an axis by the i32 id, if the id doesn't exist this returns None.
+    pub fn axis_value(&self, id: i32) -> Option<f32> {


I see, so this is how you would refer to an axis defined in the config file?

Eventually yes, I haven't quite fleshed out the rebinding part yet

This PR is as finished as it can be until #244 is merged.

Changelog entry:

Add function text_entered which returns a &str of characters sent to the game window during the current frame, useful for in game text entry fields.
Add _released functions to detect when an input is released
All _down functions have been renamed to _pressed
All _once functions have been renamed to _down, and they all will report the correct values even if they've already been called once on this frame.
Button type has been added, which is an enum containing the VirtualKeyCode and MouseButton types
Functions to compliment Button have been added,
- pub fn pressed_buttons(&self) -> PressedButtons
- pub fn button_pressed(&self, button: Button) -> bool
- pub fn buttons_pressed(&self, buttons: &[Button]) -> bool
- pub fn button_down(&self, button: Button) -> bool
- pub fn button_released(&self, button: Button) -> bool
- pub fn buttons_down(&self, buttons: &[Button]) -> bool
Actions have been added, which are mappings between Buttons and i32 values, allowing for rebinding of inputs.
Similar functions for actions were added, with the exception that there is no pressed_actions. The function was difficult to write and I couldn't think of a use case for it.
Axis struct added, which is a mapping of twoButtons, one positive and one negative.
Axes can be mapped to i32s on the InputHandler and then queried for with the axis_value function.

Xaeroxe · Jun 17, 2017

This PR is as finished as it can be until #244 is merged.

Changelog entry:

Add function text_entered which returns a &str of characters sent to the game window during the current frame, useful for in game text entry fields.
Add _released functions to detect when an input is released
All _down functions have been renamed to _pressed
All _once functions have been renamed to _down, and they all will report the correct values even if they've already been called once on this frame.
Button type has been added, which is an enum containing the VirtualKeyCode and MouseButton types
Functions to compliment Button have been added,
- pub fn pressed_buttons(&self) -> PressedButtons
- pub fn button_pressed(&self, button: Button) -> bool
- pub fn buttons_pressed(&self, buttons: &[Button]) -> bool
- pub fn button_down(&self, button: Button) -> bool
- pub fn button_released(&self, button: Button) -> bool
- pub fn buttons_down(&self, buttons: &[Button]) -> bool
Actions have been added, which are mappings between Buttons and i32 values, allowing for rebinding of inputs.
Similar functions for actions were added, with the exception that there is no pressed_actions. The function was difficult to write and I couldn't think of a use case for it.
Axis struct added, which is a mapping of twoButtons, one positive and one negative.
Axes can be mapped to i32s on the InputHandler and then queried for with the axis_value function.

kvark · Jun 17, 2017

kvark requested changes Jun 17, 2017

View changes

kvark · Jun 17, 2017

src/ecs/resources/input.rs

+    //TODO: Add controller buttons here when the engine has support.
+}
+
+impl Into<Button> for VirtualKeyCode {


I think it's more idiomatic to have impl From<VirtualKeyCode> for Button

Ah you're right, thanks.

kvark · Jun 17, 2017

src/asset_manager/asset_manager.rs

-                                            }
-                                            None => [0.0, 0.0],
-                                        },
+                        geometry


I don't think mixing your input PR with rustfmt pass is a good idea. Better have them separate. FWIW, I like the old formatting better.

I asked @ebkalderon about mixing this with rustfmt and he didn't seem to mind, however in this specific instance I just realized it's very likely to have a merge conflict with #244 and I'd rather not have that. So I'll remove the other files from this PR.

kvark · Jun 17, 2017

src/ecs/resources/input.rs

@@ -28,7 +41,7 @@ impl<'a> Iterator for PressedKeys<'a> {
 
 /// An iterator over the currently pressed down mouse buttons.
 pub struct PressedMouseButtons<'a> {
-    iterator: Keys<'a, MouseButton, KeyQueryState>,
+    iterator: Iter<'a, MouseButton>,


is there any benefit over doing just type PressedMouseButtons<'a> = Iter<'a, MouseButton>?

This conceals the implementation details a bit better, but other than that no not really. When I opened the file it was a wrapper struct so I left it that way. I'm happy to change it either way.

kvark · Jun 17, 2017

src/ecs/resources/input.rs

     // defined to match glium
     mouse_position: Option<(i32, i32)>,
     previous_mouse_position: Option<(i32, i32)>,
+    axes: HashMap<i32, Axis>,


I assume these i32 are specific indices of the elements, in which case it would be a good idea to make them opaque (i.e. struct AxeId(i32)) so that the user is not able to change them

Good note, I'll do that.

On second thought it'd probably be better for this to be transparent right now, as the facilities for making input rebinding part of the asset system don't exist yet. I'll probably see if I can do this again when #244 is merged.

kvark · Jun 17, 2017

src/ecs/resources/input.rs

+        // Before processing these events store the input states of the previous frame.
+        self.previous_mouse_position = self.mouse_position;
+        self.previous_pressed_keys = self.pressed_keys.clone();
+        self.previous_pressed_mouse_buttons = self.pressed_mouse_buttons.clone();


nit: should re-use the existing allocation instead of cloning, i.e.

vector.clear(); vector.extend_from_slice(&other_vector);

0_o TIL a cool new trick, thanks!

kvark · Jun 17, 2017

src/ecs/resources/input.rs

-                    // I.e `key_once(key)` is queried after second `Pressed` event.
-                    if let Entry::Vacant(entry) = self.pressed_keys.entry(key_code) {
-                        entry.insert(KeyQueryState::NotQueried);
+                    if self.pressed_keys.iter().all(|&k| k != key_code) {


I wonder if a HashSet would be more convenient here

It might be, although I think the performance of Vec might actually be better as iterating over <5 elements in contiguous memory is probably faster than any hashing function

Right, but I don't think it's even worth optimizing if there are just 5 keys. HashSet is just more convenient.

kvark · Jun 17, 2017

src/ecs/resources/input.rs

+    pub fn actions_pressed(&self, actions: &[i32]) -> Result<bool, Vec<i32>> {
+        let mut all_buttons_pressed = true;
+        //Prefer to skip initializing a new vec if we can.
+        let mut bad_values: Option<Vec<i32>> = None;


Vec::new() costs absolutely nothing, no need to wrap it into Option

Cool! Thanks!

kvark · Jun 17, 2017

src/ecs/resources/input.rs

+            }
+        }
+        if let Some(values) = bad_values {
+            return Err(values);


no need for return statements here

Yeah I guess it's more of a stylistic choice. I'll remove them though.

kvark · Jun 17, 2017

src/ecs/resources/input.rs

+        let mut all_buttons_pressed = true;
+        let mut any_button_pressed_this_frame = false;
+        //Prefer to skip initializing a new vec if we can.
+        let mut bad_values: Option<Vec<i32>> = None;


same here as above

kvark · Jun 18, 2017

kvark approved these changes Jun 18, 2017

View changes

I don't think HashSet is essential here (we can change it later, it's an impl detail anyway), so I'd be ready to merge this. Unless there is something else you want to do in the PR, or other reviewers want to chime in.

kvark · Jun 18, 2017

I don't think HashSet is essential here (we can change it later, it's an impl detail anyway), so I'd be ready to merge this. Unless there is something else you want to do in the PR, or other reviewers want to chime in.

As of this moment I consider this merge ready. If any other reviewers want to chime in I'll be happy to hear what they have to say.

Xaeroxe · Jun 18, 2017

As of this moment I consider this merge ready. If any other reviewers want to chime in I'll be happy to hear what they have to say.

Aceeri · Jun 18, 2017

Aceeri approved these changes Jun 18, 2017 • edited

View changes

Looks good to me. Would this also give the benefit of keys pressed in a specific order? As in you could have something based on whether A->B was pressed or if B->A was pressed or would that depend more on glutin's input handling?

@Aceeri This API doesn't store key press ordering in any way, in order to accomplish that you'd need some kind of state machine, which could be powered by the functions given here but no this won't handle the order in which keys are pressed. I'd recommend storing that information in the state of your components, as it's probably much better suited to that in my opinion. That way you can also have your visuals and audio respond to keys being pressed in a specific order. For example:

Press A -> UI icon flashes and changes what image is displayed, accompanied by some kind of "acknowledged" sound effect
Press B -> Action executes, UI icon switches back

Press B -> UI icon flashes and changes to a different image than it would if A were pressed, also accompanied by some kind of "acknowledged" sound effect
Press A -> Action executes, UI icon switches back

Xaeroxe · Jun 18, 2017

@Aceeri This API doesn't store key press ordering in any way, in order to accomplish that you'd need some kind of state machine, which could be powered by the functions given here but no this won't handle the order in which keys are pressed. I'd recommend storing that information in the state of your components, as it's probably much better suited to that in my opinion. That way you can also have your visuals and audio respond to keys being pressed in a specific order. For example:

Press A -> UI icon flashes and changes what image is displayed, accompanied by some kind of "acknowledged" sound effect
Press B -> Action executes, UI icon switches back

Press B -> UI icon flashes and changes to a different image than it would if A were pressed, also accompanied by some kind of "acknowledged" sound effect
Press A -> Action executes, UI icon switches back

@Xaeroxe How should we handle text input (e.g. for an in-game chat)?

torkleyy · Jun 18, 2017

@Xaeroxe How should we handle text input (e.g. for an in-game chat)?

@torkleyy That's a great question. This API is awful for that currently. I'd like to change that. Please don't merge until I have that in.

Xaeroxe · Jun 18, 2017

@torkleyy That's a great question. This API is awful for that currently. I'd like to change that. Please don't merge until I have that in.

@torkleyy function text_entered has been added, which returns a &str of all characters sent to the window during the current frame, you can insert the output of that function wherever your cursor is in your text entry field.

I consider this ready to merge again.

Xaeroxe · Jun 18, 2017

@torkleyy function text_entered has been added, which returns a &str of all characters sent to the window during the current frame, you can insert the output of that function wherever your cursor is in your text entry field.

I consider this ready to merge again.

Also as an aside: This won't handle for backspace and delete (backspace and delete "characters" will likely be directly in the output of text_entered). Full text entry support is more of a UI problem and in my opinion slightly out of scope for this, but at least now we have the facilities to add full text entry support at a later date.

Xaeroxe · Jun 18, 2017

Also as an aside: This won't handle for backspace and delete (backspace and delete "characters" will likely be directly in the output of text_entered). Full text entry support is more of a UI problem and in my opinion slightly out of scope for this, but at least now we have the facilities to add full text entry support at a later date.

msiglreith · Jun 18, 2017

msiglreith suggested changes Jun 18, 2017

View changes

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

 }
 
 impl InputHandler {
     /// Creates a new input handler.
     pub fn new() -> InputHandler {
         InputHandler {
-            pressed_keys: HashMap::default(),
-            pressed_mouse_buttons: HashMap::default(),
+            pressed_keys: Vec::new(),


Abstracting pressed_* and previous_* into a separate struct could reduce code duplication.

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

     }
 
     /// Checks if all the given keys are being pressed.
-    pub fn keys_down(&self, keys: &[VirtualKeyCode]) -> bool {
+    pub fn keys_pressed(&self, keys: &[VirtualKeyCode]) -> bool {


keys_pressed and key_pressed seem to have a different behavior as the first one uses key_down internally.

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

-        keys.iter().any(|key| self.key_once(*key)) && self.keys_down(keys)
+    /// Checks if the all the given keys are down and at least one was pressed on this frame.
+    pub fn keys_down(&self, keys: &[VirtualKeyCode]) -> bool {
+        keys.iter().any(|key| self.key_down(*key)) && self.keys_down(keys)


This looks like an infinite recursion.

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

     }
 
     /// Checks if all the given mouse buttons are being pressed.
-    pub fn mouse_buttons_down(&self, buttons: &[MouseButton]) -> bool {
+    pub fn mouse_buttons_pressed(&self, buttons: &[MouseButton]) -> bool {


(Same as above)

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

-        buttons.iter().any(|btn| self.mouse_button_once(*btn)) && self.mouse_buttons_down(buttons)
+    /// Checks if the all the given mouse buttons are down and at least one was pressed this frame.
+    pub fn mouse_buttons_down(&self, buttons: &[MouseButton]) -> bool {
+        buttons.iter().any(|&btn| self.mouse_button_down(btn)) && self.mouse_buttons_down(buttons)


(Same as above, recursion)

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

+                bad_values.push(*action);
+            }
+        }
+        if bad_values.len() > 0 {


nit: !bad_values.is_empty()

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

+                bad_values.push(*action);
+            }
+        }
+        if bad_values.len() > 0 {


(Same as above)

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

+        if let Some(a) = self.axes.get(&id) {
+            let pos = self.button_down(a.pos);
+            let neg = self.button_down(a.neg);
+            if pos == neg {


Some( if pos == neg { .. } else if ... ) might be nicer here

Some(if pos == neg { 0.0 } else if pos { 1.0 } else { -1.0 })

This is the rustfmt version of that. In my opinion it really hurts readability.

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

+    pub fn action_down(&self, action: i32) -> Option<bool> {
+        self.actions
+            .get(&action)
+            .and_then(|&b| Some(self.button_down(b)))


map(|&b| self.button_down(b))) should do the same I think

msiglreith · Jun 18, 2017

src/ecs/resources/input.rs

+    // can't think of a use case for it.
+
+    /// Checks if the given button is currently pressed.
+    pub fn action_pressed(&self, action: i32) -> Option<bool> {


yay, actions!

Oh man @msiglreith found lots of problematic typos. Thanks a ton @msiglreith I'll get to work on your comments.

Xaeroxe · Jun 18, 2017

Oh man @msiglreith found lots of problematic typos. Thanks a ton @msiglreith I'll get to work on your comments.

Code has been rearranged to have specific InputFrameData structures, this is nice because it more clearly separates where the functionality lives and makes my code less error prone.

Ready for review.

Xaeroxe · Jun 18, 2017

Code has been rearranged to have specific InputFrameData structures, this is nice because it more clearly separates where the functionality lives and makes my code less error prone.

Ready for review.

@msiglreith since all your comments are outdated I'm giving you a ❤️ here, thanks for catching all the typos I made while tired.

Xaeroxe · Jun 18, 2017

@msiglreith since all your comments are outdated I'm giving you a ❤️ here, thanks for catching all the typos I made while tired.

Just added _released functions to detect when an input is released.

Xaeroxe · Jun 18, 2017

Just added _released functions to detect when an input is released.

I skipped having functions like keys_released because the behavior of that would be pretty wonky and hard to predict. It'd only be reasonable to check if any of the given keys were released this frame, which is not how it's opposite keys_down behaves. You can make keys_down work because it relies on action rather than inaction, but keys_released would rely on inaction making it a bit confusing.

Xaeroxe · Jun 18, 2017

I skipped having functions like keys_released because the behavior of that would be pretty wonky and hard to predict. It'd only be reasonable to check if any of the given keys were released this frame, which is not how it's opposite keys_down behaves. You can make keys_down work because it relies on action rather than inaction, but keys_released would rely on inaction making it a bit confusing.

ebkalderon · Jun 19, 2017

ebkalderon requested changes Jun 19, 2017

View changes

Terrific work all around, @Xaeroxe! Just a few nits to handle before merging.

ebkalderon · Jun 19, 2017

src/ecs/resources/input.rs

+        }
+    }
+
+    fn advance_frame(&mut self, previous_frame: &mut FrameInputData) {


Missing a doc comment here.

ebkalderon · Jun 19, 2017

src/ecs/resources/input.rs

+    }
+
+    // Pressed actions has been omitted as the function is a little difficult to write and I
+    // can't think of a use case for it.


Given that action_pressed() is present below, this comment seems somewhat misleading.

Maybe this is a good reason to rename these functions. This comment is actually referring to pressed_actions, with pressed_ in front and an s on actions. It's a different operation from action_pressed. See key_pressed vs pressed_keys for a comparison.

Oh, I see what you mean now. Sorry, the terminology is a bit confusing.

I just renamed the functions to
pressed_keys()
key_is_pressed()
keys_are_pressed()

I think that helps disambiguate it a bit better.

Sounds great! Thank you.

ebkalderon · Jun 19, 2017

src/ecs/resources/input.rs

+        let mouse_button_convert = mouse_button_to_button as fn(&MouseButton) -> Button;
+        let key_convert = key_to_button as fn(&VirtualKeyCode) -> Button;
+        let mouse_buttons = self.pressed_mouse_buttons().map(mouse_button_convert);
+        let keys = self.pressed_keys().map(key_convert);


Perhaps you could use closures here instead of free-standing functions?

I tried, it seems rustc doesn't support casting closures to fn types yet. I spent a few hours trying to get this to work and this was the best I could do. Storing functions in types without using trait objects or generics turns out to be a bit of a sore spot for Rust currently.

I meant passing closures into the map() methods directly, rather than storing them as types.

I sincerely would love to do this, but this is the compiler error I get:

jacob@jacob-laptop ~/amethyst $ cargo check Compiling amethyst v0.4.3 (file:///home/jacob/amethyst) error: non-capturing closure to fn coercion is experimental (see issue #39817) --> src/ecs/resources/input.rs:139:36 | 139 | let mouse_button_convert = (|&mb| Button::Mouse(mb)) as fn(&MouseButton) -> Button; | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: non-scalar cast: `[closure@src/ecs/resources/input.rs:139:36: 139:61]` as `fn(&glutin::MouseButton) -> ecs::resources::input::Button` --> src/ecs/resources/input.rs:139:36 | 139 | let mouse_button_convert = (|&mb| Button::Mouse(mb)) as fn(&MouseButton) -> Button; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: non-capturing closure to fn coercion is experimental (see issue #39817) --> src/ecs/resources/input.rs:140:27 | 140 | let key_convert = (|&k| Button::Key(k)) as fn(&VirtualKeyCode) -> Button; | ^^^^^^^^^^^^^^^^^^^^^ error: non-scalar cast: `[closure@src/ecs/resources/input.rs:140:27: 140:48]` as `fn(&glutin::VirtualKeyCode) -> ecs::resources::input::Button` --> src/ecs/resources/input.rs:140:27 | 140 | let key_convert = (|&k| Button::Key(k)) as fn(&VirtualKeyCode) -> Button; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 4 previous errors error: Could not compile `amethyst`. To learn more, run the command again with --verbose.

These fns have to be stored in the PressedButtons iterator that is passed back so it is necessary to define a type for them. I could just Box the iterator but then we're moving this structure onto the heap unnecessarily.

Basically this whole system ends up being a type system jigsaw puzzle and this was the only way I could see to solve it and maintain performance.

I'm sorry, perhaps I'm not being very clear. I meant you could do something like:

let mouse_buttons = self.pressed_mouse_buttons().map(|mb| Button::Mouse(mb)); let keys = self.pressed_keys().map(|k| Button::Key(k));

Tried that too, with that you get

jacob@jacob-laptop ~/amethyst $ cargo check Compiling amethyst v0.4.3 (file:///home/jacob/amethyst) error[E0308]: mismatched types --> src/ecs/resources/input.rs:141:36 | 141 | PressedButtons { iterator: mouse_buttons.chain(keys) } | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found closure | = note: expected type `std::iter::Chain<std::iter::Map<ecs::resources::input::PressedMouseButtons<'_>, fn(&glutin::MouseButton) -> ecs::resources::input::Button>, std::iter::Map<ecs::res ources::input::PressedKeys<'_>, fn(&glutin::VirtualKeyCode) -> ecs::resources::input::Button>>` found type `std::iter::Chain<std::iter::Map<ecs::resources::input::PressedMouseButtons<'_>, [closure@src/ecs/resources/input.rs:139:62: 139:85]>, std::iter::Map<ecs::resources ::input::PressedKeys<'_>, [closure@src/ecs/resources/input.rs:140:44: 140:63]>>` error: aborting due to previous error error: Could not compile `amethyst`. To learn more, run the command again with --verbose.

I spent hours trying to jam a closure in there and I couldn't see a way to do it. I'm sorry if I come off flippant, it's just the majority of the time spent on this PR was trying to get this piece of code to compile.

Relevant rustc issue: rust-lang/rust#39817

It seems that we'll be able to correct this code in a very short time, as this has been stabilized in the master branch of rustc.

@Xaeroxe No problem at all; I was just curious. 😁 Thank you for all your effort!

ebkalderon · Jun 19, 2017

src/ecs/resources/input.rs

+    pub neg: Button,
+}
+
+#[derive(Default)]


Missing doc comment here.

ebkalderon · Jun 19, 2017

src/ecs/resources/input.rs

+    }
+}
+
+pub struct Axis {


Missing doc comment here.

@ebkalderon ready for your review again

Xaeroxe · Jun 19, 2017

@ebkalderon ready for your review again

Woops, closing was a misclick, my bad.

Xaeroxe · Jun 19, 2017

Woops, closing was a misclick, my bad.

ebkalderon · Jun 19, 2017

ebkalderon approved these changes Jun 19, 2017

View changes

@Xaeroxe Thank you so much for addressing all my comments. LGTM!

ebkalderon · Jun 19, 2017

@Xaeroxe Thank you so much for addressing all my comments. LGTM!

Excellent! So what's our usual procedure for merging? It's gotten approval from 3 collaborators, so I think it's ready now.

Xaeroxe · Jun 19, 2017

Excellent! So what's our usual procedure for merging? It's gotten approval from 3 collaborators, so I think it's ready now.

Aside from pinging @torkleyy that he should probably rebase #244 to the latest upstream, nothing else. Let's merge this now.

ebkalderon · Jun 19, 2017

Aside from pinging @torkleyy that he should probably rebase #244 to the latest upstream, nothing else. Let's merge this now.

Xaeroxe referenced this pull request Jun 17, 2017
Closed
Nitro vs Amethyst comparison #245

Xaeroxe changed the title from [WIP] Input rebind patch to Input rebind patch Jun 17, 2017

kvark referenced this pull request in three-rs/three Jun 17, 2017
Closed
Input format #8

Add input rebinding

Loading status checks…

12899d7

Xaeroxe added some commits Jun 18, 2017

Correct lots of typos.

Loading status checks…

f4b0568

Rewrite axis_value to be cleaner.

Loading status checks…

2b33363

Add _released functions to compliment _down functions.

Loading status checks…

0396df1

Remove outdated inline comment

Loading status checks…

e5e44fa

Xaeroxe added some commits Jun 19, 2017

Rename functions to be less confusing.

Loading status checks…

0e14916

Add missing doc comments

Loading status checks…

e75bd9d

Xaeroxe closed this Jun 19, 2017

Xaeroxe reopened this Jun 19, 2017

ebkalderon merged commit 1cc906d into amethyst:develop Jun 19, 2017
1 of 2 checks passed

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

nchashch referenced this pull request Jun 19, 2017
Closed
Release 0.5.0 #241
13 of 13 tasks complete

Xaeroxe referenced this pull request Jun 23, 2017
Closed
Input manager #225
0 of 2 tasks complete

Xaeroxe deleted the Xaeroxe:input-rebind-patch branch Jun 23, 2017

Xaeroxe referenced this pull request Jun 23, 2017
Closed
Map input devices to actions #177

amethyst/amethyst

Join GitHub today

Input rebind patch #247

Conversation

Xaeroxe commented Jun 17, 2017

This comment has been minimized.

Xaeroxe commented Jun 17, 2017 • edited Edited 1 time Xaeroxe edited Jun 18, 2017 (most recent)

torkleyy reviewed Jun 17, 2017 View changes

This comment has been minimized.

This comment has been minimized.

Xaeroxe Jun 17, 2017 • edited Edited 1 time Xaeroxe edited Jun 17, 2017 (most recent)

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Xaeroxe referenced this pull request Jun 17, 2017

This comment has been minimized.

Xaeroxe commented Jun 17, 2017 • edited Edited 1 time Xaeroxe edited Jun 18, 2017 (most recent)

Xaeroxe changed the title from [WIP] Input rebind patch to Input rebind patch Jun 17, 2017

kvark referenced this pull request in three-rs/three Jun 17, 2017

kvark requested changes Jun 17, 2017 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark approved these changes Jun 18, 2017 View changes

This comment has been minimized.

kvark commented Jun 18, 2017

This comment has been minimized.

Xaeroxe commented Jun 18, 2017

Aceeri approved these changes Jun 18, 2017 • edited View changes

This comment has been minimized.

Xaeroxe commented Jun 18, 2017 • edited Edited 1 time Xaeroxe edited Jun 18, 2017 (most recent)

This comment has been minimized.

torkleyy commented Jun 18, 2017

This comment has been minimized.

Xaeroxe commented Jun 18, 2017 • edited Edited 1 time Xaeroxe edited Jun 18, 2017 (most recent)

This comment has been minimized.

Xaeroxe commented Jun 18, 2017

This comment has been minimized.

Xaeroxe commented Jun 18, 2017 • edited Edited 1 time Xaeroxe edited Jun 18, 2017 (most recent)

msiglreith suggested changes Jun 18, 2017 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Xaeroxe commented Jun 17, 2017 •

edited

Edited 1 time

Xaeroxe edited Jun 18, 2017 (most recent)

torkleyy reviewed Jun 17, 2017

View changes

Xaeroxe Jun 17, 2017 •

edited

Edited 1 time

Xaeroxe edited Jun 17, 2017 (most recent)

Xaeroxe commented Jun 17, 2017 •

edited

Edited 1 time

Xaeroxe edited Jun 18, 2017 (most recent)

kvark requested changes Jun 17, 2017

View changes

kvark approved these changes Jun 18, 2017

View changes

Aceeri approved these changes Jun 18, 2017 • edited

View changes

Xaeroxe commented Jun 18, 2017 •

edited

Edited 1 time

Xaeroxe edited Jun 18, 2017 (most recent)

Xaeroxe commented Jun 18, 2017 •

edited

Edited 1 time

Xaeroxe edited Jun 18, 2017 (most recent)

Xaeroxe commented Jun 18, 2017 •

edited

Edited 1 time

Xaeroxe edited Jun 18, 2017 (most recent)

msiglreith suggested changes Jun 18, 2017

View changes