ONBS Crashes with Error upon Song Play

[bbayu123]

Describe the bug
ONBS just straight out crashes upon the below steps being executed with the following error:

___________________________________________
############################################################################################
FATAL ERROR in
action number 1
of Draw Event
for object obj_controller:

local variable e(100026, -2147483648) not set before reading it.
at gml_Script_control_draw
############################################################################################
--------------------------------------------------------------------------------------------
stack frame is
gml_Script_control_draw (line -1)
gml_Object_obj_controller_Draw_0

To Reproduce
Steps to reproduce the behavior:

1. Open the file attached below by double-clicking on it.
2. Click anywhere to dismiss the song information message.
3. Don't scroll, don't do anything, move your mouse to the play button, and press play.
4. The first line of notes play, then crash.

Expected behavior
Don't crash?

Screenshots
No screenshots, but I have a video.
https://youtu.be/RaH_QBgS4zU

Additional context
Windows 10 running OpenNoteBlockStudio Version 3.8.0
Does not happen when the song is scrolled (vertically) before playing the song.
I know the title isn't very descriptive, but I can't come up with one that's short yet descriptive.

Attachments:

1. The file I was testing with. Zipped because Github doesn't like nbs files.
   Through the Fire and Flames.zip

[Bentroen]

Thanks for reporting! This issue has been bugging us for a while, but with the info you provided it will be much easier to figure out what's causing it. :)

[Bentroen]

The crash is caused by variable d not being set in this line:
https://github.com/HielkeMinecraft/OpenNoteBlockStudio/blob/862ee521f239c600c53f21ef8f9714cdb08f3940/scripts/control_draw/control_draw.gml#L343

In reality, variables c and e aren't set properly at that point either. The reason why d causes the crash is that c is assigned some value somewhere higher on the script for a completely different purpose, specifically here:
https://github.com/HielkeMinecraft/OpenNoteBlockStudio/blob/862ee521f239c600c53f21ef8f9714cdb08f3940/scripts/control_draw/control_draw.gml#L277-L278
(This calculates the opacity of each note according to its velocity).

Yes, variable names are reused everywhere throughout the codebase and this is a really bad practice. If variables d and e were also set somewhere before, that bug would be almost impossible to track, as it would result in no crash, yet play notes with wrong velocity, panning and pitch values.

But what is the cause of those values being undefined at that point? They should be defined in a piece of code just a bit before:
https://github.com/HielkeMinecraft/OpenNoteBlockStudio/blob/862ee521f239c600c53f21ef8f9714cdb08f3940/scripts/control_draw/control_draw.gml#L319-L327

It runs under the condition b < endb2, that is, the current layer being analyzed is less than than the number of layers in the song. Under normal circumstances, it should always be smaller. For example, in a song with 14 layers, the value of endb2 should be 14, and the indices of the layers should go from 0 to 13.

For whatever reason, this is NOT the case in the attached song. Although the last layer containing a note block is 25, the value of endb2 is 24 (when it should be 25). The above snippet checks indices 0-24, but since 24 is not smaller than 24, the snippet of code that sets the velocity, panning and pitch values will not run, thus causing a crash when attempting to play the note.

The reason why scrolling down fixes the issue is that it creates more empty layers to be displayed on screen, thus the condition b < endb2 is always met and the variables c, d and e are defined properly, avoiding the crash.

This is possibly an issue with the MIDI import, or some other selection operation (i.e. moving note blocks) not setting the last layer properly. It was written to the file as being 24, (hex 18), not 25, which means the file is technically invalid.

To track down the issue, I would need the original imported MIDI and the procedures done on the song to achieve that result. Like I said, at some point in time some operation did not update the layer count properly, thus causing this whole ordeal.

Since it would probably be difficult to obtain this information at this point, I'll leave this issue on hold until I can investigate it further.

[bbayu123]

For reference, the song was downloaded from this file hosting site: https://host-a.net/f/90042-dragonforcethrough-the-fire-and-flamesnbs
The website that points to this file is the original NBS forum post on Minecraft Forums, under the section "Note Block Songs". Linked below via WaybackMachine.
http://web.archive.org/web/20171224051842/https://www.minecraftforum.net/forums/mapping-and-modding-java-edition/minecraft-tools/1260747-minecraft-note-block-studio

I'm assuming that originally, NBS never crashed when playing this file. Not to say that the file was fine, but NBS probably did something different back then and never had something like that, which is probably why it was listed on the forums. If you want to track down the original source, you might need to reach out to the original creator on the forums.
It would be interesting if out of all the songs on that forum post, only this one has the issue...

Edit: More information: Looking into the original file with a text editor... actually it's just Sublime Text, and comparing it with the original file format, the "song height" value is indeed set to hex 0018, or decimal 24.

Sidenote: It's interesting to see that in the FAQ of the original forum post, one of the solutions of "I'm getting errors/crashes" (Q.1) is to "Download an earlier version". I wonder if they knew that some NBS files would cause issues to surface in later versions... 🤔

[bbayu123]

Update: I believe it should be this exact midi file... again zipped because screw the upload file type restrictions (lol), but trying to import the midi with MCNBS version 3.2.1 and saving the file shows it does indeed write the intended hex 0019... so the problem might be a much earlier version had this problem, and it (the wrong written byte) persisted even until today...
For reference, I have also attached the nbs file I made using 3.2.1, also in the same zip, named Test.nbs (yeah I know, not a very creative name lol). Also, the midi file was obtained from here: http://www.hamienet.com/midi33557_Dragonforce--Through-Fire-and-Flames.html (you might need to access the link again after the captcha prompt if any before seeing the correct page)

Attachments

TTFAF More Testing.zip

[OctoFlareDev]

It's fine for me on release 3.8.2

[bbayu123]

It's fine for me on release 3.8.2

Nope, just tested, no change.

The file that was generated from MCNBS 3.2.1 still crashes on immediate playback.
I know the intended procedure is to regenerate the NBS file using the MIDI file, but the NBS file was publicly published on the former MCNBS forum on Minecraft Forums, which can still be accessed through the Wayback Machine, so in a sense, it was "officially" recognised as "example music"(?), and for that file to crash ONBS doesn't seem like a good idea.

I am aware that being that the bug belongs in the file rather than in the code itself, the resolution of this should be fixing the file, yet I can't help but wonder if it is possible to do in all circumstances. This time, I was able to dig out the original MIDI file (mind you, it wasn't an easy task), but I won't be so lucky next time.

I believe that since MCNBS 3.2.1's import system was the cause of this irregular byte, so ONBS should have some way to fix the mistakes made by the past. I have no idea how many files might be affected by this, could be just this one file, could be more, after all this was the only file that had this issue on the original forum post. If the file was corrected, let's say when you opened it in a newer ONBS version and saved it, thus updating the version of the file, then I can envision that the remnants of the MCNBS 3.2.1 import bug could finally be fully eradicated.

[OctoFlareDev]

I have a video here showing it not crash when loading
I loaded the nbs file up there
I loaded the one I generated in 3.2.1
I loaded the nbs file up there using the development branch

[bbayu123]

I have a video here showing it not crash when loading
I loaded the nbs file up there
I loaded the one I generated in 3.2.1
I loaded the nbs file up there using the development branch

You were able to have it working because Layer 25 was visible on your screen.

@Bentroen mentioned the following in his reply:

The reason why scrolling down fixes the issue is that it creates more empty layers to be displayed on screen, thus the condition b < endb2 is always met and the variables c, d and e are defined properly, avoiding the crash.

In your scenario, all the layers were visible when you loaded the song, which meets the condition to prevent the crash.

In my video, I do not have Layer 25 visible on screen, only up to Layer 22. I would suggest you try again with less visible layers on screen and see if you can reproduce it.

[mckuhei]

ok,there were have a smple way to repair file.

repair.mp4

[OctoFlareDev]

Should be j u s t fine now
https://www.youtube.com/watch?v=FMoGvAZQke8