Can't use KeyDown and KeyUp events properly? (Visual C# Windows Form)

Question

I am having problems in breaking a loop (apparently) with the KeyUp event; Character moves, but then I can't make it stop after releasing the key. Looks like I am doing something wrong. What could I change in this code for it to work? Thanks for the help!

private void Character_KeyDown(object sender, KeyEventArgs e)
{
    if (e.KeyCode == Keys.A)
    {
        XSpeed = 1;
    }

    for (; e.KeyCode == Keys.A;)
    {
        Thread.Sleep(50);
        Character.Left -= XSpeed;
        if (XSpeed == 0)
        {
            break;
        }
    }
}

private void Character_KeyUp(object sender, KeyEventArgs e)
{
    if (e.KeyCode == Keys.Left)
    {
        XSpeed = 0; 
    }
}

Answer 1

The problem here is that you are never going out from the Character_KeyDown handler does not terminate. Because of that, your bit of code in the Character_KeyUp is never executed.

The root issue that you may not realize is that you only have a single UI thread (at least in all UI frameworks that I know such has been the case), and you are monopolizing it with your for loop.

In order to do the right thing, something like WPF's Dispatcher.BeginInvoke, or DispatcherTimer can be used. (If you use WPF). If you can tell us which UI framework you are using, we might be able to come up with a satisfactory code sample.

Here is how you might do it with a DispatcherTimer in WPF:

// Add that field to your class.
private readonly DispatcherTimer moveLeftTimer;

// In the constructor, add the lines inside:
YourClassName() // This line is a stub - I do not know your class name.
{
    moveLeftTimer = new DispatcherTimer()
    {
        Interval = TimeSpan.FromMilliseconds(50)
    };
    moveLeftTimer.Tick += MoveLeft;
}

private void MoveLeft(object source, EventArgs e)
{
    Character.Left -= XSpeed;
}

private void Character_KeyDown(object sender, KeyEventArgs e)
{
    if (e.KeyCode == Keys.A)
    {
        moveLeftTimer.IsEnabled = true;
    }
}

private void Character_KeyUp(object sender, KeyEventArgs e)
{
    if (e.KeyCode == Keys.A)
    {
        moveLeftTimer.IsEnabled = false;
    }
}

In order to adapt for Windows Forms, use the Timer class instead of DispatcherTimer, and the property is named Enabled instead of IsEnabled.

Answer 2

That's because you are doing everything on one thread, in your case it happens to be the Main UI thread. All actions are done sequentially. It does't matter that you have a handler to change XSpeed = 0, it's not going to process until the Character_KeyDown is done. Put break points in both and you will see the point. What you need to do is to put your processing on the background thread and release your UI handlers.

Task.Factory.StartNew creates a background thread and executes an action you give it. now if you need to update something that belongs to the UI thread, you gotta do it on the UI thread...

private void Character_KeyDown(object sender, KeyEventArgs e)
{
    Task.Factory.StartNew(() =>
    {
      if (e.KeyCode == Keys.A)
      {
          OnUI(() => XSpeed = 1);
      }

      for (; e.KeyCode == Keys.A;)
      {
          Thread.Sleep(50);
          OnUI(() => Character.Left -= XSpeed);
          if (XSpeed == 0)
          {
              break;
          }
      }});
}

private void Character_KeyUp(object sender, KeyEventArgs e)
{
    Task.Factory.StartNew(() =>
    {
        if (e.KeyCode == Keys.Left)
        {
            OnUI(() => XSpeed = 0); 
        }
    }
  }

here's a sample for OnUI:

private void OnUi (Action action)
{
    if (_dispatchService == null) 
        _dispatchService = ServiceLocator.Current.GetInstance<IDispatchService>();
        //or _dispatchService  = Application.Current.Dispatcher - whatever is suitable

    if (_dispatchService.CheckAccess())
        action.Invoke ();
    else
        _dispatchService.Invoke(action);
 }