Opened 15 years ago

Closed 14 years ago

Last modified 14 years ago

#1861 closed defect (fixed)

FT: The verb icon and subtitle drawing glitch

Reported by: salty-horse Owned by: cyxx
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Full Throttle

Description

Whenever the verb icon is displayed along with some
subtitle text, the name of the hotspot is drawn over
the subtitles. When the verb icon disappears, the
subtitles that were drawn beneath the hotspot name
aren't redrawn.

Steps to reproduce:
1) In room 26, click the tower to move towards it.
Then, look at it.
2) The verb icon will disappear a moment after the
subtitles are displayed, and the subtitles will display
incorrectly.
3) You can also try looking at the tower from afar, and
after ben walks towards let and the speech appears,
show the verb icon on the tower.

I haven't check the default bahaviour. I think that in
the case where both subtitles and verb icon appear, the
subtitles should be above the verb.

Screenshot included.

Ticket imported from: #1092993. Ticket imported from: bugs/1861.

Attachments (4)

ft.png (42.0 KB ) - added by salty-horse 15 years ago.
screenshot
ft_1092993-2.diff (18.9 KB ) - added by cyxx 14 years ago.
Patch against a November 1 CVS snapshot
ft_alter.diff (14.4 KB ) - added by Kirben 14 years ago.
Alternative patch
ft_1092993-4.diff (21.7 KB ) - added by cyxx 14 years ago.

Download all attachments as: .zip

Change History (22)

by salty-horse, 15 years ago

Attachment: ft.png added

screenshot

comment:1 by salty-horse, 15 years ago

Update: To get to room 26, it would be easy to start the
game with bootparam 360.

comment:2 by fingolfin, 15 years ago

To process your bug report appropriately, we need you to
provide the following additional information:

* ScummVM version (PLEASE test the latest CVS/Daily build)
* Bug details, including instructions on reproducing it
* Language of game (English, German, ...)
* Version of game (talkie, floppy, ...)
* Platform and Compiler (Win32, Linux, MacOS, ...)
* Attach a save game if possible
* If this bug only occurred recently, please note the last
version without the bug, and the first version including
the bug. That way we can fix it quicker by looking at the
changes made.

This should only take you a little time but will make it much easier for
us to process your bug report in a way that satisfies both you and us.

Thank you for your support!

comment:3 by fingolfin, 15 years ago

Status: newclosed

comment:4 by sev-, 15 years ago

I'll answer for salty-horse.

Current CVS, any OS. Savegame is not required, as it works
with bootparam well, and there are 2 versions of FT and he
has rare version b.

The bug appears because word "Tower" is written on that
interface down to skull. Then subtitle text is drawn, and
_after_ that word "Tower" gets cleared, which leads to that
glitch.

Though it happens not always, i.e. to see the glitch it you
may need to do it several times from different Ben positions.

comment:5 by sev-, 15 years ago

Fingolfin, I wonder why did you close it?

comment:6 by sev-, 15 years ago

Status: closednew

comment:7 by fingolfin, 15 years ago

I don't recall closing this item; most probably I set it to pending, which
auto-closes the bug if no reply is made within 30 days.

comment:8 by sev-, 14 years ago

ScummVM behaviour has improved. It restores text correctly
now, though verb is still on top of the subtitle

comment:9 by Kirben, 14 years ago

The verb issues have not improved, the subtitles been cut off
by verbs, can still be re-produced.

The original version of FT used blastTexts for subtitles, using
an additional queue. It seems to work this way:
The subtitles queue is drawn every time CHARSET_1() is
called.
Subtitles are added to the queue by CHARSET_1() and
drawn the next time the function is called.
Subtitles are removed from queue by resetCharsetBG().

comment:10 by cyxx, 14 years ago

I checked disassembly of FT, TheDig and COMI and, as Kirben
mentionned, for v7 and v8, subtitles are displayed using
blastexts. I wrote some code to make scummvm behave like the
original. Without much surprise, this fixes the mentionned
issue. Basically, I just wrote a new CHARSET_1() function
based on FT disassembly. I also tried to do some cleanup :
- I tried to get rid of the (_hackMsg == 0xFE) hack, and
added a new flag _haveActorSpeechMsg, like the original did
(based samnmax and FT)
- I got rid of the kanji code hack, as it seems that in
v7/v8 charset code prefix is always 0xFF (and not 0xFE, so
no conflict could occur)
- I also simplified the original ScummEngine::CHARSET_1() by
removing version checks etc.

I just tested it with COMI and FT both with subtitles
enabled and disabled and I haven't noticed special issues.

Anyway, I haven't committed this directly as I think it
would be nice that someone with a 'stronger' SCUMM
background than me could just have a quick look at it.
Especially the (_hackMsg == 0xFE) related thing, Fingolfin,
do you have some time ? ;)

comment:11 by fingolfin, 14 years ago

"_hackMsg == 0xFE", yuck, I still deeply regret adding that particular
hack, but hey, I was young, and our savegame format wasn't versioned, so
you thought twice before adding a new saved variable and instead made
evil hacks... <sigh>.

What you describe sounds nice. I'll have a look at the patch...

comment:12 by cyxx, 14 years ago

No problem, I think such patch is the perfect occasion to
(try to) get rid of those old hacks :)

Anyway, updated patch with latest CVS changes.

by cyxx, 14 years ago

Attachment: ft_1092993-2.diff added

Patch against a November 1 CVS snapshot

comment:13 by Kirben, 14 years ago

Attached alternative patch, which merges changes into
current functions, instead of duplicating CHARSET_1() and
stopTalk().

by Kirben, 14 years ago

Attachment: ft_alter.diff added

Alternative patch

comment:14 by cyxx, 14 years ago

Attached a new version with a single CHARSET_1() function
and a new virtual function to handle specific charset codes.
This allows to reduce CHARSET_1() size.

comment:15 by cyxx, 14 years ago

New version of the patch, this fixes some errors introduced
while merging Kirben's one. This also moves all the new
specific subtitle stuff to the ScummEngine_v7 class (as it
was initially). That way, the code is disabled when V7/V8
support is not compiled in and in
ScummEngine_v7::saveOrLoad, we only reference the V7
specific stuff.

by cyxx, 14 years ago

Attachment: ft_1092993-4.diff added

comment:16 by fingolfin, 14 years ago

I'm sorry, due to limited spare time (I just started my new job
yesterday), I haven't been able to thouroughly review this patch. The
first impression I got is quite good though. You might as well apply it
and we sort out the consequences in HEAD :-)

One suggestion: Maybe base SubtitleText and BlastText on a common
base class/struct, or at least order their members the same way. This is
merely cosmetic, mind you, but it helps highligthing the similarities.
Might also add comments to each pointing to the other...

comment:17 by cyxx, 14 years ago

Committed to CVS. As you suggested, I made a common struct
for BlastText and SubtitleText.

comment:18 by cyxx, 14 years ago

Owner: set to cyxx
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.