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

Transfer the autosave status/interval byte to user storage rather than within the NBS file #445

Open
CreeperPookie opened this issue Mar 30, 2024 · 2 comments
Labels
C: NBS format change Suggestions or issues that require changes to the NBS format

Comments

@CreeperPookie
Copy link

CreeperPookie commented Mar 30, 2024

Describe the enhancement you'd like
One thing I noticed when debugging a partially corrupt NBS file (not the program's fault, don't worry lol) is that the autosave status is stored in the file. Since it's already deprecated and unused, this would be a great opportunity to remove it entirely for NBS v6. Also, if the byte(s) were ever to be re-used in the future, it could cause a conflict with the user's own autosave setting, and could raise the question of which should take priority.

I realize this should probably be in the NBS v6 discussion #402, but given this is such a minor change it felt more appropriate to make a feature request for this.

@Bentroen
Copy link
Member

Hi, thanks for your suggestion!

The reason auto-save and auto-save interval were not removed from the format, although deprecated, is to avoid changing the internal song format structure as much as possible, unless it is absolutely necessary. When adding a new feature, one has no choice but to add a new field somewhere. But it causes no harm to keep a feature that's no longer used.

If anything, it should make it easier for parsers to implement full version range compatibility, since 1) it avoids one more conditional if statement to not encode the information based on the version, and 2) their internal representation can keep track of the auto-save value for the purpose of saving the song in older version, even if the song in question comes from a recent version. Although the field is deprecated in modern versions, songs can export the user's global auto-save setting, so that, should the song ever be saved in an older version, the behavior they had set up the moment the file was saved will be preserved in earlier versions of the program.

It also makes the documentation simpler to mark the field as deprecated rather than, somehow, indicating that it is simply absent. Additions to a specification are somewhat "permanent", i.e. once you add it, there's no going back — we'll always carry the burden of having to maintain information regarding that choice, whether it's to tell it was deprecated or removed.

On the other hand, I agree keeping unused information around can litter the file and make the song's structure confusing, so I'll consider adding this topic to the NBS version 6 proposal when the time comes. Thanks again for your careful consideration! :)

@Bentroen Bentroen added the C: NBS format change Suggestions or issues that require changes to the NBS format label Mar 30, 2024
@Bentroen
Copy link
Member

Bentroen commented Mar 31, 2024

And just to clarify, the auto-save and auto-save duration settings are already stored in user preferences – the values written to saved songs are just redundance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: NBS format change Suggestions or issues that require changes to the NBS format
Projects
None yet
Development

No branches or pull requests

2 participants