-
-
Notifications
You must be signed in to change notification settings - Fork 53
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
Issues to be addressed with NBS format version 6 #307
Comments
First of all, I definitely support the idea of the format discussion and I think it may bring some really good things to the .nbs format.
This is definitely interresting issue and as far as I remember it is also inconsistently used. NoteBlockAPI use the 'Sound file' property for custom instrument's since there is lower probabilty that it will be in incorrect format (OpenNoteBlockStudio's default custom instrument name is not allowed in Minecraft).
Most of the songs I came across didn't even have their metadata field filled but I agree that on the specification level this may be a problem. The first obvious idea is to switch to UTF-8 which is completely universal when it comes to letters, symbols and whatever may user enter. However, it's decoding is definitely more complicated than ASCII since it's not only multibyte but also with different count of bytes per symbol. For Java parsers like NoteBlockAPI this shouldn't be a problem.
I think that the fact that NBS files are binary is a good thing and I would definitely stick to it because any textual format would make the files a lot bigger. The solution to the issue of future proof implementation also is not as complicated and have been used in many file formats over the years. The NBS file is already splited into blocks (metadata block, note blocks, layer blocks, custom instrument blocks) if you add a length information in bytes to the beginning of each block, you can later add any field you want to the end of the block and properly implemented parsers should simply seek to the end of the block and ignore these new fields.
Actually, this is not really true. There are hard limits in the NBS specification. Since e.g. song's length is signed short in the specification you simply can't have a song longer than 32 767 ticks even if your parser would be able to handle it. And the same holds for every numeric field. I think that especially numbers like song's length (short), instrument's identifier (byte), custom instrument's count and identifier (both byte) should be changed to at least int or short. From the point of view of the NoteBlockAPI developer, the rest of the issues is not very important for me. Layer lock was ignored so far because it doesn't make much sense to lock some layer in the exported song (and there is also no reason why would you export song with multiple layers but only want to play only one layer). Since the tempo is stored as ticks per second in file the issue with BPM is also not relevant for me. NoteBlockAPI already supports future instruments so that is also not an issue for me. |
I believe this would be a great feature to keep and document, as I and many others mute incomplete sections of the song or single out certain layers. The lack of documentation creates inconsistencies between ONBS, NoteBlockAPI, NBS.js, and whatever other
I believe this is up to the library developers and users to deal with. Every implementation of the format would be different either way. NBAPI would have to map to Minecraft's sound events, NBS.js has to load instruments from an ArrayBuffer somehow, etc. The only solution I'd have in mind is to include the instrument files directly in the file, but that'd add an extreme amount of bloat fast. Potentially a way to export the song and its instruments separately from saving the song?
Saving with a JSON-like format would be much easier to read, manipulate, and write. However, as @koca2000 said, the file size would explode. While I was working on the playlist feature of NBSPlayer, I attempted to save the loaded songs as their loaded JSON form. The resulting file was megabytes larger than all the loaded There's most likely a better way to keep the format future-proof while sticking with the binary format. For example, containing "version" blocks at the end of the file, such as a "v1" block, "v2", and so on. The parser can stop when it's finished reading all the version data it needs, and won't run into issues with any unimplemented versions.
This could be easily solved by making BPM the primary tempo format. Load as BPM, then calculate and save the TPS from said BPM. I greatly appreciate the new approach on spec changes. As the community expands and more parsers are developed, this will ensure the future stability of the format and its parsers! |
Thank you @koca2000 and @encode42 for your input! It's great to have different perspectives on the problem.
This goes to show how important proper standardization is. We've had two different implementations having to come up with workarounds for issues with the format in their own way!
I can confirm we won't be moving to a different format. Not only the increased file size would be undesirable, but this would mean an added difficulty for plugin makers to maintain two different formats. That being said, JSON could be used in the future as an intermediate format for saving project files. (I'll get back to this!)
This is probably the solution we're gonna go with. Though, we must also clearly determine how existing songs have to be interpreted, as for example pynbs crashes when reading a field containing non-ASCII characters due to not specifying an encoding (see OpenNBS/pynbs#3).
This is planned as a separate file format! See #239.
These are both great ideas, but I'm wondering if they can be implemented in a successful way, given the kinds of changes we might have to make. For example, if you look at the kinds of fields that have been added over the past few NBS versions, not all of them are simply part of a block, but they are compound fields that appear once for every instance of an element (e.g. layer lock, note block pitch). So to make this work while allowing new fields to be added anywhere -- even in note blocks or layers --, we would probably have to repeat this procedure on multiple levels. Not only this, but some of the changes could even modify the way some fields are interpreted: see, for example, @encode42's suggestion:
Considering changes of this nature could be needed even after the future-proof update, no change in the file structure would account for this in a way that would avoid the need for parser updates, unless we marked the existing field as deprecated and re-added it later in the file. Accounting for all of this would increase the format's complexity in such a way that it makes me wonder whether it's worth to make the format future-proof at all. On the long term it would be a wise idea, but on the other hand, the number of fields we could potentially still add is so small that it possibly doesn't justify such a large structural change.
And this brings me to the third point: NBS files are already kinda saturated with features. Anything we could add further on from this point would probably be irrelevant for NBS players -- as mentioned by @koca2000, features such as layer lock already are. There are two solutions for this: one is to delay NBS version 6 so we have enough time to think of all features that could potentially cause breaking changes, and add them to the format in that version. The other, and more ideal, one, would be to split responsibilities: create a new type of file called So, here are the key takeaways of this:
NB: in the past, I've done a detailed write-up on both character encoding and layer lock, but on both occasions I've lost them due to typing directly on the GitHub form. :( |
Prevents `UnicodeDecodeError: invalid continuation byte` when decoding non-ASCII characters, due to a bug in Note Block Studio where only the first byte of the character's UTF-16 codepoint is written to the file, resulting in an invalid UTF-8 codepoint. This will be patched in a future version of the NBS format (see OpenNBS/NoteBlockStudio#307). `cp1252` was chosen to closely match the current (unintended) interpretation of those characters by Note Block Studio, given the fact that it has always been a Windows-only application. Also, unlike ISO-8859-1/Latin-1, all characters in the `0x8A-0xFF` range are printable characters (whereas `0x80-0x9F` are control characters in Latin-1). By no means this is an ideal solution -- it's nothing short of a workaround to prevent that exception, until the issue is properly fixed in Note Block Studio. As the information in those characters is already lost the moment they are saved to the file, it's preferable to have the read process yield "garbage" characters than incur in an exception. Fixes #3
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Overview
Since the transition from MCNBS to ONBS, many updates to the .nbs format have been released. While those updates were necessary to support new features being added to the program, new NBS versions have been, so far, released on an "as necessary" basis, without any specific criteria or public notice prior to the official release.
Ideally, releasing a new version of a widely adopted format should be preceded by public discussions with its users. In the case of the NBS format, these would include plugin developers, as well as anyone creating software based on the format. The release schedule should also include an adaptation period, so developers have enough time to update plugins to comply with the new version.
In practice, this was never the case with the format versions released to this date. This impairs the widespread adoption of the format and causes plugins to be automatically outdated as a new version is released, to the detriment of their users and the NBS ecosystem as a whole.
Starting in NBS version 6, the project should strive to be as clear and transparent as possible about the upcoming changes, as well as take community input into consideration as much as possible. This way, future changes to the format can be bundled up in fewer version updates released as sparingly as possible.
This issue is an attempt to gather all the current issues with the NBS format as of version 5, as well as suggestions for new features that could potentially be included in version 6. Keep in mind this is only an overview of the current issues -- where necessary, more detailed write-ups on each of these issues may be written, including their possible workarounds.
If you're a developer creating something based on the .nbs format and would like to suggest a change, feel free to send it in the comments below!
Format issues
0x00
-0x7F
), the equivalent ASCII character is displayed; otherwise, the displayed character is whatever ANSI character Windows maps that codepoint to. This can be fixed by encoding strings in UTF-8. Once this issue is fixed, the documentation should indicate how invalid ASCII characters should be treated in versions 0-5.Documentation issues
Feature suggestions
The text was updated successfully, but these errors were encountered: