#2894 closed defect (fixed)
IMUSE: Typo in sysex_scumm.cpp ?
| Reported by: | SF/albeu | Owned by: | sev- |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Hi,
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[0] & 0x0F); a = (p[1] & 0x0F) << 12 |(p[2] & 0x0F) << 8 |(p[4] & 0x0F) << 4 |(p[4] & 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[1] & 0x0F) << 12 |(p[2] & 0x0F) << 8 |(p[3] & 0x0F) << 4 |(p[4] & 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.
Attachments (2)
Change History (9)
by , 18 years ago
| Attachment: | sysex_scumm-typo.diff added |
|---|
comment:1 by , 18 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[3]&0xF != p[4]&0x0F.
comment:2 by , 18 years ago
| Summary: | Typo in sysex_scumm.cpp ? → IMUSE: Typo in sysex_scumm.cpp ? |
|---|
comment:3 by , 18 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.
comment:4 by , 18 years ago
I just checked against dott imuse disasm, p[3] is clearly used here. Attached disasm for reference.
comment:6 by , 18 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:7 by , 6 years ago
| Component: | → Engine: SCUMM |
|---|
Patch against r24646