Fix microtonal key sig playback #34030
Conversation
Ash-86
force-pushed
the
apply-microtonal-key-sig-
branch
from
8141ebc to
1bcf660
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change adds Changes
Sequence Diagram(s)sequenceDiagram
participant ComponentA
participant ComponentB
ComponentA->>ComponentB: observable interaction
Related issues: None found in the provided context. Related PRs: None found in the provided context. Suggested labels: engraving, playback Suggested reviewers: None specified in the provided context. Poem 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/engraving/dom/note.cpp`:
- Around line 2615-2618: Update Note::playingTuning() and the
accidental/custom-key handling in Note::effectiveCentOffset() so explicit cent
offsets stored via Pid::CENT_OFFSET/setCentOffset() are preserved even when
there is no current accidental. Also tighten the custom key-signature cents
lookup to require the active accidental state, not just the note degree, so
inherited chromatic notes without m_accidental do not get degree-only microtonal
offsets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4fe54173-ee3f-402a-b739-e9f9acaa92f3
📒 Files selected for processing (2)
src/engraving/dom/note.cppsrc/engraving/dom/note.h
| double Note::playingTuning() const | ||
| { | ||
| return m_tuning + m_centOffset; | ||
| return m_tuning + effectiveCentOffset(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve explicit cent offsets and avoid degree-only key matches.
Line 2617 now makes effectiveCentOffset() the playback source, but Lines 4138-4159 never fall back to m_centOffset; notes set through Pid::CENT_OFFSET/setCentOffset() without a current accidental will stop playing their stored offset. Also, Lines 4151-4155 apply custom-key cents by degree only, so a chromatically altered note that inherits a prior accidental and has no m_accidental can still receive the custom key-signature microtonal offset.
Proposed fix
double Note::effectiveCentOffset() const
{
if (m_accidental) { // explicit note-level accidental wins
- double offset = Accidental::subtype2centOffset(m_accidental->accidentalType());
- return offset;
+ return Accidental::subtype2centOffset(m_accidental->accidentalType());
+ }
+ if (!muse::RealIsNull(m_centOffset)) {
+ return m_centOffset;
}
if (!staff() || !chord()) {
return 0.0;
}
@@
}
int deg = tpc2step(tpc());
+ AccidentalVal accVal = tpc2alter(tpc());
for (const CustDef& cd : kse.customKeyDefs()) {
- if (kse.degInKey(cd.degree) == deg) {
- return Accidental::subtype2centOffset(
- Accidental::name2subtype(SymNames::nameForSymId(cd.sym)));
+ if (kse.degInKey(cd.degree) != deg) {
+ continue;
+ }
+ AccidentalType accType = Accidental::name2subtype(SymNames::nameForSymId(cd.sym));
+ if (Accidental::subtype2value(accType) == accVal) {
+ return Accidental::subtype2centOffset(accType);
}
}
return 0.0;
}Also applies to: 4138-4159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/engraving/dom/note.cpp` around lines 2615 - 2618, Update
Note::playingTuning() and the accidental/custom-key handling in
Note::effectiveCentOffset() so explicit cent offsets stored via
Pid::CENT_OFFSET/setCentOffset() are preserved even when there is no current
accidental. Also tighten the custom key-signature cents lookup to require the
active accidental state, not just the note degree, so inherited chromatic notes
without m_accidental do not get degree-only microtonal offsets.
2 participants
Resolves: #25604