fix: #15

[xadaemon]

Great work! I can't review the ringbuffer changes right away (I'll get to them a bit later, maybe tomorrow), but I've reviewed the rest.

The terminal & paint work great, and things are much much better in doom, with much fewer stuck keys issues, though that still happens a bit. I'd consider the issue fixed though, this is good.

~on the doom bit I believe a larger queue size might help I went with a very conservative 5 but something like 20 might do the trick for doom~ edit: this doesn't make much sense since the buffer being overwritten shouldn't make it happen needs further investigation

[29jm]

~on the doom bit I believe a larger queue size might help I went with a very conservative 5 but something like 20 might do the trick for doom~ edit: this doesn't make much sense since the buffer being overwritten shouldn't make it happen needs further investigation

I think it could be that, after all there's only one keyup event for many more keydowns (from other keys being pressed at the same time, when moving with the arrows). This calls for another form of solution I think, extending ringbuffers feels a bit hacky (or perhaps in a case by case basis with a system call?). Outside the scope of this PR and issue anyway ^^

[xadaemon]

yeah maybe some form of raw input might help

[xadaemon]

I fixed every point you suggested (I think)

[xadaemon]

~on the doom bit I believe a larger queue size might help I went with a very conservative 5 but something like 20 might do the trick for doom~ edit: this doesn't make much sense since the buffer being overwritten shouldn't make it happen needs further investigation

I think it could be that, after all there's only one keyup event for many more keydowns (from other keys being pressed at the same time, when moving with the arrows). This calls for another form of solution I think, extending ringbuffers feels a bit hacky (or perhaps in a case by case basis with a system call?). Outside the scope of this PR and issue anyway ^^

should not be terribly hard to allow a program to request a buffer resize but like you said outside of this scope

[29jm]

Looking at the ringbuffer implementation, there are things that bother me. It clearly appears to work, but I don't believe it is correct, in a few ways (do argue if I'm wrong of course). Three things:

• ringbuffer_available: [3,4,_,_,1,2], w_base after 4 (indice 2), r_base at 1 (indice 4), your implementation returns 2, when it should return 4. Something like this looks more correct (but still isn't, because of the next point):

  if (ref->w_base < ref->r_base) {
      return ref->size - ref->r_base + ref->w_base;
  }
  
  return ref->w_base - ref->r_base;

• same function: when the buffer is full, w_base == r_base, and your implementation returns 0. Wikipedia highlights this issue: when dealing with two pointers (read and write), you need a way to distinguish empty and full buffers (I have a preference for dealing with (r_base, data_size) pairs myself for this reason, and ease of understanding)
• reusing the example ringbuffer from above, consider writing 3 more elements to it, say 5, 6 and 7. The 1 gets overwritten (by the 7), yet your implementation keeps r_base at index 4, which means the next read will return the last element, and not the first, which is weird for a FIFO data structure. This is the reasoning behind the if I had added.

edit: if hell was a data structure it'd be a ringbuffer.

[29jm]

Take a look at the last commit on ringbuffers-evolved (naming branches is easy but gives poor results ^^), the fix from your previous PR to working ringbuffers was really nothing much (fingers crossed it keeps working but this time I'm convinced it's mathematically correct).

[xadaemon]

Looking at the ringbuffer implementation, there are things that bother me. It clearly appears to work, but I don't believe it is correct, in a few ways (do argue if I'm wrong of course). Three things:

* `ringbuffer_available`:  [3,4,_,_,1,2], `w_base` after 4 (indice 2), `r_base` at 1 (indice 4), your implementation returns 2, when it should return 4. Something like this looks more correct (but still isn't, because of the next point):
  ```c
  if (ref->w_base < ref->r_base) {
      return ref->size - ref->r_base + ref->w_base;
  }

  return ref->w_base - ref->r_base;
  ```

* same function: when the buffer is full, `w_base == r_base`, and your implementation returns 0. [Wikipedia](https://en.wikipedia.org/wiki/Circular_buffer#Circular_buffer_mechanics) highlights this issue: when dealing with two pointers (read and write), you need a way to distinguish empty and full buffers (I have a preference for dealing with `(r_base, data_size)` pairs myself for this reason, and ease of understanding)

* reusing the example ringbuffer from above, consider writing 3 more elements to it, say 5, 6 and 7. The 1 gets overwritten (by the 7), yet your implementation keeps `r_base` at index 4, which means the next read will return the last element, and not the first, which is weird for a FIFO data structure. This is the reasoning behind the `if` I had added.

edit: if hell was a data structure it'd be a ringbuffer.

I will try to address your concerns in order:

• in this case I made available to return the distance between the read and write pointers, as in how much space there is until data starts being overwritten;
• if it returns 0 it means there is no further space to add data without overwriting
• imho it adds unneeded complexity as much as it seems weird I don't think it's wrong behavior per se

ringbuffers-evolved

oh no they are evolving! ^^

[xadaemon]

I did some testing on the ringbuffer to rule em all it works! I think we can go with it yay ^^

[xadaemon]

!! merged your changes and will upload the commit

[29jm]

Glorious!

[xadaemon]

It's merged and uploaded if there are no other concerns I think we can merge this and end 2020 with one less issue ^^

[29jm]

I'll try github's cli tool to merge & squash this correctly while still having the PR marked as merged, the online interface is of no help at all when you need more than just squashing everything...

[xadaemon]

I did the merging already to my master branch if you just squash it should just work:tm:

[29jm]

Aye, the cli tool doesn't do anything more than the web interface anyway. I really wish there was a way to make github cooperate better with git in this regard, but this'll be good enough ^^

[29jm]

Thanks again for the help, have a great new year's eve!

[xadaemon]

Thanks again for the help, have a great new year's eve!

Have a great new year's eve too