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

Generate standard drumset from online instruments spreadsheet #26082

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shoogle
Copy link
Contributor

@shoogle shoogle commented Jan 14, 2025

The spreadsheet is now the source of truth for all instrument data, which makes it more convienient for @zacjansheski to edit.

By keeping the standard drumset in the C++ code, we ensure it is available to engraving, playback, MIDI & MusicXML import systems during startup, prior to instruments.xml being loaded.

Replaces PR #25759, which caused problems for MIDI import, and potentially for other things too.

@Jojo-Schmitz
Copy link
Contributor

Why don't you add that spreadsheed to the repo?

@zacjansheski zacjansheski self-assigned this Jan 14, 2025
@shoogle
Copy link
Contributor Author

shoogle commented Jan 14, 2025

Why don't you add that spreadsheed to the repo?

Two big advantages of the spreadsheet are the formulas and formatting, which ensure consistency and provide instant feedback when you make a mistake. This means a simple CSV file wouldn't work; it would have to be in Excel or ODS (Open Document Spreadsheet) format.

If you download the existing Google Spreadsheet in ODS format and extract it like a Zip file, you'll find the main content.xml file inside is a 50 MB XML file full of formatting jargon and with zero indentation. Even if we had that under version control, you wouldn't see anything useful in the diff, nor could we rely on Git to properly resolve conflicts when merging independent PRs.

So even if it was in the repository, only one person would be able to work on it at a time. At least in Google Sheets you can see when another person is working on it, so you know you're not wasting your time. And then all you have to do is run a Python script to generate instruments.xml (and now this C++ file), which enables you to see a much more readable diff than you would get with an actual spreadsheet format, or even pure CSV.

The instrument data is effectively another open source project that is managed in a separate repository, like Qt or the Leland Music Font, except in this case the repository is a Google spreadsheet rather than a GitHub repo. Nevertheless, anybody can comment on the spreadsheet to suggest changes, or "fork" it to create their own copy to work on, or "clone" it to their machine by downloading it in Excel, ODS (Open Document Spreadsheet), or CSV format.

@shoogle shoogle force-pushed the generate-standard-drumset branch from 7b9dee5 to 8323a8e Compare January 14, 2025 17:56
@Jojo-Schmitz
Copy link
Contributor

Is a link to the spreadsheat in this Repo here?, Somehwere in a README.md next to instrument.xml?

@shoogle shoogle marked this pull request as ready for review January 14, 2025 18:49
The spreadsheet is the source of truth for all instrument data except:

* Some text (e.g. drum names) in src/engraving/types/typesconv.cpp.

* Playback data for Muse Sounds and MS Basic.
@shoogle shoogle force-pushed the generate-standard-drumset branch from 8323a8e to e02bd33 Compare January 15, 2025 07:12
@shoogle
Copy link
Contributor Author

shoogle commented Jan 15, 2025

Is a link to the spreadsheat in this Repo here?, Somehwere in a README.md next to instrument.xml?

Yes, as you would have found if you had looked for it. It's also in an XML comment at the top of instruments.xml.

I've updated the README to mention drumset.cpp, and added more details about how to use the spreadsheet and Python script.

@Jojo-Schmitz
Copy link
Contributor

Guilty as charged 😉

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.

3 participants