#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)
Change History (17)
by , 22 years ago
| Attachment: | spec-palette.diff added |
|---|
comment:3 by , 22 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 , 22 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 , 22 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 , 22 years ago
I hope we can get a new version of this patch which works against latest CVS? :-)
comment:7 by , 22 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).
by , 22 years ago
| Attachment: | spec-palette-refinement.diff added |
|---|
Patch against a May 19 CVS snapshot
comment:9 by , 22 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 , 22 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 , 22 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 , 22 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 , 22 years ago
| Owner: | set to |
|---|---|
| Status: | new → closed |
comment:14 by , 6 years ago
| Component: | → Engine: SCUMM |
|---|---|
| Game: | → Sam and Max |
Patch against a May 17 CVS snapshot