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

Added double and triple click detection for SDL2 and GL #309

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

Lyle-Tafoya
Copy link
Contributor

SDL2 and GL builds were not detecting double or triple clicks, but instead would just produce a series of single click events when clicking multiple times. This simply turns the click detection into a loop that increments from BUTTON_PRESSED to BUTTON_CLICKED to BUTTON_DOUBLE_CLICKED to BUTTON_TRIPLE_CLICKED and bails out if any other input is received.

I get the impression from #117 that you wanted to refine the logic for double and triple click detection used in wingui and then port it over to other platforms, but maybe we can use this for sdl2 and gl until then.

@Lyle-Tafoya
Copy link
Contributor Author

Upon closer inspection, I realize that this code will eat a mouse button press event if some other event is received between the mouse button press and mouse button release. For example, If I double click, but press the 'a' key before the final mouse button release, it will generate a mouse click event, key press event, and a mouse button release event. I think I can fix this by keeping track of the last event type and pushing the mouse button press event on the queue if we receive some other input before the mouse button release event.

@Bill-Gray
Copy link
Owner

Overall, this is looking pretty good. It can even be extended to SDL1 if one makes a replacement SDL_WaitEventTimeout(), which is easy to do (I've just done it; see below. I'm not actually going to push it in until we settle the current issue for SDL2 and GL.)

I was going to say that keystrokes superimposed on double/triple clicks shouldn't be much of a problem. And they aren't in "normal" use. But I could imagine running into trouble with, say, games that involve simultaneous keyboard and mouse activity.

I rather like the simplicity of your solution, and would still take it over the current approach. But I'd agree that a 'full' solution is at least worth looking into.

Incidentally, one difference I do see between our original napms( SP->mousewait), then SDL_PollEvent() approach and the new SDL_WaitEventTimeout() one is that in the former, we always wait SP->mousewait milliseconds, even if the next event comes along really quickly. SDL_WaitEventTimeout() will (I think) return as soon as that next event occurs. (Which is behavior I had to mimic in making an SDL_WaitEventTimeout() for SDL1.)

/* SDL1.2 lacks this SDL2 function,  but we can 'fake' it.  We wait for the 
specified timeout,  checking every 20 ms to see if an event occurred. */

int SDL_WaitEventTimeout( SDL_Event *event, int timeout_ms)
{
   int rval = 0;

   while( timeout_ms && !rval)
      {
      const int slice_ms = (timeout_ms > 20 ? 20 : timeout_ms);

      napms( slice_ms);
      rval = SDL_PollEvent( event);
      timeout_ms -= slice_ms;
      }
   return( rval);
}

@Lyle-Tafoya
Copy link
Contributor Author

Lyle-Tafoya commented Oct 26, 2023

I discovered some more information which might be worth discussing.

I don't know how important it is for you to match the behavior of ncurses, but I consider it important since I use ncurses for the Linux build of my application and PDCursesMod for the Windows builds. I tried to see how ncurses handles a key press occurring between the mouse button press and mouse button release events in a double/triple click. It turns out that this produces a series of mouse button press and mouse button release events, without any mouse button click events. Not only that, but all these mouse button press/release events are put on the mouse event queue and only a single KEY_MOUSE input is seen from wgetch. This means that to process all the mouse button press/release events properly during this edge case in ncurses, getmouse must be called from within a loop until it returns ERR. However, I see in the manual for PDCursesMod that nc_getmouse does not actually have an event queue (and in fact, calling nc_getmouse in a loop until it returns ERR would result in an infinite loop as it processes the last event repeatedly), but only contains the most recent event. This makes it impossible to match the behavior of ncurses without adding an actual mouse event queue behind nc_getmouse.

How do you feel about the addition of a mouse event loop to be used by nc_getmouse? I think it is a good idea even without trying to match ncurses behavior in this edge case, because without it, I think it could be possible that a mouse event could get overwritten by a new mouse event before the application's input handling routine can process it, which would most likely result in processing the same mouse event twice (since it would still see 2 KEY_MOUSE inputs).

@Lyle-Tafoya
Copy link
Contributor Author

Also, it appears that ncurses also waits for the full mouseinterval, even if the input is received before then. So depending on how much we care about matching the behavior of ncurses, the use of SDL_WaitEventTimeout may not be the way to go. However, I am having a hard time imagining a scenario where the early return could be problematic, and in fact, I consider it to be better behavior.

@Lyle-Tafoya
Copy link
Contributor Author

I also learned that in ncurses, if a person performs more than 3 clicks in a row, it will add a mouse button click event and a mouse button triple click event to the mouse event queue. This happens regardless of how many clicks occur, and both of these events come with only a single KEY_MOUSE input.

@Bill-Gray
Copy link
Owner

Matching ncurses is of moderate importance to me. It's the most common implementation of curses out there. If nothing else, if the behavior of PDCursesMod doesn't match that of ncurses, I at least want to know why. There are cases where the difference is defensible, and have been cases where the difference caused me to realize we were doing something the Wrong Way.

I've thought about adding a mouse event queue to PDCursesMod. I'd see three benefits offhand :

  • It might aid in solving the double and triple-click issue in a non-platform-specific manner; i.e., each platform would say "here is a stream of presses, releases, movements, and mouse wheel events; synthesizing the first two into clicks and double-clicks should be resolved at a higher, non-platform-specific level." Thus, you might have presses and releases in the queue that stay there until SP->mouse_interval milliseconds have elapsed, because they're awaiting events with which they might be merged.
  • As you describe, events can get missed without such a queue, and the queue makes handling interspersed mouse and key events slightly easier.
  • It would enable ungetmouse() to work in the same manner as it does in ncurses. Since ungetmouse() is, in fact, a totally ncurses-specific function (doesn't exist in the SysV mouse interface), its behavior ought to match the specification from ncurses.

I expect it would require a good bit of code to be written/revised. Among other things, I think it would mean that napms() would have to check for mouse input and add events to the queue, on each platform. Still... just the fact that double- and triple-clicks would be handled in one place instead of on each separate platform might let us get rid of a lot of redundant code; the final result might actually be simpler than the current hodge-podge of solutions.

@Bill-Gray Bill-Gray merged commit a4e563d into Bill-Gray:master Nov 17, 2023
4 checks passed
Bill-Gray added a commit that referenced this pull request Nov 17, 2023
…d GL, to SDL1. The logic is almost exactly the same, except that SDL1 lacks SDL_WaitEventTimeout(); fortunately, it was easy to implement that function. See issue #309.
@Bill-Gray
Copy link
Owner

While I'd still like some of the "gee, it'd be nice if" features we discussed above, your patch is a definite improvement over what we had before (thank you!), so I've pulled it (and made the corresponding SDL1 fix). We can achieve perfection on another day.

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

Successfully merging this pull request may close these issues.

None yet

2 participants