Refactor score unwinding #6248
Refactor score unwinding #6248
Conversation
| @@ -748,7 +748,7 @@ void MidiRenderer::collectMeasureEventsDefault(EventMap* events, Measure* m, con | |||
| // redirects to the correct function based on the passed method | |||
| //--------------------------------------------------------- | |||
|
|
|||
| void MidiRenderer::collectMeasureEvents(EventMap* events, Measure* m, const StaffContext& sctx, int tickOffset) | |||
| void MidiRenderer::collectMeasureEvents(EventMap* events, Measure const * m, const StaffContext& sctx, int tickOffset) | |||
There was a problem hiding this comment.
Correct me if I'm wrong, but don't we prefer const Measure* over Measure const *? Or maybe that's just an inconsequential thing, I don't know :)
There was a problem hiding this comment.
I believe the order wasn't specified in the "old" coding rules in effect on 3.x
The new ones indeed write const T* as a preference; but I'm still fighting that rule from a consistency pov, One should read type declarations from right to left and that would mean language natural that T const * makes more sense.
Anyhow.. let's not get into that discussion here.
If it must be changed to abide by coding rules, then I'll make that change; if I don't have to, then I won't ;-)
There was a problem hiding this comment.
Yeah, please don't feel you have to just because I've piped up ;)
There was a problem hiding this comment.
wow, I just realised that jeetee and jthistle were not the same person after all 😆
All that time I thought both were a short for johan temmerman, pointing to the same person...
There was a problem hiding this comment.
@Marr11317 Who says we're not the same person? 😇
I'm that talkative one that posts in the forums and occasionally fixes something.
James is the one that quietly fixes a bunch of stuff and occasionally speaks up.
There was a problem hiding this comment.
And I believe there's a certain difference in age. 😉
|
You really want to put these changes to 3.5RC, don't you? :) I suggest postponing to 3.5.1. What do you think? |
|
3.5.1 is fine too @anatoly-os And of course my code is so perfect that any PR I make warrants its own release just for the benefit of our users ;) So yeah, preferably before 4 will do |
|
3.5.1 is even better. |: | volta1 :||: volta2 | :| [EDIT:] fixed by commit below and added test::repeat56 to cover it. |
…#304795 Refactor repeatlist unwinding - Fix D.S. issues - Support for jumping into Volta - Support for multiple jumps - Prepare section break indications for when the pause should really matter - Prepare repeatlist to include playbackCount for each segment, opening the way for play x-th time for all elements
…ach test now includes: * a description of its main purpose * the expected playback order * measure numbers * tempo marker of 400QPM added test for jump targets under volta
… not be played on regular repeats and jumps
jeetee
force-pushed
the
refactor_score_unwinding
branch
from
4c64e21
to
246fc0d
Compare
|
Rebased and added test::repeat57 to cover https://musescore.org/en/node/274690#comment-1013746 |
| if (tick > lastMeasure()->endTick()) { | ||
| // End Reverb may last longer than written notation, but cursor position should not | ||
| tick = lastMeasure()->endTick(); | ||
| } |
|
That was not the easiest one, but see 87a8931 |
|
Easy is a relative term ;-) |
Resolves: issues with Jump/repeat combinations: #287447, #299320, #300362, #304795
#284887: crash on playback end in Debug build
Refactored unwinding to mimic "real" interpretation chain by a human over the previous copy-and-paste-rewind logic for jumps.
Also tracks section break pauses on RepeatSegments (but not used to fix related issue #32696 yet)