Fix up MIDI export and import (within reason)

This clarifies the logic around tempo and time
signature in both exports and imports. The
tempo/bpm conversion seems to be correct
and reliable. The time signature is not so
lucky.

First of all, Bosca has a fixed time signature,
the default 4/4. There is no way to change
that, so we pretty much hardcode it (although
now it's a bit clearer what we hardcode, MIDI
spec is weird). On import the old code used
to attempt to rebuild the pattern size using
the time signature. But that's not exactly a
robust approach, since, as I mentioned, even
in Bosca it's always 4/4, whereas the pattern
size can vary.

The best we can do is to just use some fixed
unit of size and hope it's going to generate
nice patterns on import. And for this unit we
can use a part of the signature that makes
sense for this role, the numerator. It could be
nice to add an extra step to the import and let
users decide how large the pattern should be.

That said, there is no working around other
MIDI-specific quirks, like each track having its
own time signature — a feature that would
be impossible to preserve in Bosca.

Aside of all that, this also fixes a couple of
issues related to the order of events (both on
import and on export), and a problem with
invalid notes slipping into MIDI files anyway.

Author: YuriSizov

gui/views/note_map/NoteMap.gd

@@ -529,8 +529,8 @@ func _update_active_notes() -> void:
 
 	for i in current_pattern.note_amount:
 		var note_data := current_pattern.notes[i]
-		if note_data.y < 0 || note_data.y >= pattern_size || note_data.z < 1:
-			continue # Outside of the pattern row, or too short to play.
+		if not current_pattern.is_note_valid(note_data, pattern_size):
+			continue # Outside of the pattern bounds, or too short to play.
 
 		var note_value_normalized := note_data.x - current_pattern.key # Shift to its C-key equivalent.
 		if note_value_normalized < 0 || note_value_normalized >= _note_value_row_map.size():

gui/views/pattern_map/PatternDock.gd

@@ -86,20 +86,20 @@ func _draw_item(on_control: Control, item_index: int, item_rect: Rect2) -> void:
 		Vector2(_item_gutter_width - _note_border_width, 2.0 * item_rect.size.y / 5.0)
 	)
 	on_control.draw_rect(label_underline, item_color)
-
+	
 	# Draw gutter string.
 
 	var string_position := item_rect.position + _item_label_offset + Vector2(0, item_rect.size.y)
 	var shadow_position := string_position + _shadow_size
 	var gutter_string := "%d" % [ item_index + 1 ]
 	on_control.draw_string(_font, shadow_position, gutter_string, HORIZONTAL_ALIGNMENT_LEFT, -1, _font_size, _shadow_color)
 	on_control.draw_string(_font, string_position, gutter_string, HORIZONTAL_ALIGNMENT_LEFT, -1, _font_size, _font_color)
-
+	
 	# Draw pattern note map.
 
 	if pattern.note_amount > 0:
 		var pattern_size := Controller.current_song.pattern_size
-
+		
 		var note_span := pattern.get_active_note_span_size()
 		var note_width := note_area.size.x / pattern_size
 		var note_height := note_area.size.y / note_span
@@ -117,9 +117,9 @@ func _draw_item(on_control: Control, item_index: int, item_rect: Rect2) -> void:
 
 		for i in pattern.note_amount:
 			var note := pattern.notes[i]
-			if note.x < 0 || note.y < 0 || note.y >= pattern_size || note.z < 1:
+			if not pattern.is_note_valid(note, pattern_size):
 				continue
-
+			
 			var note_index := note.x - note_value_offset
 			var note_position := note_origin + Vector2(note_width * note.y, note_span_height - note_height * (note_index + 1))
 			var note_size := Vector2(note_width * note.z, note_height)

gui/views/pattern_map/PatternMap.gd

@@ -489,9 +489,9 @@ func _update_active_patterns() -> void:
 				var note_value_offset := pattern.active_note_span[0]
 				for j in pattern.note_amount:
 					var note := pattern.notes[j]
-					if note.x < 0 || note.y < 0 || note.y >= pattern_size || note.z < 1:
+					if not pattern.is_note_valid(note, pattern_size):
 						continue
-
+					
 					var note_index := note.x - note_value_offset
 					var note_position := note_origin + Vector2(note_width * note.y, note_span_height - note_height * (note_index + 1))
 					var note_size := Vector2(note_width * note.z, note_height)

io/MidiExporter.gd

@@ -45,8 +45,8 @@ static func _write(writer: MidiFileWriter, song: Song) -> void:
 	# Track chunk 1. Meta events.
 
 	var meta_track := writer.create_track()
-	meta_track.add_time_signature(4, 2, 24, 8)
-	meta_track.add_tempo(song.bpm)
+	meta_track.add_time_signature(4, 4) # Bosca always operates with the 4/4 signature.
+	meta_track.add_tempo(song.bpm, 4)
 
 	# Track chunk 2. Non-drumkit instruments and notes.
 	# Channel 9 is special and is used for drums, so we avoid it by splitting the list in two.
@@ -95,6 +95,8 @@ static func _write_instruments_to_track(track: MidiTrack, song: Song, limit: int
 
 			for k in pattern.note_amount:
 				var note := pattern.notes[k]
+				if not pattern.is_note_valid(note, song.pattern_size):
+					continue
 
 				track.add_note(pattern.instrument_idx - offset, note_offset + note.y, note.x, note.z, instrument.volume)
 
@@ -119,8 +121,10 @@ static func _write_drumkits_to_track(track: MidiTrack, song: Song) -> void:
 
 				for k in pattern.note_amount:
 					var note := pattern.notes[k]
-					var note_value := drumkit_instrument.get_midi_note(note.x)
+					if not pattern.is_note_valid(note, song.pattern_size):
+						continue
 
+					var note_value := drumkit_instrument.get_midi_note(note.x)
 					track.add_note(MidiFile.DRUMKIT_CHANNEL, note_offset + note.y, note_value, note.z, instrument.volume)
 
 

io/MidiImporter.gd

@@ -117,6 +117,12 @@ class MidiFileReader:
 		var signature_found := false
 		var tempo_found := false
 
+		# Default values per MIDI spec.
+		var signature_numerator := 4
+		var signature_denominator := 4
+		var tempo := 500_000
+		
+		# Look for the settings in any track. Usually it's the first one, but we can't be sure.
 		for track in _tracks:
 			var events := track.get_events()
 
@@ -126,26 +132,56 @@ class MidiFileReader:
 				var meta_payload := event.payload as MidiTrackEvent.MetaPayload
 
 				if not signature_found && meta_payload.meta_type == MidiTrackEvent.MetaType.TIME_SIGNATURE:
-					var numerator := meta_payload.data[0]
-					var denominator := 1 << meta_payload.data[1]
+					signature_numerator = meta_payload.data[0]
+					signature_denominator = 1 << meta_payload.data[1]
 
-					# This is a bit dubious, but matches the original implementation.
-					song.pattern_size = clampi(numerator * denominator, 0, 32)
+					# Bosca only supports one time signature per song, which isn't necessarily
+					# true for MIDI files. Can't do much about it, so using the first one and praying.
+					print_verbose("MidiImporter: Found a time signature message (%d, %d)." % [ signature_numerator, signature_denominator ])
 					signature_found = true
 
 				if not tempo_found && meta_payload.meta_type == MidiTrackEvent.MetaType.TEMPO_SETTING:
 					# Tempo is stored in 24 bits.
-					var tempo := 0
+					tempo = 0
 					tempo += meta_payload.data[0] << 16
 					tempo += meta_payload.data[1] << 8
 					tempo += meta_payload.data[2]
 
-					@warning_ignore("integer_division")
-					song.bpm = 60_000_000 / tempo
+					print_verbose("MidiImporter: Found a tempo message (%d)." % [ tempo ])
 					tempo_found = true
 
 				if signature_found && tempo_found:
-					return
+					break
+			if signature_found && tempo_found:
+				break
+		
+		# Compute pattern size and BPM based on our findings.
+		
+		# The original implementation tries to use the numerator and denominator
+		# from the time signature to guess a pattern size. This is obviously not
+		# a reliable method, and we cannot even recreate an original Bosca song
+		# exported as MIDI this way.
+		#
+		# This is because the time signature defines the nature of a beat (how
+		# many quarter* notes it has). For example, in Bosca Ceoil the time
+		# signature is ALWAYS 4/4, regardless of the pattern size or other settings.
+		#
+		# To determine the pattern size properly we'd have to inspect the notes.
+		# And, in the end, we have no guarantee that notes would neatly pack into
+		# patterns anyway. It'd be a complex task to try and build patterns out of
+		# random MIDI notes.
+		#
+		# So the best we can do is to use an arbitrary pattern size (which was,
+		# perhaps, what the original implementation did, in a way). We can use
+		# the numerator of the time signature alone, to try and turn a MIDI song
+		# into a bunch of beat-long patterns.
+		
+		# TODO: Perhaps expose a factor to this as an import setting somehow?
+		var beats_per_pattern := 1
+		song.pattern_size = clampi(signature_numerator * beats_per_pattern, 1, Song.MAX_PATTERN_SIZE)
+		
+		@warning_ignore("integer_division")
+		song.bpm = roundi(MidiFile.TEMPO_BASE / tempo * signature_denominator / 4.0)
 
 
 	func extract_composition(song: Song) -> void:
@@ -230,14 +266,32 @@ class MidiFileReader:
 		var midi_track := _tracks[track_idx]
 		var note_pitch := midi_payload.data[0]
 
+		# We first try to find a note that has been around for a bit and turn it off.
+		# So we're skipping all notes which aren't valid. But in case we don't find
+		# any, we should turn off the first "not-so-valid" note at least. Hence,
+		# candidates.
+		var candidates: Array[MidiNote] = []
+		var check_candidates := true
+		
 		var i := instrument_notes.size() - 1
 		while i >= 0:
 			var midi_note := instrument_notes[i]
 			if midi_note.pitch == note_pitch && midi_note.length < 0:
+				candidates.push_back(midi_note)
+				
 				@warning_ignore("integer_division")
-				midi_note.length = (timestamp - midi_note.timestamp) / midi_track.note_time
+				var new_length := (timestamp - midi_note.timestamp) / midi_track.note_time
+				if new_length > 0:
+					midi_note.length = new_length
+					check_candidates = false
+					break
 
 			i -= 1
+		
+		if check_candidates:
+			var midi_note: MidiNote = candidates.pop_front()
+			@warning_ignore("integer_division")
+			midi_note.length = (timestamp - midi_note.timestamp) / midi_track.note_time
 
 
 	func create_patterns(song: Song) -> void:

io/SongMerger.gd

@@ -36,10 +36,7 @@ static func encode_pattern(pattern: Pattern, instrument: Instrument, pattern_siz
 	# Prepare note data.
 	for i in pattern.note_amount:
 		var note_data := pattern.notes[i]
-		if note_data.x < 0 || note_data.y < 0 || note_data.y >= pattern_size || note_data.z < 1:
-			# X — note number is invalid.
-			# Y — note position in the pattern is invalid.
-			# Z — note length is shorter than 1 unit of length.
+		if not pattern.is_note_valid(note_data, pattern_size):
 			continue
 
 		# Encode the note itself.

io/midi/MidiFile.gd

@@ -16,5 +16,7 @@ enum FileFormat {
 const DEFAULT_RESOLUTION := 120
 const DRUMKIT_CHANNEL := 9
 
+const TEMPO_BASE := 60_000_000
+
 const FILE_HEADER_MARKER := "MThd"
 const FILE_TRACK_MARKER := "MTrk"

io/midi/MidiTrack.gd

@@ -65,6 +65,9 @@ func get_events() -> Array[MidiTrackEvent]:
 
 
 func _sort_events(a: MidiTrackEvent, b: MidiTrackEvent) -> bool:
+	if a.timestamp == b.timestamp:
+		return a.order < b.order # Preserve insertion order when timestamp is the same.
+	
 	return a.timestamp < b.timestamp
 
 
@@ -74,6 +77,7 @@ func add_meta_event(meta_type: MidiTrackEvent.MetaType, timestamp: int, data: Pa
 	var event := MidiTrackEvent.new()
 	event.type = MidiTrackEvent.Type.META_EVENT
 	event.timestamp = timestamp
+	event.order = _events.size()
 
 	var payload := MidiTrackEvent.MetaPayload.new()
 	payload.meta_type = meta_type
@@ -84,22 +88,30 @@ func add_meta_event(meta_type: MidiTrackEvent.MetaType, timestamp: int, data: Pa
 	_events_unsorted = true
 
 
-func add_time_signature(numerator: int, denominator: int, clocks_per_click: int, notated_notes: int) -> void:
+func add_time_signature(numerator: int, denominator: int, clocks_per_click: int = 24, quarter_note_resolution: int = 8) -> void:
 	var event_data := PackedByteArray()
+	
+	# Denominator is stored as a power-of-2 value (e.g. 4 -> 2, 8 -> 3, etc).
+	var po2_denominator := 0
+	var n := denominator
+	while n > 1:
+		n >>= 1
+		po2_denominator += 1
+	
 	event_data.append(numerator)
-	event_data.append(denominator)
+	event_data.append(po2_denominator)
 	event_data.append(clocks_per_click)
-	event_data.append(notated_notes)
+	event_data.append(quarter_note_resolution)
 
 	add_meta_event(MidiTrackEvent.MetaType.TIME_SIGNATURE, 0, event_data)
 
 
-func add_tempo(bpm: int) -> void:
+func add_tempo(bpm: int, denominator: int) -> void:
 	var event_data := PackedByteArray()
 
 	# Tempo is stored in microseconds per MIDI quarter-note.
 	@warning_ignore("integer_division")
-	var misec_per_beat := 60_000_000 / bpm
+	var misec_per_beat := int(MidiFile.TEMPO_BASE / bpm * denominator / 4.0)
 
 	# Value is always stored in 24 bits.
 	event_data.append((misec_per_beat >> 16) & 0xFF)
@@ -115,6 +127,7 @@ func add_midi_event(midi_type: MidiTrackEvent.MidiType, channel_num: int, timest
 	var event := MidiTrackEvent.new()
 	event.type = MidiTrackEvent.Type.MIDI_EVENT
 	event.timestamp = timestamp
+	event.order = _events.size()
 
 	var payload := MidiTrackEvent.MidiPayload.new()
 	payload.midi_type = midi_type

io/midi/MidiTrackEvent.gd

@@ -50,6 +50,7 @@ enum MidiType {
 
 var type: Type = Type.META_EVENT
 var timestamp: int = 0
+var order: int = 0
 var payload: Payload = null
 
 

objects/Pattern.gd

@@ -389,3 +389,9 @@ func remove_note_at(index: int) -> void:
 			notes[i] = Vector3i(-1, 0, 0)
 
 	note_amount -= 1
+
+
+func is_note_valid(note: Vector3i, pattern_size: int) -> bool:
+	if note.x < 0 || note.y < 0 || note.y >= pattern_size || note.z < 1:
+		return false
+	return true