InputSystem doesn't use input in correct order

[denniskaselow]

InputSystem uses input_queue.keys_pressed.pop() and is commented as // Get the first key pressed but pop() removes the last item.

If you input keys fast enough (very easy when not using release mode and batch rendering) this can be seen because of erratic movement of the player character.

[iolivia]

good catch, we probably want a queue not a stack!

[denniskaselow]

Something simple as this could also be sufficient:

impl InputQueue {
    pub fn get_first(&mut self) -> Option<KeyCode> {
        if self.keys_pressed.is_empty() {
            None
        } else {
            Some(self.keys_pressed.remove(0))
        }
    }
}

But I'm new to Rust, so I have no idea if this is the Rust way to do things, but it's what I'd do, considering the key_pressed Vec will never have more than a few keys even in worst case (ignoring automated input).

[fineconstant]

I used VecDaque in my implementation:

#[derive(Default)]
pub struct KeyDownQueue {
    pub keys_pressed: VecDeque<KeyCode>,
}

[iolivia]

that looks great @fineconstant! feel free to submit a PR, will be very helpful for others using the repo 😄

[fineconstant]

I looked into it and I think that Vec<KeyCode> is sufficient, so there is no need to make any changes.

With Vec<KeyCode> calling input_queue.keys_pressed.pop() function removes the last element indeed, but input_queue.keys_pressed.push(keycode); appends an element at the back of the collection - it should not cause any problems.

std::vec::Vec - Rust #method.push
std::vec::Vec - Rust #method.pop

I've just recently started learning Rust so I may be wrong, but if you agree then I think we can close this issue 😄

[denniskaselow]

Like I said in the initial comment:

If you input keys fast enough (very easy when not using release mode and batch rendering) this can be seen because of erratic movement of the player character.

A push can happen several times in the time it takes one pop to move the character. Thus you can input up, left, down and it'll move the character down, left, up. So your VecDeque would be much better.

[fineconstant]

Oh I see now, you are right. I will prepare a PR with changes 😃

[fineconstant]

@iolivia , @denniskaselow
Take a look at #91 - what do you think? :)