Opened 6 years ago

Closed 5 years ago

#6455 closed defect (fixed)

SCI: SQ5 - Incorrect text color on Cliffy's toolbox screen

Reported by: SF/boneosaurusrex Owned by: m-kiewitz
Priority: normal Component: Engine: SCI
Keywords: Cc:
Game: Space Quest 5

Description

On the toolbox screen the buttons' text is white in ScummVM instead of yellow as in Sierra SCI. Obviously we're talking about a pretty big blocker here!

Attached is a comparison screenshot and save games for ScummVM and Sierra SCI.

ScummVM 1.7.0 nightly
SQ5 1.04 English

Ticket imported from: bugs/6455.

Attachments (3)

sq5_toolbox_comparison.png (108.0 KB ) - added by SF/boneosaurusrex 6 years ago.
sq5.004 (45.7 KB ) - added by SF/boneosaurusrex 6 years ago.
SQ5SG.001 (10.8 KB ) - added by SF/boneosaurusrex 6 years ago.

Download all attachments as: .zip

Change History (20)

by SF/boneosaurusrex, 6 years ago

Attachment: sq5_toolbox_comparison.png added

by SF/boneosaurusrex, 6 years ago

Attachment: sq5.004 added

comment:1 by SF/boneosaurusrex, 6 years ago

ScummVM and Sierra SCI savegame standing in front of toolboxes

comment:2 by m-kiewitz, 6 years ago

text is displayed using kDisplay in script 226 and using global 98h (152) for the pen color

comment:3 by Strangerke, 6 years ago

Component: Engine: SCI
Game: Space Quest 5

comment:4 by bluegr, 5 years ago

This is one of the colors initialized in script 12, export 0:

pushi 4 // params
pushi 5 // subop 5, find color
pushi 223
dup // 223 again
dup // 223 again
callk kPalette, 8
sag global[98h]

I can't see any other place where this is initialized, at a first glance

comment:5 by wjp, 5 years ago

This is actually a bug in SQ5. The scripts ask for grey (223,223,223), as Filippos mentions above, but the kPalette/FindColor call is broken in SQ5. The "distance" between two color components is done with signed 8 bit arithmetic, so 0 and 255 are considered to have a distance of 1. Our "correct" FindColor implementation returns light-grey/white, but SQ5 thinks yellow is closer to grey.

comment:6 by bluegr, 5 years ago

Very interesting findings. So overall, here's the situation:
- Game scripts as for a white/light grey color (223, 223, 223)
- The original interpreter is buggy, and finds a completely wrong color (yellow)

So, there are two ways to handle this:
1) use the original silly algorithm, or alternatively, hardcode the offending color, so that the end result is what users expect
2) Leave it as-is, since this is obviously a bug in the original interpreter, and not fix it at all

I'm more in favor of option 2 (leave it as-is). I don't believe that we should break a working algorithm just to get non-intended behavior in games.

comment:7 by m-kiewitz, 5 years ago

We need to figure out if it's SQ5 interpreter only or if all SCI1.1 interpreter do it that way and then adjust our code of course.

EDIT: Update - I just looked into KQ6 and it seems it does it the same way. KQ5 used 16 bits instead. So it seems that's it's really SCI1.1

Will have to verify this with more interpreters.

comment:8 by wjp, 5 years ago

I looked at qfg3, qfg4demo, sq5, kq6. All are the same, and broken.

comment:9 by m-kiewitz, 5 years ago

Have you checked qfg3demo? qfg3demo was the first SCI1.1 game.
And I think I remember that Sierra used some old graphics code for the first few SCI1.1 games. I think it changed during Laura Bow 2 Floppy.

comment:10 by m-kiewitz, 5 years ago

Hmm, I just checked Laura Bow 2 Floppy. That one uses 16 bits.

Mixed-Up Mother Goose SCI1.1 is inbetween qfg3+lb2floppy.

comment:11 by wjp, 5 years ago

qfg3demo uses 16 bits too.

comment:12 by m-kiewitz, 5 years ago

We need to check Mixed Up Mother Goose interpreter (SCI1.1). That one was inbetween Laura Bow 2 floppy + QfG3. Sadly it seems I never disassembled that specific interpreter.

comment:13 by wjp, 5 years ago

I have the mgoose from the Roberta Williams collection; it uses 16 bit computation here.

comment:14 by m-kiewitz, 5 years ago

Roberta Williams Anthology? That's Mother Goose SCI1, not SCI1.1

comment:15 by wjp, 5 years ago

I always mix up those mgoose versions... The 1.1 version is also 16 bit, anyway. (EXE version 1.001.031)

comment:16 by m-kiewitz, 5 years ago

Just found kPalette inside the Mixed Up Mother Goose SCI1.1 interpreter. It's using 16 bits too.

So it seems qfg3 is the first game that had that change.

comment:17 by m-kiewitz, 5 years ago

fixed / sierra bug implemented in f317e8c8777bdc8482b5e55f3f28873a61eab7da

please note that saved games before this fix will still show the "wrong" color (because SQ5 matches that color right at the start and remembers the color number)

comment:18 by m-kiewitz, 5 years ago

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