From 6232b7ceb0548ecd9031230d3b90d9b372ef599a 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     |  4 ++++
 tracker/files.go |  1 +
 tracker/model.go | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 772b68ba..3d184d5c 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,5 @@ 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..928c3bd0 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.fixIDCorruptions()
 				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) fixIDCorruptions() {
+	// 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 {