Opened 12 years ago

Closed 12 years ago

Last modified 11 months ago

#8664 closed patch

SCUMM: Improved ctrl+t subtitle cycling

Reported by: salty-horse Owned by: sev-
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game:

Description

When changing SCUMM subtitle settings with the ctrl+t shortcut, most of the time I want to press it twice to get to the desired setting. (speech only -> text only -> text and speech).

The message dialog that pops up with the current setting takes a lot of time to disappear, and I have to wait before pressing ctrl+t again.

In the original COMI (and maybe other scumm games), pressing ctrl+t again would make the older message disappear.

ScummVM should at least emulate this useful behaviour.

The patch adds a such a toggle-able option to the TimedMessageDialog. It's disabled by default, and I enabled it for the SCUMM subtitle settings.

BTW, an even even better approach would be to catch subsequent ctrl+t presses and automatically move to the next setting, without making the message dialog disappear.

Ticket imported from: #1723779. Ticket imported from: patches/769.

Attachments (3)

timed-message.patch (1.6 KB ) - added by salty-horse 12 years ago.
Patch
subtitles-settings.patch (3.4 KB ) - added by salty-horse 12 years ago.
SCUMM Subtitle settings dialog
subtitles-settings2.patch (3.5 KB ) - added by fingolfin 12 years ago.
Slightly revised patch

Download all attachments as: .zip

Change History (14)

by salty-horse, 12 years ago

Attachment: timed-message.patch added

Patch

comment:1 by fingolfin, 12 years ago

I disagree with this patch -- it adds common functionality to the base class TimedMessageDialog w/o proper justification.

Instead I think we should just add a subclass of TimedMessageDialog to SCUMM for this specific dialog. This would also make it relatively easy to catch multiple ctrl-t presses and handle them appropriately.

comment:2 by salty-horse, 12 years ago

How about a GUI::ValueCycleDialog with the following constructor:
ValueCycleDialog(key, ...)

the va_list will be pairs of (number, string).

Similar to ValueDisplayDialog, the usage in this case will be:

ValueCycleDialog dlg(20 /* Ctrl+T - there's a FIXME about it to make it more readable */,
0, "Speech Only",
1, "Speech and Subtitles",
2, "Subtitles only");
int chosenValue = rundialog(dlg);

comment:3 by fingolfin, 12 years ago

As a rule of a thumb: If you have to do something once, do it once. If you have to do it thrice, generalize the code. If you have to do it twice, well, choose.

In this case, though, I don't see why we should spend much effort to come up with a contrived generalized solution, when we only have one very specific and simple problem to solve at hand :-).

by salty-horse, 12 years ago

Attachment: subtitles-settings.patch added

SCUMM Subtitle settings dialog

comment:4 by salty-horse, 12 years ago

Attached a new patch that implements a SCUMM-specific dialog.
File Added: subtitles-settings.patch

comment:5 by salty-horse, 12 years ago

Summary: GUI: Make TimedMessageDialog interruptible by kbd/mouseSCUMM: Improved ctrl+t subtitle cycling

by fingolfin, 12 years ago

Attachment: subtitles-settings2.patch added

Slightly revised patch

comment:6 by fingolfin, 12 years ago

Owner: set to sev-

comment:7 by fingolfin, 12 years ago

Attached a slightly revised form of the patch.

Sev, I would like to see this go into SVN now, but as we are still in a freeze, I wanted to run this by you before commiting. Any objections? It looks safe enough to me (a local change which is easy to verify as working fine).
File Added: subtitles-settings2.patch

comment:8 by fingolfin, 12 years ago

Owner: changed from sev- to fingolfin
Status: newclosed

comment:9 by sev-, 12 years ago

Owner: changed from fingolfin to sev-

comment:10 by sev-, 12 years ago

It looks completely safe to me. So I committed it. Thank you, Ori.

comment:11 by digitall, 11 months ago

Component: Engine: SCUMM
Note: See TracTickets for help on using tickets.