Opened 14 years ago

Closed 13 years ago

Last modified 12 months ago

#8532 closed patch

Simplified keyboard repeat

Reported by: eriktorbjorn Owned by: eriktorbjorn
Priority: normal Component: Ports
Keywords: Cc:
Game:

Description

Right now, the GUI and some game engines implement
keyboard repeat independent of each other. But wouldn't
it be both easier and more consistent to let the
backend handle this?

The attached patch implements this for the SDL backend
(simply a matter of calling SDL_EnableKeyRepeat()), and
removes any custom repeat code that I've been able to find.

If it's not desirable to always have keyboard repeating
(though I can't currently think of any such case), we
could easily make keyboard repeating a property that
can be toggled on or off.

Ticket imported from: #1487149. Ticket imported from: patches/637.

Attachments (3)

keyboard-repeat.diff (14.3 KB ) - added by eriktorbjorn 14 years ago.
Patch against current SVN
keyboard-repeat2.diff (14.2 KB ) - added by eriktorbjorn 13 years ago.
Updated patch
keyboard-repeat3.diff (18.7 KB ) - added by eriktorbjorn 13 years ago.
Updated patch for the new event manager.

Download all attachments as: .zip

Change History (16)

by eriktorbjorn, 14 years ago

Attachment: keyboard-repeat.diff added

Patch against current SVN

comment:1 by eriktorbjorn, 14 years ago

There may of course be a perfectly good reason why we're not
doing this already. The only one I can think of is that in
the past we used to be able to move the cursor using the
arrow keys on the keyboard.

Maybe automatic keyboard repeating interfered with that? But
that feature was removed because - I think - it interfered
with using the keyboard to scroll GUI lists, etc.

Assigning to Fingolfin, since he may know more about this,
and because I trust his instincts.

comment:2 by eriktorbjorn, 14 years ago

Owner: set to fingolfin

comment:3 by fingolfin, 14 years ago

Hum, somehow I never saw this tracker item before, and hence didn't comment
on it earlier :-(.

Anyway, it *seems* to me that this should be safe (from your description, I
haven't looked at the actual patch yet). I woudl recommend emailing scummvm-
devel about this, though, to make porters aware, and maybe add a comment to
OSystem, somewhere...

comment:4 by fingolfin, 14 years ago

AH, I just noticed that the date on this tracker item is 2006-05-12 and *not*
2005-05-12 as I read it first... phew :-). I already was feeling guilty for having
overlooked this for so long :-)

comment:5 by eriktorbjorn, 13 years ago

Since there have been some code restructuring since I first
submitted this patch, I'm attaching an updated version.

Note that there have been some discussion on the mailing
list about the patch. Not everyone's in favour of it.
Perhaps more discussion is needed.

by eriktorbjorn, 13 years ago

Attachment: keyboard-repeat2.diff added

Updated patch

comment:6 by sev-, 13 years ago

What is the status of this item?

comment:7 by eriktorbjorn, 13 years ago

Last thing I heard, people liked the idea but thought there
was a better way of implementing it. That's why I haven't
bothered to update the patch.

by eriktorbjorn, 13 years ago

Attachment: keyboard-repeat3.diff added

Updated patch for the new event manager.

comment:8 by eriktorbjorn, 13 years ago

I'm attaching an updated patch where the keyboard repeat is handled by the new event manager.

However, this exposed a slight problem: In the AGI engine, pressing a direction key causes your character to walk in that direction. Pressing the same key again causes the character to stop. In this case, keyboard repeat causes the character to start/stop, in effect walking slowly and jerkily.

To counteract this, I've added a "synthetic" field to the event struct, to mark which events are real and which are artificial. With the patch, the AGI engine ignores synthetic key down events for the direction keys, but not for other keys. This still isn't ideal, since it would be nice if the movement keys repeated in the save/load dialog, but that should be easily fixable. I just don't want to do it until we've decided what to do about this patch.
File Added: keyboard-repeat3.diff

comment:9 by fingolfin, 13 years ago

An alternative solution would be to add "enable/disable keyboard repeat" methods to the event manager.

comment:10 by eriktorbjorn, 13 years ago

Yes, but simply disabling all keyboard repeat (to avoid repeating movement keys) would also disable keyboard repeat when typing commands, and that's probably not what we want.

(Later SCI games pause the game while you type commands, but in the AGI games you walk and chew gum^H^H^H^H^H^H^H^Htype at the same time.)

comment:11 by eriktorbjorn, 13 years ago

Owner: changed from fingolfin to eriktorbjorn
Status: newclosed

comment:12 by eriktorbjorn, 13 years ago

Committed, with Fingolfin's blessing.

comment:13 by digitall, 12 months ago

Component: Ports
Note: See TracTickets for help on using tickets.