Opened 17 years ago

Closed 17 years ago

#609 closed defect (fixed)

MI1VGA: wrong color

Reported by: SF/sir_kill_a_lot Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Monkey Island 1

Description

This bug was introduced shortly. After starting MI1VGA
(german) the black color is shown blue at least on the
copy protection screens. The font should be outlined
black and the faces doesn't contain any black color
anymore.

Last version without bug I have:
ScummVM 0.2.82 CVS Built on Nov 28 2002 17:54:41

First one with bug:
ScummVM 0.3.0 Built on Dec 2 2002 17:27:20

Ticket imported from: #647844. Ticket imported from: bugs/609.

Change History (8)

comment:1 by eriktorbjorn, 17 years ago

I'll try to look into this one later tonight.

Off-hand I can only remember two recent changes that would
affect colours: The setPaletteFromPtr() change and the
o5_roomOps case 2 ("room color") one.

The former was made specifically for MonkeyVGA, and should
only affect near-white colours anyway, while the latter was
made for Zak256.

comment:2 by fingolfin, 17 years ago

It is the o5_roomOps (if I uncomment the change to
copyPalColor it works). Which is a bit weird, need to look
more into this.

comment:3 by eriktorbjorn, 17 years ago

Hmm... Maybe it should be copyPalColor(b, a) instead of
copyPalColor(a, b)? Though that makes the background colour
black for the copy protection screen, and I have no idea
what it would do to Zak256.

Unfortunately the original interpreter doesn't show the copy
protection screen at all (because it was part of a
collection that didn't include the Dial-A-Pirate wheel), so
I can't check what it should look like.

Either way we could remove the checkRange() call, since
copyPalColor() checks its parameters, and probably the
_fullRedraw = 1 line as well.

comment:4 by fingolfin, 17 years ago

I don't really think that (b,a) makes any more sense, esp.
since as you noticed, this would turn the screen background
black.
Rather I fear our whole implementation there might be wrong.
At the start of MonkeyVGA, this call is made (in the current
notation, so (a,b)):
copyPalColor(0, 1)
Then on the screen with the pirates, this one:
copyPalColor(1, 1)
Obviously that doesn't make sense. Hence I am 99% certain
our implementation is still wrong. Two guesses:
1) We are reading the params completly wrong (but I doubt
that, I don't think the script would run correctly then)

2) My original thought about this is right after all (I
discussed this opcode with endy maybe 5-6 month ago): what
is happening here is that in SMALL_HEADER games, the
"palette" is not a map from index to RBG; rather, it is a
map from an index to the HW palette! In this situation, a
call like above with (1,1) would actually make sense: it
would mean "revert index 1 to point to its default value again".

I must admit that 2) is half a guess, half based on some
disassmbly I am not sure I interpret correctly... this needs
more investigation.

comment:5 by eriktorbjorn, 17 years ago

Hmm... I am, again, tempted to guess on _shadowPalette[]
because in GF_SMALL_HEADER games updatePalette() uses that
one to remap colours, i.e. it has almost the same effect as
copyPalColor(). (This is what we use to implement the
animation of the Great Loom in LoomCD.)

But I don't have anything at all to back this up, and
besides it would make the "room color" and "set palette
color" cases almost identical, which doesn't make much sense
either.

comment:6 by fingolfin, 17 years ago

I checked in a "fix". Now the monkeyvga will have initiall a
black background, but the moment the pirate face is shown,
it switches back to a blue background. Can somebody check
how it looks in the original?

comment:7 by SF/sir_kill_a_lot, 17 years ago

I tried latest cvs and it seems that this bug is fixed now.
There is no black background at the beginning (the "history
quiz and press enter"-screens are blue like the original were).

comment:8 by fingolfin, 17 years ago

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