Skip to content

Commit

Permalink
SCI: (KQ5 FM-Towns) - fix voice mapping
Browse files Browse the repository at this point in the history
(Driver channels would get reserved via the 0x4b control, but they would never get released)
  • Loading branch information
athrxx committed Nov 1, 2011
1 parent 6a91508 commit 9b83823
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
24 changes: 22 additions & 2 deletions engines/sci/sound/music.cpp
Expand Up @@ -44,8 +44,10 @@ SciMusic::SciMusic(SciVersion soundVersion)
// operations
_playList.reserve(10);

for (int i = 0; i < 16; i++)
for (int i = 0; i < 16; i++) {
_usedChannel[i] = 0;
_channelRemap[i] = -1;
}

_queuedCommands.reserve(1000);
}
Expand Down Expand Up @@ -355,6 +357,7 @@ int16 SciMusic::tryToOwnChannel(MusicEntry *caller, int16 bestChannel) {
if (!_usedChannel[bestChannel]) {
// currently unused, so give it to caller directly
_usedChannel[bestChannel] = caller;
_channelRemap[bestChannel] = bestChannel;
return bestChannel;
}
// otherwise look for unused channel
Expand All @@ -363,6 +366,7 @@ int16 SciMusic::tryToOwnChannel(MusicEntry *caller, int16 bestChannel) {
continue;
if (!_usedChannel[channelNr]) {
_usedChannel[channelNr] = caller;
_channelRemap[bestChannel] = channelNr;
return channelNr;
}
}
Expand All @@ -375,8 +379,24 @@ int16 SciMusic::tryToOwnChannel(MusicEntry *caller, int16 bestChannel) {
void SciMusic::freeChannels(MusicEntry *caller) {
// Remove used channels
for (int i = 0; i < 15; i++) {
if (_usedChannel[i] == caller)
if (_usedChannel[i] == caller) {
if (_channelRemap[i] != -1) {
// athrxx: The original handles this differently. It seems to be checking for (and effecting) necessary
// remaps / resets etc. more or less all the time. There are several more tables to keep track of everything.
// I don't know whether all of that is needed and to which SCI versions it applies, though.
// At least it is necessary to release the allocated channels inside the driver. Otherwise these channels
// won't be available any more (e.g. after half of the KQ5 FM-Towns intro there will be no more music
// since the driver can't pick up any more channels). The channels also have to be reset to
// default values, since the original does the same (although in a different manny) and the music will be wrong
// otherwise (at least KQ5 FM-Towns).

sendMidiCommand(0x4000e0 | _channelRemap[i]); // Reset pitch wheel

This comment has been minimized.

Copy link
@OmerMor

OmerMor Nov 2, 2011

Contributor

why not assign a name to the magic numbers (e.g 0x4000e0)?

This comment has been minimized.

Copy link
@wjp

wjp Nov 2, 2011

Contributor

They're midi commands rather than magic numbers, though.

This comment has been minimized.

Copy link
@OmerMor

OmerMor Nov 2, 2011

Contributor

Yeah I know. but instead of using raw numbers in the code to represent those midi commands, won't it be clearer to #define it a meaningful name?

This comment has been minimized.

Copy link
@lordhoto

lordhoto Nov 2, 2011

Contributor

That's what the comments are for.

This comment has been minimized.

Copy link
@OmerMor

OmerMor Nov 8, 2011

Contributor

that's one option. another is to write self documenting code, which in this example would mean named constants that would make the comments unnecessary (http://en.wikipedia.org/wiki/Self-documenting). However it's subjective, there's no single truth here, and since I'm not a dev in this project, I'm not at liberty to judge.

sendMidiCommand(0x0040b0 | _channelRemap[i]); // Release pedal
sendMidiCommand(0x004bb0 | _channelRemap[i]); // Release assigned driver channels
}
_usedChannel[i] = 0;
_channelRemap[i] = -1;
}
}
// Also tell midiparser, that he lost ownership
caller->pMidiParser->lostChannels();
Expand Down
1 change: 1 addition & 0 deletions engines/sci/sound/music.h
Expand Up @@ -219,6 +219,7 @@ class SciMusic : public Common::Serializable {
bool _soundOn;
byte _masterVolume;
MusicEntry *_usedChannel[16];
int8 _channelRemap[16];
int8 _globalReverb;

MidiCommandQueue _queuedCommands;
Expand Down

0 comments on commit 9b83823

Please sign in to comment.