From d46605c63852f480ef0e8989f932c16c756ad5ba Mon Sep 17 00:00:00 2001
From: "5684185+vsariola@users.noreply.github.com"
 <5684185+vsariola@users.noreply.github.com>
Date: Fri, 6 Sep 2024 20:14:46 +0300
Subject: [PATCH] fix: assign new IDs to loaded instruments

Fixes #146.
---
 CHANGELOG.md     |  3 +++
 tracker/files.go |  1 +
 tracker/model.go | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 772b68ba..beedf015 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
 ### Fixed
+- Assign new IDs to loaded instruments, and fix ID collisions in case they
+  somehow still appear ([#146][i146])
 - In x86 templates, do not optimize away phase modulations when unisons are used
   even if all phase inputs are zeros, as unisons use the phase modulation
   mechanism to offset the different oscillators
@@ -176,3 +178,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 [i129]: https://github.com/vsariola/sointu/issues/129
 [i130]: https://github.com/vsariola/sointu/issues/130
 [i139]: https://github.com/vsariola/sointu/issues/139
+[i146]: https://github.com/vsariola/sointu/issues/146
diff --git a/tracker/files.go b/tracker/files.go
index 6fcda6c5..01af83c1 100644
--- a/tracker/files.go
+++ b/tracker/files.go
@@ -171,6 +171,7 @@ success:
 	for len(m.d.Song.Patch) <= m.d.InstrIndex {
 		m.d.Song.Patch = append(m.d.Song.Patch, defaultInstrument.Copy())
 	}
+	m.assignUnitIDs(instrument.Units)
 	m.d.Song.Patch[m.d.InstrIndex] = instrument
 	if m.d.Song.Patch[m.d.InstrIndex].Comment != "" {
 		m.commentExpanded = true
diff --git a/tracker/model.go b/tracker/model.go
index f0af45e8..676bd3ad 100644
--- a/tracker/model.go
+++ b/tracker/model.go
@@ -225,6 +225,7 @@ func (m *Model) change(kind string, t ChangeType, severity ChangeSeverity) func(
 				m.send(m.d.Song.Score.Copy())
 			}
 			if m.changeType&PatchChange != 0 {
+				m.fixIDCollisions()
 				m.d.InstrIndex = clamp(m.d.InstrIndex, 0, len(m.d.Song.Patch)-1)
 				m.d.InstrIndex2 = clamp(m.d.InstrIndex2, 0, len(m.d.Song.Patch)-1)
 				unitCount := 0
@@ -441,6 +442,37 @@ func (m *Model) assignUnitIDs(units []sointu.Unit) {
 	}
 }
 
+func (m *Model) fixIDCollisions() {
+	// loop over all instruments and units and check if two units have the same
+	// ID. If so, give the later units new IDs.
+	usedIDs := map[int]bool{}
+	needsFix := false
+	maxID := 0
+	for i, instr := range m.d.Song.Patch {
+		for j, unit := range instr.Units {
+			if usedIDs[unit.ID] {
+				m.d.Song.Patch[i].Units[j].ID = 0
+				needsFix = true
+			}
+			if unit.ID > maxID {
+				maxID = unit.ID
+			}
+			usedIDs[unit.ID] = true
+		}
+	}
+	if needsFix {
+		m.Alerts().Add("Some units had duplicate IDs, they were fixed", Error)
+		for i, instr := range m.d.Song.Patch {
+			for j, unit := range instr.Units {
+				if unit.ID == 0 {
+					maxID++
+					m.d.Song.Patch[i].Units[j].ID = maxID
+				}
+			}
+		}
+	}
+}
+
 func (m *Model) updatePatternUseCount() {
 	for i, track := range m.d.Song.Score.Tracks {
 		for len(m.cachePatternUseCount) <= i {