Opened 16 years ago

Closed 16 years ago

Last modified 8 months ago

#8226 closed patch

proc_special() / _proc_special_palette[] experimental fix

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Sam and Max

Description

This is an attempt to fix bug #651077 ("SAM: Bad Colors
in first Room") at least partially. The colours still
don't look like the screenshot attached to that bug
report, but I think it looks better than the current
ScummVM behaviour.

There are two parts to the patch. There's some changes
to proc_special(), because it appears to have a masking
problem, and there are changes to createSpecialPalette().

The latter change is pure guesswork, but the 'from' and
'to' parameters make much more sense to me this way. I
still wonder if the loop should be as it is, though, or
if it should be "j <= to" instead. Doesn't seem to make
much difference when I try it though.

There are a few other places where the special palette
is used as well. The tank in the hall of oddities, and
probably things like the glass elevator at the Ball of
Twine and the pool of water at the Mystery Vortex. I'd
be grateful if someone who knows what these should look
like could take a look and tell me if the patch is an
improvement or not.

Of course, I'd be even more grateful if someone could
figure out exactly how createSpecialPalette() should
work, and how _shadow_mode should be handle. ;-)

Ticket imported from: #739119. Ticket imported from: patches/331.

Attachments (3)

spec-palette.diff (1.8 KB) - added by eriktorbjorn 16 years ago.
Patch against a May 17 CVS snapshot
spec-palette2.diff (4.2 KB) - added by eriktorbjorn 16 years ago.
Revised patch against a May 17 CVS snapshot
spec-palette-refinement.diff (709 bytes) - added by eriktorbjorn 16 years ago.
Patch against a May 19 CVS snapshot

Download all attachments as: .zip

Change History (17)

Changed 16 years ago by eriktorbjorn

Attachment: spec-palette.diff added

Patch against a May 17 CVS snapshot

comment:1 Changed 16 years ago by eriktorbjorn

Oops, the patch is against a May *16* snapshot. I mistyped.

comment:2 Changed 16 years ago by eriktorbjorn

Looks like the patch will need a little bit more work. Moment...

Changed 16 years ago by eriktorbjorn

Attachment: spec-palette2.diff added

Revised patch against a May 17 CVS snapshot

comment:3 Changed 16 years ago by eriktorbjorn

There. The placement of the tank in the Hall of Oddities
still looks a bit off, but unlike the previous patch this
one at least shouldn't make it any worse than before. It
looks much better if I add one to the X coordinate, but that
only seems to be true in this particular case. I have no
idea what's causing this.

The patch is a bit larger than it had to be, because I
reordered a few lines in proc_special() to make it look more
like proc3(), and I removed an unused variable from proc1().

I've also tested the other cases where I know the special
palette is used. The patch makes a lot of difference for the
black light in the office, some difference to the tank in
the Hall of Oddities, and very little difference elsewhere
from what I can see.

comment:4 Changed 16 years ago by eriktorbjorn

Judging by a screenshot I found at www.tentakelvilla.de,
that positioning error is in the original as well. In that
case, we can always add a hack for it later. Maybe something
like:

// HACK: The tank in the Hall of Oddities isn't
positioned quite
// right. This appears to be a bug in the original
game as well.
if (_vm->_gameId == GID_SAMNMAX && _vm->_currentRoom
== 16 && _dirty_id == 5)
_xpos++;

right after setting _xpos and _ypos in
CostumeRenderer::mainRoutine().

comment:5 Changed 16 years ago by fingolfin

"<=" makes more sense to me. Compare also with the two
setupShadowPalette() methods (which are quite similar in function, it
seems, to createSpecialPalette...) and also darkenPalette() and
desaturatePalette().

In some ways, createSpecialPalette() looks like a more generic version
of the second setupShadowPalette(). Alas I only glimpsed at it, so I
might have missed something. I'll look closer later.

comment:6 Changed 16 years ago by fingolfin

I hope we can get a new version of this patch which works against latest
CVS? :-)

comment:7 Changed 16 years ago by fingolfin

Or rather asked: is this patch even necessary with latest CVS, or is it made
obsolete by your other costume patch? Almost looks like (except for the
gfx.cpp change).

comment:8 Changed 16 years ago by fingolfin

I applied the gfx.cpp change, looks good to me.

Changed 16 years ago by eriktorbjorn

Patch against a May 19 CVS snapshot

comment:9 Changed 16 years ago by eriktorbjorn

Yep, the costume.cpp changes are obsolete. I did find a few
more things to do in gfx.cpp though. Since this patch is
still open, I'll attach them here.

The patch makes a noticeable difference on the light ray at
Bumpusville, and a tiny difference on the black light at the
office.

comment:10 Changed 16 years ago by fingolfin

I have to wonder, is this even right:
int b = (int) (*curPtr++ * blueScale) >> 8;
or should it be:
int b = (int) (*curPtr++ * blueScale) / 255;

To find out, I guess we could just look at the various *scale parameters,
whether they max out at 255 or 256.

comment:11 Changed 16 years ago by eriktorbjorn

As far as I remember, for the Bumpusville ray one of the
scale parameters is 500. That's why the latest patch makes a
difference for that case.

One thing that struck me as a bit odd that some of the
palette functions check for "<= endColor" while some check
for "< endColor". The way Sam & Max uses
createSpecialPalette(), endColor is often (always?) 256, so
for that one it pretty much has to be "< endColor".

It would be nice if we could clean up these functions so
that they are written along the same pattern, if that's
possible.

comment:12 Changed 16 years ago by eriktorbjorn

Is there any reason to keep this open? I don't think I've
got anything to add to it any more.

comment:13 Changed 16 years ago by fingolfin

Owner: set to fingolfin
Status: newclosed

comment:14 Changed 8 months ago by digitall

Component: Engine: SCUMM
Game: Sam and Max
Note: See TracTickets for help on using tickets.