Opened 16 years ago
Closed 16 years ago
Last modified 4 years ago
#2894 closed defect (fixed)
IMUSE: Typo in sysex_scumm.cpp ?
|Reported by:||SF/albeu||Owned by:||sev-|
while digging the iMUSE code for ScummC i found some code that look very much like a typo. At the end of sysex_scumm.cpp (around line 190) there is:
case 96: // Set instrument part = player->getPart(p & 0x0F); a = (p & 0x0F) << 12 |(p & 0x0F) << 8 |(p & 0x0F) << 4 |(p & 0x0F);
Which should read a 16 bit value from the "one-nibble-per-byte" encoding used in the imuse sysex, but that should probably be:
a = (p & 0x0F) << 12 |(p & 0x0F) << 8 |(p & 0x0F) << 4 |(p & 0x0F);
If not then a comment would be a good idea because the code just doesn't look logical. I attached a patch with the fix above.
Ticket imported from: #1592006. Ticket imported from: bugs/2894.
Change History (9)
by , 16 years ago
comment:1 by , 16 years ago
I agree with your observations, the code looks fishy (good catch). But I also just verified that it has been just like this since the very first incarnation of that code. So *maybe* it is right, maybe it was a mistake made during RE.
So, two things could be done: Somebody checks against disassembly, or we try to find spots where this change would matter, i.e. where p&0xF != p&0x0F.
comment:2 by , 16 years ago
|Summary:||Typo in sysex_scumm.cpp ? → IMUSE: Typo in sysex_scumm.cpp ?|
comment:3 by , 16 years ago
A small update. Using some of my tools i checked all the SOUN ressource in dott and none of them use this SysEx.
by , 16 years ago
op96 disasm from dott imuse
comment:4 by , 16 years ago
I just checked against dott imuse disasm, p is clearly used here. Attached disasm for reference.
comment:5 by , 16 years ago
Ok, fixed in SVN. Thanks for reporting and cyx for research.
comment:6 by , 16 years ago
|Status:||new → closed|
comment:7 by , 4 years ago
|Component:||→ Engine: SCUMM|
Patch against r24646