From 1759c5c014adf3a01b4002244a4260072649631a Mon Sep 17 00:00:00 2001 From: Nils <> Date: Tue, 30 Apr 2019 23:06:08 +0200 Subject: [PATCH] fix crash --- engine/api.py | 9 +++++---- engine/items.py | 37 +++++++++++++------------------------ engine/main.py | 35 ++++++++++++++++++++++++++--------- engine/track.py | 2 +- qtgui/items.py | 11 +++++++---- qtgui/submenus.py | 9 +++------ 6 files changed, 55 insertions(+), 48 deletions(-) diff --git a/engine/api.py b/engine/api.py index d78585f..debef75 100644 --- a/engine/api.py +++ b/engine/api.py @@ -124,7 +124,7 @@ class ClientCallbacks(Callbacks): #inherits from the templates api callbacks can be used by a GUI to draw notes. track.staticRepresentation also creates the internal midi representation for cbox. """ if self.updateTrack: - ex = session.data.trackById(trId).staticRepresentation() #Includes cbox.updatePlayback + ex = session.data.trackById(trId).staticRepresentation() for func in self.updateTrack: func(trId, ex) self._updateBlockTrack(trId) @@ -447,6 +447,7 @@ def _deleteSelection(backspaceParamForCompatibilityIgnoreThis = None): #this is moveFunction = _createLambdaMoveToForCurrentPosition() listOfChangedTrackIDs = session.data.deleteSelection() + if listOfChangedTrackIDs: #delete succeeded. def registeredUndoFunction(): moveFunction() @@ -1699,7 +1700,7 @@ def insertCommonMetricalInstrucions(scheme): "1/1" : (D1,), } lilypond = { - "off" : "\\mark \"No meter\" \\cadenzaOn ", + "off" : "\\mark \"X\" \\cadenzaOn ", "2/4" : "\\cadenzaOff \\time 2/4", "3/4" : "\\cadenzaOff \\time 3/4", "4/4" : "\\cadenzaOff \\time 4/4", @@ -2303,8 +2304,8 @@ def insertRandomFromClipboard(): #Midi only -def instrumentChange(program, msb, lsb, instrumentName, shortInstrumentName): - change = items.InstrumentChange(program, msb, lsb, instrumentName, shortInstrumentName) +def instrumentChange(program, msb, lsb, shortInstrumentName): + change = items.InstrumentChange(program, msb, lsb, shortInstrumentName) insertItem(change) def channelChange(value, text): diff --git a/engine/items.py b/engine/items.py index 0fa0350..1dc411d 100644 --- a/engine/items.py +++ b/engine/items.py @@ -2614,8 +2614,9 @@ class LegatoSlur(Item): } class InstrumentChange(Item): - """Includes program change, both bank changes and the lilypond instrument name and short name change""" - def __init__(self, program, msb, lsb, instrumentName, shortInstrumentName): + """Includes program change, both bank changes and the lilypond short name change. + Lilypond does not support a full name change, so we don't offer that here.""" + def __init__(self, program, msb, lsb, shortInstrumentName): """Either init gets called, by creation during runtime, or instanceFromSerializedData. That means everything in init must be matched by a loading call in instanceFromSerializedData.""" super().__init__() @@ -2624,8 +2625,7 @@ class InstrumentChange(Item): assert 0 <= lsb <= 127, lsb #a CC value self.program = program self.msb = msb - self.lsb = lsb - self.instrumentName = instrumentName + self.lsb = lsb self.shortInstrumentName = shortInstrumentName self._secondInit(parentBlock = None) #see Item._secondInit. @@ -2642,8 +2642,7 @@ class InstrumentChange(Item): self = cls.__new__(cls) self.program = serializedObject["program"] self.msb = serializedObject["msb"] - self.lsb = serializedObject["lsb"] - self.instrumentName = serializedObject["instrumentName"] + self.lsb = serializedObject["lsb"] self.shortInstrumentName = serializedObject["shortInstrumentName"] self.deserializeDurationAndNotelistInPlace(serializedObject) self._secondInit(parentBlock = parentObject) @@ -2653,14 +2652,13 @@ class InstrumentChange(Item): result = super().serialize() #call this in child classes result["program"] = self.program result["msb"] = self.msb - result["lsb"] = self.lsb - result["instrumentName"] = self.instrumentName + result["lsb"] = self.lsb result["shortInstrumentName"] = self.shortInstrumentName return result def _copy(self): """return an independent copy of self""" - new = type(self)(self.program, self.msb, self.lsb, self.instrumentName, self.shortInstrumentName) #all init parameters are immutable types and copied implicitly + new = type(self)(self.program, self.msb, self.lsb, self.shortInstrumentName) #all init parameters are immutable types and copied implicitly return new def _exportObject(self, trackState): @@ -2671,8 +2669,8 @@ class InstrumentChange(Item): _msbStr = " msb" + str(self.msb) if self.msb > 0 else "" _lsbStr = " lsb" + str(self.lsb) if self.lsb > 0 else "" - _nameStr = self.shortInstrumentName if self.shortInstrumentName else self.instrumentName - _nameStr = _nameStr + " " if (self.shortInstrumentName or self.instrumentName) else "" + _nameStr = self.shortInstrumentName + _nameStr = _nameStr + " " if self.shortInstrumentName else "" return { "type" : "InstrumentChange", "completeDuration" : duration, @@ -2680,26 +2678,17 @@ class InstrumentChange(Item): "midiBytes" : [cbox.Pattern.serialize_event(tickPosition, 0xC0 + trackState.midiChannel(), self.program, 0), cbox.Pattern.serialize_event(tickPosition, 0xB0 + trackState.midiChannel(), 0, self.msb), #position, status byte+channel, controller number, controller value cbox.Pattern.serialize_event(tickPosition, 0xB0 + trackState.midiChannel(), 32, self.lsb), #position, status byte+channel, controller number, controller value - ], - "instrumentName" : self.instrumentName, + ], "shortInstrumentName" : self.shortInstrumentName, "UIstring" : "{}[pr{}{}{}]".format(_nameStr, self.program, _msbStr, _lsbStr), #this is for a UI, possibly a text UI, maybe for simple items of a GUI. Make it as short and unambigious as possible. - } - - #hier weiter machen. die midiBytes können so exportiert werden, wahrscheinlich auch auf diesem channel. - #Aber die müssen im Track dann am Ende rausgefiltert werden und auf einen eigenen cbox midi track. - #Die Alternative wäre das aufzuteilen. Eins auf CC0, eins auf CC32 und der Program Change auf einen eigenen Kanal? - #Was ist mit sowas wie initial program? Wo wird das geändert? In den Track Properties. Vielleichst ist das selbst ein InstrumentChange Objekt. Könnte eins sein zumindest. + } - - def _lilypond(self): """called by block.lilypond(), returns a string. Don't create white-spaces yourself, this is done by the structures. When in doubt prefer functionality and robustness over 'beautiful' lilypond syntax.""" - if self.instrumentName or self.shortInstrumentName: - raise NotImplementedError("Implement _lilypond() with correct instrument change syntax") - return str + if self.shortInstrumentName: + return f'\\mark \\markup {{\\small "{self.shortInstrumentName}"}}' + "\n" + f'\\set Staff.shortInstrumentName = #"{self.shortInstrumentName}"' else: return "" diff --git a/engine/main.py b/engine/main.py index 92b1321..8267ad6 100644 --- a/engine/main.py +++ b/engine/main.py @@ -77,8 +77,9 @@ class Data(template.engine.sequencer.Score): try: ret = Track.allTracks[trackId] except: + print ("Error while trying to get track by id") print (sys.exc_info()[0]) - print (trackId, Track.allTracks, self.tracks, self.hiddenTracks, self.trackIndex) + print (trackId, hex(trackId), Track.allTracks, self.tracks, self.hiddenTracks, self.trackIndex) raise return ret @@ -436,7 +437,8 @@ class Data(template.engine.sequencer.Score): tickindex and position. We take care of duration-changed content linked items left of the selection as well. """ - t = self.selectionExport() #is there a selection at all? + + t = self.selectionExport() #is there a selection at all? if not t: return [False, None, None, [], []] #mimickes the true result. the api can use this to decide to do nothing instead of processing the selection else: @@ -450,9 +452,10 @@ class Data(template.engine.sequencer.Score): #listOfChangedTrackIds = set([id(track) for track in tracksWithSelectedItems]) #this is not the same as tracksWithSelectedItems. It gets more ids than the initial ones here. A content-linked note outside a selected track will still be affected. We start with the trackIds we already know. listOfChangedTrackIds = set() finalResult = [selectionValid, topLeft, bottomRight, listOfChangedTrackIds] #listOfChangedTrackIds is mutable. It will change in the loop below - + firstRound = True for track in tracksWithSelectedItems: + assert track in self.tracks, track #selecting from hidden or deleted tracks is impossible originalPosition = track.state.position() result = [] @@ -467,12 +470,15 @@ class Data(template.engine.sequencer.Score): if r == 1: # the item we want is already left of the cursor. So we want the previous item. if track.state.tickindex <= endTick: #yes, it is correct that the state in the parameter is ahead by one position. Why is it that way? Because everything that matters, like new dynamics will only be parsed afterwards. The trackState is always correct except for the tickindex when exporting after parsing. Thats why exportObject sometimes substracts its own duration for finding its starting tick. - item = track.previousItem() + item = track.previousItem() if (not removeContentLinkedData) or (not item in seenItems): result.append((item, {"keySignature" : track.state.keySignature()})) #The keysig is needed for undo operations after apply to selection seenItems.add(item) for parentBlock in item.parentBlocks: #TODO: just be naive and add them all for now. Performance can be improved later. - listOfChangedTrackIds.add(id(parentBlock.parentTrack)) + #assert parentBlock, item + #assert parentBlock.parentTrack, (parentBlock, parentBlock.parentTrack) + if parentBlock.parentTrack: + listOfChangedTrackIds.add(id(parentBlock.parentTrack)) #there are items without parent Tracks. They have None. We must filter them out otherwise our changedTrackIds get polluted with id(None) else: continue elif r == 2: #block end. Again, this is already the previous state. right now we are in the next block already. @@ -482,7 +488,10 @@ class Data(template.engine.sequencer.Score): track.toPosition(originalPosition) #has head() in it finalResult.append(result) - #finalResult[4:] is only lists(tracks) with items + + for changedTrId in listOfChangedTrackIds: + assert self.trackById(changedTrId) in self.tracks, changedTrId #selecting from hidden or deleted tracks is impossible + return finalResult #The next two functions have "Objects" in their name so copy gets not confused with block.copy or item.copy. This is Ctrl+C as in copy and paste. @@ -628,7 +637,10 @@ class Data(template.engine.sequencer.Score): def deleteSelection(self): """Mortals, Hear The Prophecy: Delete Selection is notoriously tricky. It was hard in Laborejo 1 and it is still hard here. Please use as many asserts and tests as you possibly can. The next bug or regression - WILL come. And it will be here.""" + WILL come. And it will be here. + + Will only delete from visible tracks. No hidden, no deleted. + """ validSelection, topLeftCursor, bottomRightCursor, listOfChangedTrackIds, *selectedTracksAndItems = self.listOfSelectedItems(removeContentLinkedData = True) #selectedTracksAndItems format: [track1[(item, itsKeysig), (item, itsKeysig), (item, itsKeysig)], track2[(item, itsKeysig), (item, itsKeysig), (item, itsKeysig)]] @@ -674,7 +686,7 @@ class Data(template.engine.sequencer.Score): assert curTrack.state.tickindex == topLeftCursor["tickindex"], (curTrack.state.tickindex, topLeftCursor["tickindex"]) #We delete always from the same cursor position. After one deletion the rest items gravitate left to our position. Except block boundaries, those will not be crossed so we have to check for them: - for item, cached6State in selectionTrack: + for item, cachedState in selectionTrack: if item is bottomRightCursor["item"]: break #without this the program will delete the first non-selected item, if zero duration, as well @@ -696,6 +708,7 @@ class Data(template.engine.sequencer.Score): #assert curTrack.state.tickindex == topLeftCursor["tickindex"] We cannot be sure of this. The selection could have started at a content-linked item that got deleted and the tickindex has no item anymore #return [id(track) for track in self.tracks] #TODO: only for debug reasons. the GUI will draw everything with this! Either this function or the api has to figure out the tracks that really changed. we could do a len check of the blocks data. + return listOfChangedTrackIds def getBlockAndItemOrder(self): @@ -955,6 +968,11 @@ class Data(template.engine.sequencer.Score): track = self.currentTrack() startBlock = track.currentBlock() #this is the only unique thing we can rely on. + #startBlock will get deleted, but we don't remove the blocks as parent because we need them for undo + #They will be finally cleared on save. + #NO. This works for splitting, but not for undo. for item in startBlock.data: + # item.parentBlocks.remove(startBlock) + dictOfTrackIdsWithListOfBlockIds = {} # [trackId] = [listOfBlockIds] protoBlockLeft = startBlock.copy(track) @@ -963,7 +981,6 @@ class Data(template.engine.sequencer.Score): protoBlockRight = startBlock.copy(track) protoBlockRight.data = startBlock.data[startBlock.localCursorIndex:] #everything right of the cursor. The local cursor index of the current block is guaranteed to be correct. The one from other blocks is not. - for block in startBlock.linkedContentBlocksInScore(): #all linked blocks in all tracks, including currentBlock. All blocks are unique. The content is not. parentTrackId = id(block.parentTrack) if parentTrackId in dictOfTrackIdsWithListOfBlockIds: diff --git a/engine/track.py b/engine/track.py index 49da1db..2f2c9de 100644 --- a/engine/track.py +++ b/engine/track.py @@ -297,7 +297,7 @@ class TrackState(object): self.duringLegatoSlur = False #there are no nested legato slurs self.duringBeamGroup = False #no nested beam groups. self.midiChannels = [track.initialMidiChannel] #these are just integers, not items. items.ChannelChange parsing changes this and items automatically get the new value. A stack of midi channels allows the cursor to know the current channel for immediate playback/feedback. - self.instrumentChanges = [InstrumentChange(track.initialMidiProgram, track.initialMidiBankMsb, track.initialMidiBankLsb, track.initialInstrumentName, track.initialShortInstrumentName, )] + self.instrumentChanges = [InstrumentChange(track.initialMidiProgram, track.initialMidiBankMsb, track.initialMidiBankLsb, track.initialShortInstrumentName, )] self.midiTranspose = track.midiTranspose # TODO: When a change item for the following functions is introduced they need to get parsed in track.parseLeft and track.parseRight diff --git a/qtgui/items.py b/qtgui/items.py index 25dfed0..8ba3f6e 100644 --- a/qtgui/items.py +++ b/qtgui/items.py @@ -738,7 +738,7 @@ class GuiMetricalInstruction(GuiItem): if staticItem["oneMeasureInTicks"] == 0: #Basically just a manual barline - displayString = "Metrical Off" + displayString = "X" markerPosition = -2 markerHeight = 4 @@ -754,12 +754,15 @@ class GuiMetricalInstruction(GuiItem): r = r.replace(",", "") displayString = "Metrical " + r - markerHeight = -7 - markerPosition = 1 + markerPosition = -2 + markerHeight = 4 + + #A metrical instruction is by definition at the beginning of a measure + #We therefore need to place the text above the barnumber self.text = QtWidgets.QGraphicsSimpleTextItem(displayString) self.text.setParentItem(self) - self.text.setPos(0, -7 * constantsAndConfigs.stafflineGap) + self.text.setPos(0, -7 * constantsAndConfigs.stafflineGap) self.marker = GuiPositionMarker(markerHeight, markerPosition) self.marker.setParentItem(self) diff --git a/qtgui/submenus.py b/qtgui/submenus.py index 5e7c3a7..a541595 100644 --- a/qtgui/submenus.py +++ b/qtgui/submenus.py @@ -566,8 +566,7 @@ class SecondaryProgramChangeMenu(Submenu): self.msb = QtWidgets.QSpinBox() self.msb.setValue(type(self).lastMsbValue) self.lsb = QtWidgets.QSpinBox() - self.lsb.setValue(type(self).lastLsbValue) - self.instrumentName = QtWidgets.QLineEdit() + self.lsb.setValue(type(self).lastLsbValue) self.shortInstrumentName = QtWidgets.QLineEdit() for label, spinbox in (("Program", self.program), ("Bank MSB", self.msb), ("Bank LSB", self.lsb)): @@ -575,9 +574,7 @@ class SecondaryProgramChangeMenu(Submenu): spinbox.setMaximum(127) spinbox.setSingleStep(1) self.layout.addRow(label, spinbox) - - - self.layout.addRow("Instr.Name", self.instrumentName) + self.layout.addRow("Short Name", self.shortInstrumentName) self.insert = QtWidgets.QPushButton("Insert") @@ -593,7 +590,7 @@ class SecondaryProgramChangeMenu(Submenu): lsb = self.lsb.value() type(self).lastLsbValue = lsb - api.instrumentChange(program, msb, lsb, self.instrumentName.text(), self.shortInstrumentName.text(), ) + api.instrumentChange(program, msb, lsb, self.shortInstrumentName.text(), ) self.done(True)