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

Merge code for better sounds #58

Closed
Robgus79 opened this issue Mar 23, 2023 · 16 comments
Closed

Merge code for better sounds #58

Robgus79 opened this issue Mar 23, 2023 · 16 comments

Comments

@Robgus79
Copy link

Can someone please merge Schendel's code https://github.com/pvanschendel/Arcade-DonkeyKong_MiSTer with the current core for better sounds.

Topic for reference: https://misterfpga.org/viewtopic.php?p=70733#p70733

Thanks!

@sorgelig
Copy link
Member

try this build with merged changes:
Arcade-DonkeyKong_t1.zip

@lagomorph
Copy link
Contributor

No music and also missing some sound effects. Terribly squeaky running sound originally introduced in #51 is not much changed.

@sorgelig
Copy link
Member

Is it worse now?

@lagomorph
Copy link
Contributor

I can't really say if the running sound is better, sounds mostly the same to me. The biggest issue is that there's no music and some of the sound effects are also missing. I'm guessing this was still a work in progress.

@Robgus79
Copy link
Author

Thanks! I had music when I tried it just now but it sounds like it did before Schendel implemented this, which in my opinion is better: https://github.com/pvanschendel/Arcade-DonkeyKong_MiSTer/blob/Improve_synthesizer_sound/releases/Arcade-DonkeyKong_20221120.rbf

@sorgelig
Copy link
Member

Thanks! I had music when I tried it just now but it sounds like it did before Schendel implemented this, which in my opinion is better: https://github.com/pvanschendel/Arcade-DonkeyKong_MiSTer/blob/Improve_synthesizer_sound/releases/Arcade-DonkeyKong_20221120.rbf

i didn't understand your message.

@Robgus79
Copy link
Author

What I meant was that the latest official update that came a few days ago still has music in my setup when I tried it today. (Lagomorph said that he didn't have music) Though it is still the "older" version of music that was before Schendel implemented his sounds.

Hope it clarifies. Thanks!

@lagomorph
Copy link
Contributor

lagomorph commented Mar 23, 2023

What I meant was that the latest official update that came a few days ago still has music in my setup when I tried it today. (Lagomorph said that he didn't have music) Though it is still the "older" version of music that was before Schendel implemented his sounds.

Hope it clarifies. Thanks!

No. The code you wished to be committed in this PR is what I'm referring to. Why would I refer to something else? You obviously did not test what's in this PR.

@sorgelig
Copy link
Member

What I meant was that the latest official update that came a few days ago still has music in my setup when I tried it today. (Lagomorph said that he didn't have music) Though it is still the "older" version of music that was before Schendel implemented his sounds.

Hope it clarifies. Thanks!

how about rbf in zip file i've posted above? Did you test it?

@Robgus79
Copy link
Author

No I didn't. I tested the core after I ran the update script. Thought it was the same.

@sorgelig
Copy link
Member

after release 20221120 there were 2 additional changes. May be they introduced regression.

@thorr2
Copy link

thorr2 commented Mar 29, 2023

I also tried the Arcade-DonkeyKong_t1.zip above and most of the sound is missing. schendel's version is great, so hopefully this can get properly integrated with the latest updates. I personally like the newer version of the walking sound. This is what it should sound like, and it is pretty close to my ears. https://youtu.be/tJagEKVJ8x4?t=56

@pvanschendel
Copy link
Contributor

pvanschendel commented Jan 25, 2024

It looks like a work-in-progress source code was pulled from my repository. This caused sound in new compiles to fail.
I fixed that issue, and a few other ones here:

pvanschendel@e30b060

At request of @thorr2, I added the user option to choose either sampled or emulated sound effect (currently only walk sound), with sampled sound as default (for now). That should fix issue #49 .

Do you think this is OK, and when yes, should I do a pull request to get this merged?

@sorgelig
Copy link
Member

If your change fixes some problems then you are welcome to do a pull request

@Robgus79
Copy link
Author

Robgus79 commented Jan 28, 2024

Thanks a lot for this, wonderful work! For some reason the general volume is lower in this version than the previous that I had... or is that just me?

Edit. Disregard what I said about the volume, it was something with my amplifier...

All the best!

@sorgelig
Copy link
Member

Looks like fixed. Thanks @pvanschendel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants