New input API by kvark · Pull Request #33 · three-rs/three

kvark · 2017-06-19T21:07:02Z

Closes #8

vitvakatu

Only some minor notes

vitvakatu · 2017-06-19T21:46:08Z

src/input.rs

+#[allow(dead_code)]
+struct InputDiff {
+    time_delta: TimerDuration,
+    //time_delta_focused: time::Duration,


Do we need this?

removed now

vitvakatu · 2017-06-19T21:49:49Z

src/input.rs

+#[derive(Clone, Copy, Debug, PartialEq)]
+pub struct Timer {
+    start: time::Instant,
+    //focus_only: bool,


vitvakatu · 2017-06-19T21:53:34Z

src/input.rs

+impl Button {
+    pub fn count_hits(&self, input: &Input) -> u8 {
+        match *self {
+            Button::Key(button) => input.1.keys_hit.iter().filter(|&&key| key == button).take(0xFF).count() as u8,


Any reason to use hex number instead of decimal?
Also, maybe we should use constant for this?

vitvakatu · 2017-06-19T21:56:00Z

src/input.rs

+}
+
+pub const AXIS_LEFT_RIGHT: KeyAxis = KeyAxis{ neg: Key::Left, pos: Key::Right };
+pub const AXIS_DOWN_UP: KeyAxis = KeyAxis{ neg: Key::Down, pos: Key::Up };


It seems irregular for me. Maybe AXIS_UP_DOWN? People usually look from the top to the bottom.

The axis go from DOWN (negative) to UP (positive), hence the name AXIS_DOWN_UP, which is consistent with AXIS_LEFT_RIGHT.

vitvakatu · 2017-06-19T21:57:43Z

src/input.rs

@@ -0,0 +1,172 @@
+#![allow(missing_docs)] //TODO


I'll try to fix it asap.

thanks, and no rush!

kvark

Thanks!

kvark · 2017-06-20T01:28:33Z

src/input.rs

@@ -0,0 +1,172 @@
+#![allow(missing_docs)] //TODO


thanks, and no rush!

kvark · 2017-06-20T01:28:40Z

src/input.rs

+#[allow(dead_code)]
+struct InputDiff {
+    time_delta: TimerDuration,
+    //time_delta_focused: time::Duration,


removed now

kvark · 2017-06-20T01:30:01Z

src/input.rs

+impl Button {
+    pub fn count_hits(&self, input: &Input) -> u8 {
+        match *self {
+            Button::Key(button) => input.1.keys_hit.iter().filter(|&&key| key == button).take(0xFF).count() as u8,


kvark · 2017-06-20T01:32:55Z

src/input.rs

+}
+
+pub const AXIS_LEFT_RIGHT: KeyAxis = KeyAxis{ neg: Key::Left, pos: Key::Right };
+pub const AXIS_DOWN_UP: KeyAxis = KeyAxis{ neg: Key::Down, pos: Key::Up };


The axis go from DOWN (negative) to UP (positive), hence the name AXIS_DOWN_UP, which is consistent with AXIS_LEFT_RIGHT.

kvark · 2017-06-20T01:35:12Z

Comments addressed, unleashing bors.
bors r+

@vitvakatu

33: New input API r=kvark Closes #8 r? @vitvakatu

bors · 2017-06-20T01:49:09Z

Build succeeded

continuous-integration/travis-ci/push

@vitvakatu

33: New input API r=kvark Closes #8 r? @vitvakatu

vitvakatu requested changes Jun 19, 2017

View reviewed changes

kvark commented Jun 20, 2017

View reviewed changes

New input API

bf87746

kvark force-pushed the input branch from 3c5f6cd to bf87746 Compare June 20, 2017 01:33

bors bot added a commit that referenced this pull request Jun 20, 2017

Merge #33

f629eb3

33: New input API r=kvark Closes #8 r? @vitvakatu

bors bot merged commit bf87746 into master Jun 20, 2017

kvark deleted the input branch June 20, 2017 01:49

kvark pushed a commit that referenced this pull request Nov 10, 2017

Merge #33

4f03499

33: New input API r=kvark Closes #8 r? @vitvakatu

Conversation

kvark commented Jun 19, 2017

Uh oh!

vitvakatu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vitvakatu Jun 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kvark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kvark commented Jun 20, 2017

Uh oh!

bors bot commented Jun 20, 2017

Build succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vitvakatu Jun 19, 2017 •

edited

Loading