Time Signature simplifications #1513
Conversation
|
(if this were a 5.0 PR, I'd get rid of |
Document everything in timeSignature.ts Add general documentation for what StaveModifier.placeGlyphOnLine does. Reset width any time Glyph is changed Fix one bug in TimeSigGlyph where the `.line` of the TimeSignature was being ignored.
|
Done -- and in fact in fixing setting width on change of glyph, uncovered another bug in the current system: The two-system StaveModifier tests work by first adding all the modifiers, including addTimeSignature() and then work by calling setTimeSignature() which reuses the current TimeSignature object but changes the timeSpec. Since the width was (previously) set only in the constructor, changing the timeSpec did not change the width. This can most easily been seen when changing the final time signature from 9/8 (a thin TS) to C (a wider TS). Current master: PR: In Petaluma it's even more obvious (again master followed by PR):
That there should be a little bit of empty staff between the end TS and the end of the line is confirmed by Gould Behind Bars: |
|
Notice that the last mentioned bug still exists on TimeSigNote since changing the TimeSigNote.timeSig.setGlyph() will only change the width of TimeSigNote.timeSig not of TimeSigNote itself. Will discuss more in the context of mixins. |
To the extent possible without breaking backwards compatibility of public interfaces, this PR implements a little bit of my (second) comment on #1498 which @sschmidTU gave a thumbs up to: deemphasizing using structs in favor of objects themselves on TimeSignature and TimeSigNote.
Places line, glyph, num (now "is_numeric") attributes directly on the TimeSignature object (where the crucial .point) already is, rather than on a .info interface. (Note that the name of this struct differs among modifiers. On the Clef object it is ".clef", so changing one type of Modifier without changing all should not break code expecting a standard .info struct.)
TimeSigNote now stores the reference to the TimeSignature object rather than the .info struct, so that it can be modified up until the point it is formatted/drawn.
Feel free to comment if this is the right direction or not. I won't be offended (too much. :-D ) if it's not where the project should go. But class-objects are quite lightweight and as long as we're creating the TimeSignature object anyhow, might as well hang onto it until we're ready.
All visual tests pass.