Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: getKey vs getKeys #48

Open
inv2004 opened this issue Mar 10, 2024 · 12 comments
Open

RFC: getKey vs getKeys #48

inv2004 opened this issue Mar 10, 2024 · 12 comments

Comments

@inv2004
Copy link
Contributor

inv2004 commented Mar 10, 2024

Initial example in readme:

while true:
  var key = getKey()
  case key
  of Key.None: discard
  of Key.Escape, Key.Q: exitProc()
  else:
    tb.write(8, 4, ' '.repeat(31))
    tb.write(2, 4, resetStyle, "Key pressed: ", fgGreen, $key)

  tb.display()
  sleep(20)
@inv2004
Copy link
Contributor Author

inv2004 commented Mar 10, 2024

getKeys example:

while true:
  for key in getKeys():
    case key
    of Key.None: discard
    of Key.Escape, Key.Q: exitProc()
    else:
      tb.write(8, 4, ' '.repeat(31))
      tb.write(2, 4, resetStyle, "Key pressed: ", fgGreen, $key)

  tb.display()
  sleep(20)

@inv2004
Copy link
Contributor Author

inv2004 commented Mar 10, 2024

getKey with buffer example:

Not sure if display should be in the inner while loop or not

while true:
  var key = getKey()
  while key != Key.None:
    case key
    of Key.None: discard
    of Key.Escape, Key.Q: exitProc()
    else:
      tb.write(8, 4, ' '.repeat(31))
      tb.write(2, 4, resetStyle, "Key pressed: ", fgGreen, $key)
    key = getKey()

  tb.display() 
  sleep(20)

@johnnovak
Copy link
Owner

getKey with buffer example:

Yeah, so this is how I want the API to look like, getKeys is an unnecessary complication.

@inv2004
Copy link
Contributor Author

inv2004 commented Mar 10, 2024

Don't you think the double while loop looks not obvious and even ugly? Also, it is just iterator over while simulation.

@johnnovak
Copy link
Owner

johnnovak commented Mar 11, 2024

Don't you think the double while loop looks not obvious and even ugly? Also, it is just iterator over while simulation.

I want to have a minimal API surface. If you really want getKeys(), you can trivially implement it in your code.

Also, you don't need a double while loop if you don't particularly care about 20ms delay between processing keypresses now and then. Or you can do 5 or 10ms sleep. Again, up to the client code how to deal with this, the minimum we need is getKey.

@inv2004
Copy link
Contributor Author

inv2004 commented Mar 11, 2024

The only good, in my opinion, solution to avoid the cpu usage and to avoid sleep without any delays is:

while true:
  let key = getKeyWithTimeout(1000)
  case key
  of Key.None:
    buf.add '.'
  of Key.Escape, Key.Q: exitProc()
  else:
    buf.add $key
  if buf.len > 20:
    buf = buf[^20..^1]
  tb.write(8, 4, ' '.repeat(31))
  tb.write(2, 4, resetStyle, "Key pressed: ", fgGreen, buf)
  tb.display()

@johnnovak
Copy link
Owner

Thanks for the examples @inv2004, but I've said all I wanted on the matter. getKeyWithTimeout is against my vision how an immediate mode UI API should operate.

I only want getKey, and getKeys needs to become private. If you feel like making the change, a PR is welcome, otherwise I will do it in the coming days.

Thank you.

@johnnovak
Copy link
Owner

Actually @inv2004, thought about it a bit more and getKeyWithTimeout(20) or something could work, yeah. Your 1000 ms wait time just threw me off ;) If you want to work on this, I'm happy for you to introduce getKeyWithTimeout in addition to getKey. Then people can decide which one to use.

@inv2004
Copy link
Contributor Author

inv2004 commented Mar 12, 2024

I am working on getKeyWithTimeout

When it is ready - getKeys can be rolled back

The main challenge is to avoid any addition buffers and to make parser for long sequences works directly on stdin next char

@johnnovak
Copy link
Owner

johnnovak commented Mar 13, 2024

@inv2004 Please make sure that your async solution works not only in Linux terminals but also on Windows in CMD.EXE and on macOS. All illwill features need to be cross-platform. I don't have much time and interest to figure this out, so getKey + sleep is always the fallback option if you can't do Windows & macOS 😏

@inv2004
Copy link
Contributor Author

inv2004 commented Mar 14, 2024

Done here #47

  • Linux stdin parser
  • some small tests for the parser
  • Windows did not work correct at all even before the change => fixed and changed

BTW: win api looks 10 times better than the stdin escapes

-- added --
Please check macos by yourself

@inv2004
Copy link
Contributor Author

inv2004 commented Mar 20, 2024

Here is an iterator for someone who wants pretty monotonic ticks without sleep:

iterator keyEachSec(): Key =
  var timeout = 1000
  while true:
    let a = getMonoTime().ticks
    let k = getKeyWithTimeout(timeout)
    if k == Key.None:
      timeout = 1000
    else:
      let b = int((getMonoTime().ticks - a) div 1000000)
      timeout = timeout - (b mod 1000)
    yield k

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants