Opened 12 years ago

Closed 12 years ago

Last modified 12 months ago

#8712 closed patch (fixed)

Possible patch for FR 1754616

Reported by: bluegr Owned by: bluegr
Priority: normal Component: GUI
Keywords: Cc:
Game:

Description

FR 1754616 asks to delete games using the delete key

I've implemented this in the attached patch, by modifying the list widget control, however it's not generic as the other commands, i.e. when "enter" is pressed, the current list item is activated, but, with this patch, when "delete" is pressed, the current game's configuration is removed (i.e it's assumed that only one listwidget control is used, the one that has all the game configurations).

AFAIK, list widgets are not used in any place other than the list of games, but if we do use a list widget someplace else, the delete key will be bound to "Remove game configuration" for that list, which is not a generalized command, but it's specific for the game list widget.

I'm not quite sure how this can become more generalized, so I'm submitting it as a patch for people who know more about ScummVM's GUI :)

Ticket imported from: #1755666. Ticket imported from: patches/817.

Attachments (1)

delete game with del key.patch (1017 bytes ) - added by bluegr 12 years ago.
Delete a game's configuration with the del key

Download all attachments as: .zip

Change History (9)

by bluegr, 12 years ago

Delete a game's configuration with the del key

comment:1 by eriktorbjorn, 12 years ago

There are actually few other places where ScummVM uses the list widget. Perhaps most noticeably, the SCUMM save/load dialog.

Anyway, the widgets should probably not have that kind of specific knowledge of what they're being used for. It would probably be better to use the "hotkey" handling (each button can have a hotkey assigned to it) somehow, but there are at least two problems with that approach: The hotkey is an ASCII code, not a "keycode", and the list widget steals the focus so we never get to the hotkey handler anyway.

comment:2 by bluegr, 12 years ago

Well, there are two approaches to solve this that I can think of:
1) create an class which inherits from listwidget (e.g. gameslistwidget), which handles the delete key
2) (a bit clumsy) add a check in the delete key case, which checks if the current list is "launcher_list" (the list of games)

I'm not sure about the hotkey class, as you said yourself that the list widget steals the focus. The Add/Edit/Remove game buttons have the "A", "E", "R" hotkeys assigned to them, but they're never triggered as the list itself has focus. So if we press "A", "E" or "R", the current item shifts to the one that starts with one of those letters (e.g. "Elvira" for "E")

comment:3 by eriktorbjorn, 12 years ago

Owner: set to fingolfin

comment:4 by eriktorbjorn, 12 years ago

The focus stealing is slightly less of a problem than I first thought. The list widget doesn't steal all keypresses. If the key isn't handled (e.g. the delete key, unless - I assume - in edit mode), the keypress passes through to the hotkey handling after all.

But the hotkey is still an ASCII code. Fingolfin might have opinions on whether or not that should be changed.

comment:5 by fingolfin, 12 years ago

In the current form, the patch is not OK.

But if you simply change "kListRemoveGameCmd" to "kListItemRemovedCmd" or "kListItemRemovalRequestedCmd", it becomes generic enough, I'd say.

A subclass should be avoided. as for hotkeys: Those are only implemented for button widgets, and indeed are rather hackish right now - but I don't think we need to worry abot those here.

BTW, I' would catch both delete *and* backspace to trigger removal.

comment:6 by bluegr, 12 years ago

Thanks fingolfin, I've made the changes you requested, and the implementation is now generic :)

I've submitted this as commit #28131

Closing this

comment:7 by bluegr, 12 years ago

Owner: changed from fingolfin to bluegr
Resolution: fixed
Status: newclosed

comment:8 by digitall, 12 months ago

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