Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCI: (FPFP/Demo) - fix bug no. 12610 #4270

Merged
merged 1 commit into from Sep 15, 2022
Merged

Conversation

athrxx
Copy link
Member

@athrxx athrxx commented Sep 14, 2022

("hanging MIDI notes")

The original interpreter resets the channels more often than we do
in the remap function. The assumption apparently was that the
loop at the very end of the function would catch everything. But
it does not catch the dontRemap channels if they are not within
the _driverFirstChannel/_driverLastChannel range.
If more bugs like this come up it might be necessary to add even
more resets, but I am very reluctant about unnecessary changes
to the remap function. And this code has been around for a long
time without any other bug reports of this sort. So I think we have
reason to be optimistic about it.

("hanging MIDI notes")

The original interpreter resets the channels more often than we do
in the remap function. The assumption apparently was that the
loop at the very end of the function would catch everything. But
it does not catch the dontRemap channels if they are not within
the _driverFirstChannel/_driverLastChannel range.
If more bugs like this come up it might be necessary to add even
more resets, but I am very reluctant about unnecessary changes
to the remap function. And this code has been around for a long
time without any other bug reports of this sort. So I think we have
reason to be optimistic about it.
@sluicebox
Copy link
Member

sluicebox commented Sep 14, 2022

Great! I will test this tonight against the FPFP Demo and the song in the full versions. I'll also run through LB2 since I fixed several music bugs there and now suffer a hyper-sensitivity to what notes should play and when. =)

@sluicebox
Copy link
Member

sluicebox commented Sep 15, 2022

Confirmed that this fixes the stuck note(s). The FPFP Demo intro is completely different than the real version so that explains the discrepancy between them. I played through the LB2 sections with complicated SCI1.1 music and everything sounded correct; that's my stress test for sound changes like this.

Thank you again, our low-level audio hero!

@sluicebox sluicebox merged commit 2caa572 into scummvm:master Sep 15, 2022
8 checks passed
@athrxx athrxx deleted the sci-fpfp-remap branch Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants