Opened 16 years ago

Closed 15 years ago

Last modified 15 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 16 years ago.
screenshot
ft_1092993-2.diff (18.9 KB ) - added by cyxx 15 years ago.
Patch against a November 1 CVS snapshot
ft_alter.diff (14.4 KB ) - added by Kirben 15 years ago.
Alternative patch
ft_1092993-4.diff (21.7 KB ) - added by cyxx 15 years ago.

Download all attachments as: .zip

Change History (22)

by salty-horse, 16 years ago

Attachment: ft.png added

screenshot

comment:1 by salty-horse, 16 years ago

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

comment:2 by fingolfin, 16 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, 16 years ago

Status: newclosed

comment:4 by sev-, 16 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-, 16 years ago

Fingolfin, I wonder why did you close it?

comment:6 by sev-, 16 years ago

Status: closednew

comment:7 by fingolfin, 16 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-, 15 years ago

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

comment:9 by Kirben, 15 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, 15 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, 15 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, 15 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, 15 years ago

Attachment: ft_1092993-2.diff added

Patch against a November 1 CVS snapshot

comment:13 by Kirben, 15 years ago

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

by Kirben, 15 years ago

Attachment: ft_alter.diff added

Alternative patch

comment:14 by cyxx, 15 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, 15 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, 15 years ago

Attachment: ft_1092993-4.diff added

comment:16 by fingolfin, 15 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, 15 years ago

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

comment:18 by cyxx, 15 years ago

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