Skip to content

Save Scale/Key Highlighting, and Many More Editor Options. #7854

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

regulus79
Copy link
Contributor

@regulus79 regulus79 commented Apr 19, 2025

This PR makes it so that the scale/key/chord in the Piano Roll is saved with the project.

Also, while I was at it, I made it so that the zoom, vertical zoom, snap settings, quantization, and note lengths are saved too.

Aaand I also decided to save the zoom and snap size in the Song Editor because why not :D

However, the zoom/snap/etc are saved in the lmmsrc file, since it doesn't really make sense to save them per-project.

Note

It seems odd that the song editor is saved as a <trackcontainer> in the xml, since it's the Editor which does the saving, not the editor window. This means that I cannot save the state of the proportional zoom option, since that is not accessible from the editor object, only the window. However, the piano roll does it via the editor window. It feels like we should be doing it that way via the gui, since saving isn't something which should be in the core(?) (or should it? idk)

@AW1534
Copy link
Member

AW1534 commented Apr 20, 2025

Tested, works well.

@regulus79
Copy link
Contributor Author

Also, currently I am saving the values as attributes of the editor's xml element. However, this means that any automation connections will not be saved. Are we okay with this? Or should I try to save each model using their own saveSettings/loadSettings functions? If so, I may need to edit them to add a way to specify a default value, since currently that's not possible (and we probably want that so that old projects get loaded with reasonable values.

@Jernemies
Copy link

Jernemies commented Apr 29, 2025

Scale highlights are a good idea. For all the other stuff like zoom, scroll mode and stuff it seems like a more sensible solution would be saving currently used settings in something like lmmsrc.xml instead. Saving them per project doesn't make much sense because old projects will have to be adjusted one by one. These defaults are entirely user preference anyways, and in case of something like a collaboration, it will present an additional step user has to do every time the project is passed back and forth

@regulus79
Copy link
Contributor Author

Scale highlights are a good idea. For all the other stuff like zoom, scroll mode and stuff it seems like a more sensible solution would be saving currently used settings in something like lmmsrc.xml instead.

After thinking about this, you're right. It doesn't make sense to save those kind of ui things per-project. I've changed it in the latest commit to save it in the lmmsrc file, so now whatever zoom/snap/quantization/notelength you have last will be restored when you open lmms again.

sakertooth
sakertooth previously approved these changes May 3, 2025
@sakertooth sakertooth dismissed their stale review May 3, 2025 01:27

Wait, maybe these kind of settings should be project specific.

@regulus79
Copy link
Contributor Author

Wait, maybe these kind of settings should be project specific.

Currently just the scale, key, and chord are project specific. Are you saying you also want the zoom/snap/note length/quantization settings to be saved in the project?

@Jernemies
Copy link

Wait, maybe these kind of settings should be project specific.

Imo user preferences should not be project specific for reasons I outlined above. In an ideal world we would just have defaults in settings, but saving these in config does achieve this in the meantime

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

Successfully merging this pull request may close these issues.

4 participants