Opened 12 years ago

Closed 12 years ago

Last modified 7 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)

Changed 12 years ago by salty-horse

Attachment: timed-message.patch added

Patch

comment:1 Changed 12 years ago by fingolfin

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 Changed 12 years ago by salty-horse

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 Changed 12 years ago by fingolfin

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 :-).

Changed 12 years ago by salty-horse

Attachment: subtitles-settings.patch added

SCUMM Subtitle settings dialog

comment:4 Changed 12 years ago by salty-horse

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

comment:5 Changed 12 years ago by salty-horse

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

Changed 12 years ago by fingolfin

Attachment: subtitles-settings2.patch added

Slightly revised patch

comment:6 Changed 12 years ago by fingolfin

Owner: set to sev-

comment:7 Changed 12 years ago by fingolfin

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 Changed 12 years ago by fingolfin

Owner: changed from sev- to fingolfin
Status: newclosed

comment:9 Changed 12 years ago by sev-

Owner: changed from fingolfin to sev-

comment:10 Changed 12 years ago by sev-

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

comment:11 Changed 7 months ago by digitall

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