OpenNBS / OpenNoteBlockStudio

An open-source Minecraft music maker.
https://noteblock.studio
MIT License
746 stars 51 forks source link

Pitch in editor doesn't match pitch in Minecraft #392

Open Fluffpumpkin opened 1 year ago

Fluffpumpkin commented 1 year ago

Describe the bug Pitch of the sound in the editor is exactly matching the midi file imported. Pitch in Minecraft after becoming a datapack is off.

To Reproduce Steps to reproduce the behavior:

  1. Import midi
  2. Export datapack
  3. Run the play command

Expected behavior I would expect the editor and Minecraft to play the same pitch for the notes

Additional context This is worse with custom instruments

Fluffpumpkin commented 1 year ago

I figured out the exact nature of the custom instruments part of the problem. Setting the pitch in the editor for custom instruments doesn't change the pitch outputted in game. The algorithm for that seems to not transfer over for some reason. This is quite a large issue as the editor upon export assumes every custom instrument is f# by default

Bentroen commented 1 year ago

Thanks for reporting the issue!

I can confirm this behavior. There's a long-standing issue where the in-game supported range is hardcoded to ≥ 33 and ≤ 57. This is indeed the case for all default instruments, but as soon as you add a custom instrument with a default pitch other than F#4, it no longer makes sense. There should be an offset applied to the key based on the custom instrument pitch (more specifically, how distant it is from the default key center, 45):

https://github.com/OpenNBS/OpenNoteBlockStudio/blob/45f35ea193268fb541c1297d0b656f4964339d97/scripts/dat_generate/dat_generate.gml#L16-L17

The reason we haven't worked on this up to this day is that this problem affects multiple features across the entirety of the program, including transposing notes to the supported range, the info that's displayed on note blocks, the red highlight in the out-of-range piano keys, etc. Once we patch this behavior in one place, we have to patch it everywhere, and since it requires a large number of code changes scattered across the codebase, I'm choosing to address this on the upcoming rewritten version of NBS (tracked in #4).