[Scummvm-devel] Simplifying keyboard repeat

[Marcus Comstedt]

Max Horn <max at quendi.de> writes:

> But it does. Or rather in this case, it documents the "uber-
> suckfullness" / "lameness" of that help dialog. It can't be fixed by
> changing the backends / engines, either -- some backends simply
> *never* will have those hotkeys, ever! So the help dialog *does*
> suck, the issue is orthogonal to whether and how we implement certain
> backend specific hotkeys.

Well, yes.  I didn't question the fact that the dialog sucks, but
rather that _every_ mismatch was to be accounted to its suckfullness.
More precisely, I'm not convinced that, for every mismatch, the best
solution is to (only) change the dialog.  Was that clearer?  :-)
So basically, I too was flagging for an amount of orthogonality from
the dialog's state of suck.  There are two separate issues, that
manifest in the same way (i.e. "what you read is not what you get").

In order to reduce the amound of suck in the dialog, I suppose some
kind of backend API would be needed to query some help text for
platform specific features from the backend.  That would allow
e.g. controller mappings for console based ports to be displayed
as well.


> Sure, I do. Do you remember my question? I'll repeat it for you:
> "Marcus, could you give a list of all hotkeys that you think should
> be shared, and state which ports would share them?"

And I gave you an answer, remember?  Oh, right, I didn't answer the
part about "which ports".  I'll do that now.  "All of them".  :-)



>>   Today it's
>> three(?) backends which would need the three lines,
>
> It's two backends.

SDL, X11, DC makes three.


> From the list you have given so far (i.e.: Ctrl-A.
> And nothing else), your proposal is to add a function which
> essentially does this:
> bool handleCommonHotkeys(Event evt) {
>    if (hotkey = ctrl-a)
>      toggle aspect correction
>      return didHandle;
>    else
>     return didNotHandle;
> }
>
> and then modifying those *two* backends to call that function.

No no.  The backends should not call the function.  That would kind
of defeat the purpose.  The platform-independent pollEvent() function
would call it, after calling the backend-specific version of
pollEvent().  This should be no more strange than having a platform-
independent installTimerProc() that wraps the call to the backend-
specific function setTimerCallback().  Sure, the backends could handle
multiple timers, it's just a couple of lines of code, but since it's
easy to make a platform-independent implementation, they don't have
to.  The same logic applies here.


> Look, I am all for factoring out shared code when it makes sense, in
> fact a lot of the work I have done in the past years on ScummVM
> revolves around exactly that. But the stress here is on "where it
> makes sense". I think to justify factoring out something, it has to
> be either repeated several times, or has to be complicated. But a
> *trivial* code check, that is used in two places, doesn't quite fill
> that role for me. That's why I ask: Are there other candidates? If we
> have 3-4 hotkeys that are shared by at least 2 backends, we can talk
> this. But it's silly to do that for the single case you have
> suggested so far. To me, it clearly falls into the category "needless
> overdesign".

Well, if it's _only_ for the single hotkey, then I agree.  The basis
for the proposal was that key repeat would be handled this way also,
which would make it a little more "critical mass".  I agree with your
suggestion to let backends which want to do their own key repeat
do so through setFeatureState(kFeatureKeyRepeat, ...), in which case
the global key repeat code would handle the case when
hasFeature(kFeatureKeyRepeat) returns false.  But if we're not going
to have a global key handler at all for key repeat (which was your
second suggestion), a global key handler just to handle the hotkey
would be overkill.

But before dismissing A) a global key handler to do key repeat and
B) a global key handler to handle hotkey(s) on account on each of them
doing to little to be worth the trouble, at least consider a global
key handler that does _both_.


  // Marcus