Opened 15 years ago
Closed 7 months ago
#4535 closed defect (fixed)
SCUMM: MI (Amiga) - Voodoo lady palette glitches
| Reported by: | SF/quietust | Owned by: | athrxx |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: | Monkey Island 1 |
Description
Several spots in the game exhibit significant palette glitches, possibly related.
1. When entering the forest maze, the yellow flowers aren't yellow, but are being colored red instead. 2. When talking to the voodoo lady, the entire screen flashes blue in a rather glitchy way when the boiling pot rises out of the floor or retracts back in.
Ticket imported from: #2838205. Ticket imported from: bugs/4535.
Change History (20)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
could you, please, try GSoC Amiga modules branch? There were palette-related fixes. The code is in branches/gsoc2009-mods
comment:3 by , 15 years ago
That's actually where I first noticed the problem (so nice to finally have music and sound effects in the game), and I double-checked with the base version to verify that it also happened there - a quick recheck indicates that the latest svn for gsoc2009-mods also exhibits the problem.
comment:4 by , 15 years ago
Further inspection reveals that this apparently stopped working between the 0.7.1 release and 0.8.0. Best I can tell, o5_roomOps() case 2 (SO_ROOM_COLOR) seems to not be working as it should (I suspect _paletteMod is confusing things).
comment:5 by , 15 years ago
I guess it must be some regression introduced in one of those functions using "_roomPalette" in gfx.cpp.
It seems 0.7.1 still had this code in the Gdi constructor:
_roomPalette = vm->_roomPalette; if ((vm->_features & GF_AMIGA) && (vm->_version >= 4)) _roomPalette += 16;
While 0.8.0 started to use just:
_roomPalette = vm->_roomPalette;
To compensate that it seems there was "_paletteMod" introduced, which is set in "Gdi::decompressBitmap". It seems some of the functions called from Gdi::decompressBitmap were changed to use "_paletteMod" in their "_roomPalette" access.
Here's an example of drawStripEGA:
0.7.1:
*(dst + y * dstPitch + x) = (z & 1) ? _roomPalette[color & 0xf] : _roomPalette[color >> 4];
0.8.0:
*(dst + y * dstPitch + x) = (z & 1) ? _roomPalette[color & 0xf] + _paletteMod : _roomPalette[color >> 4] + _paletteMod;
I have to say that looks strange. For example why isn't the "+ _paletteMod" after the indexed access to _roomPalette and not added to the index, which looks like what the 0.7.1 code did?
Maybe there is a good explanation for this, I don't know whether the palette handling itself changed during 0.7.1 to 0.8.0 changes. But the above mentioned looks odd.
Also maybe some other functions called from "Gdi::decompressBitmap" need adaption to take "_paletteMod" into calculation, it looks to me like not every function was changed. Those functions are:
unkDecode8 unkDecode9 unkDecode10 unkDecode11 drawStrip3DO drawStripHE
Judging by the names the latter 3 should not be used in MI1 Amiga though. The former four seem to be only used by FM-Towns games, according to the comments. So probably they do not need to be changed.
There's another function accessing _roomPalette and not taking _paletteMod into calculation:
drawStripC64Background
Maybe that's just fine though, since it seems for C64 "_paletteMod" is always 0 :-).
Anyway I guess the problem lies in the above mentioned change in "drawStripEGA" (and maybe the other functions changed in a similar way). I do not have MI 1 Amiga to test that though, so it would be nice if someone with that game version could look into it. Personally I would for example just change the above mentioned line in drawStripEGA to:
*(dst + y * dstPitch + x) = (z & 1) ? _roomPalette[(color & 0xf) + _paletteMod] : _roomPalette[(color >> 4) + _paletteMod];
And of course all other places doing the same addition of _paletteMod after _roomPalette index look up :-). Maybe that fixes the problem or it will just look even more incorrect and thus the problem is caused by another change...
comment:6 by , 15 years ago
Your analysis is 100% correct. In particular, in the old code base, the _palette_mod was indeed added inside the array access, which is the only logical way, too ;). See also <http://scummvm.svn.sourceforge.net/viewvc/scummvm/scummvm/trunk/engines/scumm/gfx.cpp?r1=10776&r2=10778>
So please go ahead and fix it ;).
comment:7 by , 15 years ago
| Owner: | set to |
|---|
comment:8 by , 15 years ago
I did commit it to trunk now, would be still nice if someone could verify it and when it's working just backport it to branch maybe.
comment:9 by , 15 years ago
Talking with Kirben showed that my simple patch did cause a regression in FOA/Amiga. I also fixed that and backported it to 1.0.0.
Kirben also pointed out that the voodoo lady issue is still present (as it was in 0.7.1 according to him).
I'll leave this bug report open till that is fixed. I'm also reassigning the report to "None", since I do not have any way to fix that issue :-).
comment:10 by , 15 years ago
| Owner: | removed |
|---|
comment:11 by , 15 years ago
There were no effects when the Voodoo lady rises and retracts from the floor, in the original Amiga version. So it looks like some effects were disabled in the Amiga version, and need to be disabled under ScummVM too.
I noticed that roomOps case 4 is been used at these points, so maybe_shadowPalette was not used in Amiga version?
comment:12 by , 15 years ago
| Summary: | MONKEY: Palette glitches in Amiga version → MONKEY: Voodoo lady palette glitches in Amiga version |
|---|
comment:13 by , 15 years ago
Possible. Only way to know for sure is to look at the disassembly, I guess. But in the meantime, you could try modifying / disabling roomOps case 4, resp. shadowPalette handling, to see if that resolve the issues. If it does, then we'd also have to test the rest of the game for regressions, though :)
comment:16 by , 13 years ago
This bug is nice to get fixed before the release. Raising priority for keeping the track.
comment:17 by , 13 years ago
| Priority: | normal → high |
|---|
comment:18 by , 12 years ago
| Priority: | high → normal |
|---|
comment:19 by , 4 years ago
| Summary: | MONKEY: Voodoo lady palette glitches in Amiga version → SCUMM: MI (Amiga) - Voodoo lady palette glitches |
|---|
ScummVM 2.2.0git (Jul 15 2020 10:24:49)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC A/52 FreeType2 FriBiDi JPEG PNG cloud (servers, local)
Confirmed still an issue with above ScummVM version.
The Secret of Monkey Island (Amiga/English)
AmigaOS4 - PPC - BE - SDL
comment:20 by , 7 months ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
I have made a fix. The original interpreter ignores the shadow palette.
Also, this is happening with latest code from SVN.