-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add replaceInstrument() to Plugin API #31205
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
Conversation
4fb76ec to
30635e5
Compare
0e22080 to
0a9d2fe
Compare
245bdd8 to
125d62c
Compare
125d62c to
e8e560a
Compare
src/engraving/editing/editpart.h
Outdated
|
|
||
| /// Replaces the main instrument for a part. | ||
| /// Updates the part name, instrument, and clefs for all staves. | ||
| void replacePartInstrument(Score* score, Part* part, const Instrument& newInstrument, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These new funcions might be static like in EditSystemLocks , but not sure about the preferences in MS for these
d264c75 to
507928a
Compare
| initialInstrument.setTrackName(u"Flute"); | ||
| domPart->setInstrument(initialInstrument); | ||
|
|
||
| EXPECT_EQ(domPart->instrumentId(), QString("test.flute")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| EXPECT_EQ(domPart->instrumentId(), QString("test.flute")); | |
| EXPECT_EQ(domPart->instrumentId(), u"test.flute"); |
and similar in all those cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| String newInstrumentPartName = formatInstrumentTitle(newInstrument.trackName(), newInstrument.trait()); | ||
| mu::engraving::replacePartInstrument(score(), part, newInstrument, newStaffType, newInstrumentPartName); | ||
| } else { | ||
| mu::engraving::InstrumentChange* instrumentChange = findInstrumentChange(part, instrumentKey.tick); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NotationParts::findInstrumentChange can now be deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| // Change the part's instrument and name | ||
| String newPartName = partName.isEmpty() ? newInstrument.trackName() : partName; | ||
| score->undo(new ChangePart(part, new Instrument(newInstrument), newPartName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a new problem, but it looks like the new Instrument will be leaked, at least on undo. Solution might be to implement ChangePart::cleanup, which deletes the ChangePart::instrument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/engraving/editing/editpart.h
Outdated
|
|
||
| /// Replaces the main instrument for a part. | ||
| /// Updates the part name, instrument, and clefs for all staves. | ||
| void replacePartInstrument(Score* score, Part* part, const Instrument& newInstrument, const StaffType* newStaffType = nullptr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to put these functions as static methods in an EditPart class; somehow that feels more structured to me than a bunch of free functions. But it's not a big deal; if you think there are reasons to not do that, then we should not do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
I like the new structure! This is indeed the kind of things that suit well in the I would propose to tweak and merge this PR first with just |
Expose instrument replacement functionality to plugins through new Score.replaceInstrument() method. Allows plugins to change a part's instrument (name, clef, transposition, sound) with full undo/redo support. Changes: - Add Q_INVOKABLE Score::replaceInstrument(Part*, QString) to plugin API - Use Part::MAIN_INSTRUMENT_TICK (not Fraction(0,1)) to correctly identify main instrument vs. mid-score instrument changes Usage example: var part = curScore.parts[0]; curScore.replaceInstrument(part, "violin");
Remove dependency on notation module by using engraving::ChangePart command directly instead of going through INotationParts. This: - Eliminates need for NotationMock and NotationPartsMock in tests - Simplifies test setup (no mock injection required) - Keeps the implementation purely at the engraving layer - Makes the API self-contained within the plugin API module
…lugin API Move instrument replacement logic to editpart.cpp so that both NotationParts and the Plugin API use the same implementation. - Add replacePartInstrument() for main instrument replacement - Add replaceInstrumentAtTick() for instrument changes mid-score - Update NotationParts::replaceInstrument to use new shared functions - Update Plugin API Score::replaceInstrument to use new shared functions
- Use EditPart class with static methods instead of free functions - Add ChangePart::cleanup() to fix instrument pointer leak - Remove unused NotationParts::findInstrumentChange - Replace QString with u"..." literals in tests
1a55f99 to
f6d6fd3
Compare
|
@zacjansheski please regression-check the "Replace instrument" functionality in the UI |
|
Tested on MacOS Tahoe 26.2, Windows 11, Ubuntu 22.04.3. Approved |
Resolves: #31201
Add a new
Score.replaceInstrument(part, instrumentId)method to the Plugin API, allowing plugins to programmatically change instruments on score parts.Changes
Q_INVOKABLE Score::replaceInstrument(Part*, QString)to the plugin APINotationParts::replaceInstrument()infrastructurePart::MAIN_INSTRUMENT_TICKto correctly identify the main instrument vs mid-score instrument changesUsage Example
The instrumentId parameter should match IDs from
instruments.xml.