Replace ModuleHandle with ParamHandle. Move touchedParam from Engine to RackWidget. Fix param mapping bugs in MIDI-Map.

include/app/RackWidget.hpp

@@ -5,6 +5,7 @@
 #include "app/ModuleWidget.hpp"
 #include "app/CableWidget.hpp"
 #include "app/PortWidget.hpp"
+#include "app/ParamWidget.hpp"
 
 
 namespace rack {
@@ -18,6 +19,7 @@ struct RackWidget : widget::OpaqueWidget {
 	CableWidget *incompleteCable = NULL;
 	/** The last mouse position in the RackWidget */
 	math::Vec mousePos;
+	ParamWidget *touchedParam = NULL;
 
 	RackWidget();
 	~RackWidget();

include/engine/Engine.hpp

@@ -2,6 +2,7 @@
 #include "common.hpp"
 #include "engine/Module.hpp"
 #include "engine/Cable.hpp"
+#include "engine/ParamHandle.hpp"
 #include <vector>
 
 
@@ -55,12 +56,14 @@ struct Engine {
 	float getParam(Module *module, int paramId);
 	void setSmoothParam(Module *module, int paramId, float value);
 	float getSmoothParam(Module *module, int paramId);
-	void setTouchedParam(Module *module, int paramId);
-	void getTouchedParam(Module *&module, int &paramId);
-
-	// ModuleHandles
-	void addModuleHandle(ModuleHandle *moduleHandle);
-	void removeModuleHandle(ModuleHandle *moduleHandle);
+	void addParamHandle(ParamHandle *paramHandle);
+	void removeParamHandle(ParamHandle *paramHandle);
+	/** Returns the unique ParamHandle for the given paramId */
+	ParamHandle *getParamHandle(Module *module, int paramId);
+	/** Sets the ParamHandle IDs and module pointer.
+	If the given ParamHandle is added to the engine and another ParamHandle points to the same param, unsets that one and replaces it with the given handle.
+	*/
+	void updateParamHandle(ParamHandle *paramHandle, int moduleId, int paramId);
 };
 
 

include/engine/Module.hpp

@@ -49,6 +49,10 @@ struct Module {
 	virtual void onReset() {}
 	/** Called when user clicks Randomize in the module context menu. */
 	virtual void onRandomize() {}
+	/** Called when the Module is added to the Engine */
+	virtual void onAdd() {}
+	/** Called when the Module is removed from the Engine */
+	virtual void onRemove() {}
 
 	/** Override to store extra internal data in the "data" property of the module's JSON object. */
 	virtual json_t *dataToJson() { return NULL; }
@@ -56,12 +60,5 @@ struct Module {
 };
 
 
-struct ModuleHandle {
-	int id = -1;
-	/** Automatically set when added to the Engine. */
-	Module *module = NULL;
-};
-
-
 } // namespace engine
 } // namespace rack

include/engine/ParamHandle.hpp

@@ -0,0 +1,33 @@
+#pragma once
+#include "common.hpp"
+#include "engine/Module.hpp"
+#include "engine/Param.hpp"
+
+
+namespace rack {
+namespace engine {
+
+
+/** A weak handle to a Param. Managed by Engine */
+struct ParamHandle {
+	/** Do not set these directly.
+	They are handled by Engine methods.
+	*/
+	int moduleId;
+	int paramId;
+	Module *module;
+
+	ParamHandle() {
+		reset();
+	}
+
+	void reset() {
+		moduleId = -1;
+		paramId = 0;
+		module = NULL;
+	}
+};
+
+
+} // namespace engine
+} // namespace rack

src/Core/MIDI_Map.cpp

@@ -24,10 +24,8 @@ struct MIDI_Map : Module {
 	bool learnedParam;
 	/** The learned CC number of each channel */
 	int learnedCcs[8];
-	/** The learned module handle of each channel */
-	ModuleHandle learnedModuleHandles[8];
-	/** The learned param ID of each channel */
-	int learnedParamIds[8];
+	/** The learned param handle of each channel */
+	ParamHandle learnedParamHandles[8];
 	/** The value of each CC number */
 	int8_t values[128];
 	/** The smoothing processor (normalized between 0 and 1) of each channel */
@@ -36,6 +34,7 @@ struct MIDI_Map : Module {
 	MIDI_Map() {
 		config(NUM_PARAMS, NUM_INPUTS, NUM_OUTPUTS, NUM_LIGHTS);
 		for (int i = 0; i < 8; i++) {
+			APP->engine->addParamHandle(&learnedParamHandles[i]);
 			valueFilters[i].lambda = 60.f;
 		}
 		onReset();
@@ -43,7 +42,7 @@ struct MIDI_Map : Module {
 
 	~MIDI_Map() {
 		for (int i = 0; i < 8; i++) {
-			unloadModuleHandle(i);
+			APP->engine->removeParamHandle(&learnedParamHandles[i]);
 		}
 	}
 
@@ -53,9 +52,7 @@ struct MIDI_Map : Module {
 		learnedParam = false;
 		for (int i = 0; i < 8; i++) {
 			learnedCcs[i] = -1;
-			unloadModuleHandle(i);
-			learnedModuleHandles[i].id = -1;
-			learnedParamIds[i] = 0;
+			APP->engine->updateParamHandle(&learnedParamHandles[i], -1, 0);
 			valueFilters[i].reset();
 		}
 		for (int i = 0; i < 128; i++) {
@@ -72,42 +69,27 @@ struct MIDI_Map : Module {
 
 		float deltaTime = APP->engine->getSampleTime();
 
-		// Check touched params when learning
-		if (learningId >= 0) {
-			Module *module;
-			int paramId;
-			APP->engine->getTouchedParam(module, paramId);
-			APP->engine->setTouchedParam(NULL, 0);
-			if (module) {
-				unloadModuleHandle(learningId);
-				learnedModuleHandles[learningId].id = module->id;
-				loadModuleHandle(learningId);
-				learnedParamIds[learningId] = paramId;
-				learnedParam = true;
-				commitLearn();
-			}
-		}
-
 		// Step channels
-		for (int i = 0; i < 8; i++) {
-			int cc = learnedCcs[i];
+		for (int id = 0; id < 8; id++) {
+			int cc = learnedCcs[id];
 			if (cc < 0)
 				continue;
+			// DEBUG("%d %d %p %d", id, learnedCcs[id], learnedParamHandles[id].module, learnedParamHandles[id].paramId);
 			// Check if CC value has been set
 			if (values[cc] < 0)
 				continue;
 			// Get module
-			Module *module = learnedModuleHandles[i].module;
+			Module *module = learnedParamHandles[id].module;
 			if (!module)
 				continue;
 			// Get param
-			int paramId = learnedParamIds[i];
+			int paramId = learnedParamHandles[id].paramId;
 			Param *param = &module->params[paramId];
 			if (!param->isBounded())
 				continue;
 			// Set param
 			float v = rescale(values[cc], 0, 127, 0.f, 1.f);
-			v = valueFilters[i].process(deltaTime, v);
+			v = valueFilters[id].process(deltaTime, v);
 			v = rescale(v, 0.f, 1.f, param->minValue, param->maxValue);
 			APP->engine->setParam(module, paramId, v);
 		}
@@ -135,18 +117,6 @@ struct MIDI_Map : Module {
 		values[cc] = msg.getValue();
 	}
 
-	void loadModuleHandle(int i) {
-		if (learnedModuleHandles[i].id >= 0) {
-			APP->engine->addModuleHandle(&learnedModuleHandles[i]);
-		}
-	}
-
-	void unloadModuleHandle(int i) {
-		if (learnedModuleHandles[i].id >= 0) {
-			APP->engine->removeModuleHandle(&learnedModuleHandles[i]);
-		}
-	}
-
 	void commitLearn() {
 		if (learningId < 0)
 			return;
@@ -159,7 +129,7 @@ struct MIDI_Map : Module {
 		learnedParam = false;
 		// Find next unlearned channel
 		while (++learningId < 8) {
-			if (learnedCcs[learningId] < 0 || learnedModuleHandles[learningId].id < 0)
+			if (learnedCcs[learningId] < 0 || learnedParamHandles[learningId].moduleId < 0)
 				return;
 		}
 		learningId = -1;
@@ -168,9 +138,7 @@ struct MIDI_Map : Module {
 	void clearLearn(int id) {
 		disableLearn(id);
 		learnedCcs[id] = -1;
-		unloadModuleHandle(id);
-		learnedModuleHandles[id].id = -1;
-		loadModuleHandle(id);
+		APP->engine->updateParamHandle(&learnedParamHandles[id], -1, 0);
 	}
 
 	void enableLearn(int id) {
@@ -184,11 +152,15 @@ struct MIDI_Map : Module {
 	void disableLearn(int id) {
 		if (learningId == id) {
 			learningId = -1;
-			learnedCc = false;
-			learnedParam = false;
 		}
 	}
 
+	void learnParam(int id, int moduleId, int paramId) {
+		APP->engine->updateParamHandle(&learnedParamHandles[id], moduleId, paramId);
+		learnedParam = true;
+		commitLearn();
+	}
+
 	json_t *dataToJson() override {
 		json_t *rootJ = json_object();
 
@@ -199,15 +171,12 @@ struct MIDI_Map : Module {
 		json_object_set_new(rootJ, "ccs", ccsJ);
 
 		json_t *moduleIdsJ = json_array();
-		for (int i = 0; i < 8; i++) {
-			json_array_append_new(moduleIdsJ, json_integer(learnedModuleHandles[i].id));
-		}
-		json_object_set_new(rootJ, "moduleIds", moduleIdsJ);
-
 		json_t *paramIdsJ = json_array();
 		for (int i = 0; i < 8; i++) {
-			json_array_append_new(paramIdsJ, json_integer(learnedParamIds[i]));
+			json_array_append_new(moduleIdsJ, json_integer(learnedParamHandles[i].moduleId));
+			json_array_append_new(paramIdsJ, json_integer(learnedParamHandles[i].paramId));
 		}
+		json_object_set_new(rootJ, "moduleIds", moduleIdsJ);
 		json_object_set_new(rootJ, "paramIds", paramIdsJ);
 
 		json_object_set_new(rootJ, "midi", midiInput.toJson());
@@ -225,22 +194,13 @@ struct MIDI_Map : Module {
 		}
 
 		json_t *moduleIdsJ = json_object_get(rootJ, "moduleIds");
-		if (moduleIdsJ) {
-			for (int i = 0; i < 8; i++) {
-				json_t *moduleIdJ = json_array_get(moduleIdsJ, i);
-				unloadModuleHandle(i);
-				if (moduleIdJ)
-					learnedModuleHandles[i].id = json_integer_value(moduleIdJ);
-				loadModuleHandle(i);
-			}
-		}
-
 		json_t *paramIdsJ = json_object_get(rootJ, "paramIds");
-		if (paramIdsJ) {
+		if (moduleIdsJ && paramIdsJ) {
 			for (int i = 0; i < 8; i++) {
+				json_t *moduleIdJ = json_array_get(moduleIdsJ, i);
 				json_t *paramIdJ = json_array_get(paramIdsJ, i);
-				if (paramIdJ)
-					learnedParamIds[i] = json_integer_value(paramIdJ);
+				if (moduleIdJ && paramIdsJ)
+					APP->engine->updateParamHandle(&learnedParamHandles[i], json_integer_value(moduleIdJ), json_integer_value(paramIdJ));
 			}
 		}
 
@@ -254,21 +214,22 @@ struct MIDI_Map : Module {
 struct MIDI_MapChoice : LedDisplayChoice {
 	MIDI_Map *module;
 	int id;
+	int disableLearnFrames = -1;
 
 	void setModule(MIDI_Map *module) {
 		this->module = module;
 	}
 
 	void onButton(const event::Button &e) override {
+		if (!module)
+			return;
+
 		if (e.action == GLFW_PRESS && e.button == GLFW_MOUSE_BUTTON_LEFT) {
-			APP->engine->setTouchedParam(NULL, 0);
 			e.consume(this);
 		}
 
 		if (e.action == GLFW_PRESS && e.button == GLFW_MOUSE_BUTTON_RIGHT) {
-			if (module) {
-				module->clearLearn(id);
-			}
+			module->clearLearn(id);
 			e.consume(this);
 		}
 	}
@@ -276,6 +237,8 @@ struct MIDI_MapChoice : LedDisplayChoice {
 	void onSelect(const event::Select &e) override {
 		if (!module)
 			return;
+		// Reset touchedParam
+		APP->scene->rackWidget->touchedParam = NULL;
 		module->enableLearn(id);
 		e.consume(this);
 	}
@@ -283,7 +246,17 @@ struct MIDI_MapChoice : LedDisplayChoice {
 	void onDeselect(const event::Deselect &e) override {
 		if (!module)
 			return;
-		module->disableLearn(id);
+		// Check if a ParamWidget was touched
+		ParamWidget *touchedParam = APP->scene->rackWidget->touchedParam;
+		if (touchedParam) {
+			APP->scene->rackWidget->touchedParam = NULL;
+			int moduleId = touchedParam->paramQuantity->module->id;
+			int paramId = touchedParam->paramQuantity->paramId;
+			module->learnParam(id, moduleId, paramId);
+		}
+		else {
+			module->disableLearn(id);
+		}
 	}
 
 	void step() override {
@@ -303,7 +276,6 @@ struct MIDI_MapChoice : LedDisplayChoice {
 			bgColor = nvgRGBA(0, 0, 0, 0);
 
 			// HACK
-			// Don't let the event state call onDeselect()
 			if (APP->event->selectedWidget == this)
 				APP->event->setSelected(NULL);
 		}
@@ -313,10 +285,10 @@ struct MIDI_MapChoice : LedDisplayChoice {
 		if (module->learnedCcs[id] >= 0) {
 			text += string::f("CC%d ", module->learnedCcs[id]);
 		}
-		if (module->learnedModuleHandles[id].id >= 0) {
+		if (module->learnedParamHandles[id].moduleId >= 0) {
 			text += getParamName();
 		}
-		if (module->learnedCcs[id] < 0 && module->learnedModuleHandles[id].id < 0) {
+		if (module->learnedCcs[id] < 0 && module->learnedParamHandles[id].moduleId < 0) {
 			if (module->learningId == id) {
 				text = "Mapping...";
 			}
@@ -326,7 +298,7 @@ struct MIDI_MapChoice : LedDisplayChoice {
 		}
 
 		// Set text color
-		if ((module->learnedCcs[id] >= 0 && module->learnedModuleHandles[id].id >= 0) || module->learningId == id) {
+		if ((module->learnedCcs[id] >= 0 && module->learnedParamHandles[id].moduleId >= 0) || module->learningId == id) {
 			color.a = 1.0;
 		}
 		else {
@@ -337,18 +309,18 @@ struct MIDI_MapChoice : LedDisplayChoice {
 	std::string getParamName() {
 		if (!module)
 			return "";
-		ModuleHandle *moduleHandle = &module->learnedModuleHandles[id];
-		if (moduleHandle->id < 0)
+		ParamHandle *paramHandle = &module->learnedParamHandles[id];
+		if (paramHandle->moduleId < 0)
 			return "";
-		ModuleWidget *mw = APP->scene->rackWidget->getModule(moduleHandle->id);
+		ModuleWidget *mw = APP->scene->rackWidget->getModule(paramHandle->moduleId);
 		if (!mw)
 			return "";
-		// Get the Module from the ModuleWidget instead of the ModuleHandle.
+		// Get the Module from the ModuleWidget instead of the ParamHandle.
 		// I think this is more elegant since this method is called in the app world instead of the engine world.
 		Module *m = mw->module;
 		if (!m)
 			return "";
-		int paramId = module->learnedParamIds[id];
+		int paramId = paramHandle->paramId;
 		if (paramId >= (int) m->params.size())
 			return "";
 		Param *param = &m->params[paramId];

src/app/ParamWidget.cpp

@@ -147,7 +147,7 @@ void ParamWidget::onButton(const event::Button &e) {
 	// Touch parameter
 	if (e.action == GLFW_PRESS && e.button == GLFW_MOUSE_BUTTON_LEFT && (e.mods & WINDOW_MOD_MASK) == 0) {
 		if (paramQuantity) {
-			APP->engine->setTouchedParam(paramQuantity->module, paramQuantity->paramId);
+			APP->scene->rackWidget->touchedParam = this;
 		}
 	}
 

src/app/RackWidget.cpp

@@ -362,6 +362,13 @@ void RackWidget::addModuleAtMouse(ModuleWidget *m) {
 }
 
 void RackWidget::removeModule(ModuleWidget *m) {
+	// Unset touchedParamWidget
+	if (touchedParam) {
+		ModuleWidget *touchedModule = touchedParam->getAncestorOfType<ModuleWidget>();
+		if (touchedModule == m)
+			touchedParam = NULL;
+	}
+
 	// Disconnect cables
 	m->disconnect();
 

src/engine/Engine.cpp

@@ -127,7 +127,7 @@ struct EngineWorker {
 struct Engine::Internal {
 	std::vector<Module*> modules;
 	std::vector<Cable*> cables;
-	std::vector<ModuleHandle*> moduleHandles;
+	std::vector<ParamHandle*> paramHandles;
 	bool paused = false;
 
 	bool running = false;
@@ -150,9 +150,6 @@ struct Engine::Internal {
 	std::vector<EngineWorker> workers;
 	SpinBarrier engineBarrier;
 	SpinBarrier workerBarrier;
-
-	Module *touchedModule = NULL;
-	int touchedParamId = 0;
 };
 
 
@@ -177,7 +174,7 @@ Engine::~Engine() {
 	// If this happens, a module must have failed to remove itself before the RackWidget was destroyed.
 	assert(internal->cables.empty());
 	assert(internal->modules.empty());
-	assert(internal->moduleHandles.empty());
+	assert(internal->paramHandles.empty());
 
 	delete internal;
 }
@@ -406,13 +403,14 @@ void Engine::addModule(Module *module) {
 			internal->nextModuleId = module->id + 1;
 		}
 	}
-	// Update ModuleHandle
-	for (ModuleHandle *moduleHandle : internal->moduleHandles) {
-		if (moduleHandle->id == module->id)
-			moduleHandle->module = module;
-	}
 	// Add module
 	internal->modules.push_back(module);
+	module->onAdd();
+	// Update ParamHandles
+	for (ParamHandle *paramHandle : internal->paramHandles) {
+		if (paramHandle->moduleId == module->id)
+			paramHandle->module = module;
+	}
 }
 
 void Engine::removeModule(Module *module) {
@@ -428,20 +426,16 @@ void Engine::removeModule(Module *module) {
 		assert(cable->outputModule != module);
 		assert(cable->inputModule != module);
 	}
-	// Remove touched param
-	if (internal->touchedModule == module) {
-		internal->touchedModule = NULL;
-		internal->touchedParamId = 0;
-	}
-	// Update ModuleHandle
-	for (ModuleHandle *moduleHandle : internal->moduleHandles) {
-		if (moduleHandle->id == module->id)
-			moduleHandle->module = NULL;
+	// Update ParamHandles
+	for (ParamHandle *paramHandle : internal->paramHandles) {
+		if (paramHandle->moduleId == module->id)
+			paramHandle->module = NULL;
 	}
 	// Check that the module actually exists
 	auto it = std::find(internal->modules.begin(), internal->modules.end(), module);
 	assert(it != internal->modules.end());
 	// Remove the module
+	module->onRemove();
 	internal->modules.erase(it);
 }
 
@@ -587,37 +581,66 @@ float Engine::getSmoothParam(Module *module, int paramId) {
 	return getParam(module, paramId);
 }
 
-void Engine::setTouchedParam(Module *module, int paramId) {
-	internal->touchedModule = module;
-	internal->touchedParamId = paramId;
-}
+void Engine::addParamHandle(ParamHandle *paramHandle) {
+	VIPLock vipLock(internal->vipMutex);
+	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+
+	// Check that the ParamHandle is not already added
+	auto it = std::find(internal->paramHandles.begin(), internal->paramHandles.end(), paramHandle);
+	assert(it == internal->paramHandles.end());
 
-void Engine::getTouchedParam(Module *&module, int &paramId) {
-	module = internal->touchedModule;
-	paramId = internal->touchedParamId;
+	updateParamHandle(paramHandle, paramHandle->moduleId, paramHandle->paramId);
+	internal->paramHandles.push_back(paramHandle);
 }
 
-void Engine::addModuleHandle(ModuleHandle *moduleHandle) {
+void Engine::removeParamHandle(ParamHandle *paramHandle) {
 	VIPLock vipLock(internal->vipMutex);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 
-	// Check that the ModuleHandle is not already added
-	auto it = std::find(internal->moduleHandles.begin(), internal->moduleHandles.end(), moduleHandle);
-	assert(it == internal->moduleHandles.end());
+	paramHandle->module = NULL;
+	// Check that the ParamHandle is already added
+	auto it = std::find(internal->paramHandles.begin(), internal->paramHandles.end(), paramHandle);
+	assert(it != internal->paramHandles.end());
+	internal->paramHandles.erase(it);
+}
 
-	moduleHandle->module = getModule(moduleHandle->id);
-	internal->moduleHandles.push_back(moduleHandle);
+ParamHandle *Engine::getParamHandle(Module *module, int paramId) {
+	VIPLock vipLock(internal->vipMutex);
+	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+
+	for (ParamHandle *paramHandle : internal->paramHandles) {
+		if (paramHandle->module == module && paramHandle->paramId == paramId)
+			return paramHandle;
+	}
+	return NULL;
 }
 
-void Engine::removeModuleHandle(ModuleHandle *moduleHandle) {
+void Engine::updateParamHandle(ParamHandle *paramHandle, int moduleId, int paramId) {
 	VIPLock vipLock(internal->vipMutex);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 
-	moduleHandle->module = NULL;
-	// Check that the ModuleHandle is already added
-	auto it = std::find(internal->moduleHandles.begin(), internal->moduleHandles.end(), moduleHandle);
-	assert(it != internal->moduleHandles.end());
-	internal->moduleHandles.erase(it);
+	// Set IDs
+	paramHandle->moduleId = moduleId;
+	paramHandle->paramId = paramId;
+	paramHandle->module = NULL;
+
+	auto it = std::find(internal->paramHandles.begin(), internal->paramHandles.end(), paramHandle);
+
+	if (it != internal->paramHandles.end() && paramHandle->moduleId >= 0) {
+		// Remove existing ParamHandles pointing to the same param
+		for (ParamHandle *p : internal->paramHandles) {
+			if (p != paramHandle && p->moduleId == moduleId && p->paramId == paramId) {
+				p->reset();
+			}
+		}
+		// Find module with same moduleId
+		for (Module *module : internal->modules) {
+			if (module->id == moduleId) {
+				paramHandle->module = module;
+			}
+		}
+	}
+	DEBUG("%d %p %d %d", it != internal->paramHandles.end(), paramHandle->module, paramHandle->moduleId, paramHandle->paramId);
 }