-
Notifications
You must be signed in to change notification settings - Fork 3k
Show localized Piano Keyboard key names #30389
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
Show localized Piano Keyboard key names #30389
Conversation
01385a1
to
e11fb6a
Compare
Updated this PR as well based on the comment from #30415 |
That's a very cheap fix, I like it ;-) |
For SPN notation, I removed the space in #30530, so it looks better. |
I removed spaces from all my PRs. It is better this way. Logic is easier. |
bd10618
to
a0a3ad8
Compare
int octaveNumber = (key / 12) - 1; | ||
|
||
QString octaveLabel = "C" + QString::number(octaveNumber); | ||
octaveLabel = muse::qtrc("global/pitchName", octaveLabel.toStdString().c_str()); |
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.
I believe this is unsafe, because the std::string
is destructed immediately after c_str()
has been called, meaning that the pointer passed to qtrc
is dangling. This would be solved by making octaveLabel
immediately a std::string instead of QString, because then the lifetime of the std::string and its data is until the end of the scope, which is enough here.
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.
Or make it static
;-)
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.
I don't think that would solve anything, would it? (and octaveNumber
isn't static either)
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.
Probably, but how is this here different from
preset.name = muse::trc("instruments/stringTunings", presetObj.value("name").toStdString().c_str()); |
Or is that simply having the same issue?
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.
I think that has the same issue indeed (but in practice, it probably only leads to a crash very seldom, because even though the std::string's memory is deleted
, it is not immediately removed or filled with garbage).
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.
MuseScore/src/notation/internal/instrumentsrepository.cpp
I saw that as well, but I think @cbjeukendrup is right, seems to be a dangling pointer issue. Will fix accordingly. Thanks!
The C piano keys in the Piano Keyboard panel are always shown in standard pitch notation. Display the notes with respect to locale by using the strings from `EditPitchBase`.
to global/pitchName
a0a3ad8
to
b79a9fe
Compare
Partially resolves: #30167 (Part 2)
The C piano keys in the Piano Keyboard panel are always shown in standard pitch notation.
Display the notes with respect to locale by using the strings from
global/pitchName
.Piano keyboard with Romanian locale:
Piano keyboard with German locale: