Opened 18 years ago

Closed 18 years ago

Last modified 2 years ago

#8226 closed patch

proc_special() / _proc_special_palette[] experimental fix

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Version: 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 18 years ago.
Patch against a May 17 CVS snapshot
spec-palette2.diff (4.2 KB ) - added by eriktorbjorn 18 years ago.
Revised patch against a May 17 CVS snapshot
spec-palette-refinement.diff (709 bytes ) - added by eriktorbjorn 18 years ago.
Patch against a May 19 CVS snapshot

Download all attachments as: .zip

Change History (17)

by eriktorbjorn, 18 years ago

Attachment: spec-palette.diff added

Patch against a May 17 CVS snapshot

comment:1 by eriktorbjorn, 18 years ago

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

comment:2 by eriktorbjorn, 18 years ago

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

by eriktorbjorn, 18 years ago

Attachment: spec-palette2.diff added

Revised patch against a May 17 CVS snapshot

comment:3 by eriktorbjorn, 18 years ago

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 by eriktorbjorn, 18 years ago

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 by fingolfin, 18 years ago

"<=" 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 by fingolfin, 18 years ago

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

comment:7 by fingolfin, 18 years ago

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 by fingolfin, 18 years ago

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

by eriktorbjorn, 18 years ago

Patch against a May 19 CVS snapshot

comment:9 by eriktorbjorn, 18 years ago

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 by fingolfin, 18 years ago

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 by eriktorbjorn, 18 years ago

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 by eriktorbjorn, 18 years ago

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

comment:13 by fingolfin, 18 years ago

Owner: set to fingolfin
Status: newclosed

comment:14 by digitall, 2 years ago

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