Use various other history actions when interacting with the rack

include/app/ModuleWidget.hpp

@@ -62,8 +62,8 @@ struct ModuleWidget : OpaqueWidget {
 
 	/** Serializes/unserializes the module state */
 	void copyClipboard();
-	void pasteClipboard();
-	void load(std::string filename);
+	void pasteClipboardAction();
+	void loadAction(std::string filename);
 	void save(std::string filename);
 	void loadDialog();
 	void saveDialog();
@@ -72,18 +72,20 @@ struct ModuleWidget : OpaqueWidget {
 	Called when the user clicks Disconnect Cables in the context menu.
 	*/
 	void disconnect();
+
 	/** Resets the parameters of the module and calls the Module's randomize().
 	Called when the user clicks Initialize in the context menu.
 	*/
-	void reset();
+	void resetAction();
 	/** Randomizes the parameters of the module and calls the Module's randomize().
 	Called when the user clicks Randomize in the context menu.
 	*/
-	void randomize();
-
-	void removeAction();
-	void bypassAction();
+	void randomizeAction();
+	void disconnectAction();
 	void cloneAction();
+	void bypassAction();
+	/** Deletes `this` */
+	void removeAction();
 	void createContextMenu();
 	/** Override to add context menu entries to your subclass.
 	It is recommended to add a blank MenuEntry first for spacing.

include/app/PortWidget.hpp

@@ -21,8 +21,10 @@ struct PortWidget : OpaqueWidget {
 
 	PortWidget();
 	~PortWidget();
+
 	void step() override;
 	void draw(NVGcontext *vg) override;
+
 	void onButton(const event::Button &e) override;
 	void onDragStart(const event::DragStart &e) override;
 	void onDragEnd(const event::DragEnd &e) override;

include/app/RackWidget.hpp

@@ -25,6 +25,7 @@ struct RackWidget : OpaqueWidget {
 	void draw(NVGcontext *vg) override;
 
 	void onHover(const event::Hover &e) override;
+	void onHoverKey(const event::HoverKey &e) override;
 	void onDragHover(const event::DragHover &e) override;
 	void onButton(const event::Button &e) override;
 	void onZoom(const event::Zoom &e) override;
@@ -33,7 +34,7 @@ struct RackWidget : OpaqueWidget {
 	void clear();
 	json_t *toJson();
 	void fromJson(json_t *rootJ);
-	void pastePresetClipboard();
+	void pastePresetClipboardAction();
 
 	// Module methods
 
@@ -67,6 +68,8 @@ struct RackWidget : OpaqueWidget {
 	/** Returns the most recently added complete cable connected to the given Port, i.e. the top of the stack */
 	CableWidget *getTopCable(PortWidget *port);
 	CableWidget *getCable(int cableId);
+	/** Returns all cables attached to port, complete or not */
+	std::list<CableWidget*> getCablesOnPort(PortWidget *port);
 };
 
 

include/history.hpp

@@ -83,6 +83,15 @@ struct ModuleBypass : ModuleAction {
 };
 
 
+struct ModuleChange : ModuleAction {
+	json_t *oldModuleJ;
+	json_t *newModuleJ;
+	~ModuleChange();
+	void undo() override;
+	void redo() override;
+};
+
+
 struct ParamChange : ModuleAction {
 	int paramId;
 	float oldValue;

src/app/ModuleWidget.cpp

@@ -22,7 +22,7 @@ struct ModuleDisconnectItem : MenuItem {
 		rightText = WINDOW_MOD_CTRL_NAME "+U";
 	}
 	void onAction(const event::Action &e) override {
-		moduleWidget->disconnect();
+		moduleWidget->disconnectAction();
 	}
 };
 
@@ -33,7 +33,7 @@ struct ModuleResetItem : MenuItem {
 		rightText = WINDOW_MOD_CTRL_NAME "+I";
 	}
 	void onAction(const event::Action &e) override {
-		moduleWidget->reset();
+		moduleWidget->resetAction();
 	}
 };
 
@@ -44,7 +44,7 @@ struct ModuleRandomizeItem : MenuItem {
 		rightText = WINDOW_MOD_CTRL_NAME "+R";
 	}
 	void onAction(const event::Action &e) override {
-		moduleWidget->randomize();
+		moduleWidget->randomizeAction();
 	}
 };
 
@@ -66,7 +66,7 @@ struct ModulePasteItem : MenuItem {
 		rightText = WINDOW_MOD_CTRL_NAME "+V";
 	}
 	void onAction(const event::Action &e) override {
-		moduleWidget->pasteClipboard();
+		moduleWidget->pasteClipboardAction();
 	}
 };
 
@@ -214,13 +214,13 @@ void ModuleWidget::onHoverKey(const event::HoverKey &e) {
 		switch (e.key) {
 			case GLFW_KEY_I: {
 				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
-					reset();
+					resetAction();
 					e.consume(this);
 				}
 			} break;
 			case GLFW_KEY_R: {
 				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
-					randomize();
+					randomizeAction();
 					e.consume(this);
 				}
 			} break;
@@ -232,18 +232,19 @@ void ModuleWidget::onHoverKey(const event::HoverKey &e) {
 			} break;
 			case GLFW_KEY_V: {
 				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
-					pasteClipboard();
+					pasteClipboardAction();
 					e.consume(this);
 				}
 			} break;
 			case GLFW_KEY_D: {
 				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
 					cloneAction();
+					e.consume(this);
 				}
 			} break;
 			case GLFW_KEY_U: {
 				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
-					disconnect();
+					disconnectAction();
 					e.consume(this);
 				}
 			} break;
@@ -267,7 +268,7 @@ void ModuleWidget::onDragStart(const event::DragStart &e) {
 
 void ModuleWidget::onDragEnd(const event::DragEnd &e) {
 	if (!box.pos.isEqual(oldPos)) {
-		// Push ModuleMove history action
+		// history::ModuleMove
 		history::ModuleMove *h = new history::ModuleMove;
 		h->moduleId = module->id;
 		h->oldPos = oldPos;
@@ -412,7 +413,7 @@ void ModuleWidget::copyClipboard() {
 	glfwSetClipboardString(app()->window->win, moduleJson);
 }
 
-void ModuleWidget::pasteClipboard() {
+void ModuleWidget::pasteClipboardAction() {
 	const char *moduleJson = glfwGetClipboardString(app()->window->win);
 	if (!moduleJson) {
 		WARN("Could not get text from clipboard.");
@@ -429,10 +430,18 @@ void ModuleWidget::pasteClipboard() {
 		json_decref(moduleJ);
 	});
 
+	// history::ModuleChange
+	history::ModuleChange *h = new history::ModuleChange;
+	h->moduleId = module->id;
+	h->oldModuleJ = toJson();
+
 	fromJson(moduleJ);
+
+	h->newModuleJ = toJson();
+	app()->history->push(h);
 }
 
-void ModuleWidget::load(std::string filename) {
+void ModuleWidget::loadAction(std::string filename) {
 	INFO("Loading preset %s", filename.c_str());
 
 	FILE *file = fopen(filename.c_str(), "r");
@@ -455,7 +464,15 @@ void ModuleWidget::load(std::string filename) {
 		json_decref(moduleJ);
 	});
 
+	// history::ModuleChange
+	history::ModuleChange *h = new history::ModuleChange;
+	h->moduleId = module->id;
+	h->oldModuleJ = toJson();
+
 	fromJson(moduleJ);
+
+	h->newModuleJ = toJson();
+	app()->history->push(h);
 }
 
 void ModuleWidget::save(std::string filename) {
@@ -496,7 +513,7 @@ void ModuleWidget::loadDialog() {
 		free(path);
 	});
 
-	load(path);
+	loadAction(path);
 }
 
 void ModuleWidget::saveDialog() {
@@ -535,39 +552,68 @@ void ModuleWidget::disconnect() {
 	}
 }
 
-void ModuleWidget::reset() {
-	if (module) {
-		app()->engine->resetModule(module);
-	}
-}
+void ModuleWidget::resetAction() {
+	assert(module);
 
-void ModuleWidget::randomize() {
-	if (module) {
-		app()->engine->randomizeModule(module);
-	}
+	// history::ModuleChange
+	history::ModuleChange *h = new history::ModuleChange;
+	h->moduleId = module->id;
+	h->oldModuleJ = toJson();
+
+	app()->engine->resetModule(module);
+
+	h->newModuleJ = toJson();
+	app()->history->push(h);
 }
 
-void ModuleWidget::removeAction() {
-	history::ComplexAction *complexAction = new history::ComplexAction;
+void ModuleWidget::randomizeAction() {
+	assert(module);
 
-	// Push ModuleRemove history action
-	history::ModuleRemove *moduleRemove = new history::ModuleRemove;
-	moduleRemove->setModule(this);
-	complexAction->push(moduleRemove);
+	// history::ModuleChange
+	history::ModuleChange *h = new history::ModuleChange;
+	h->moduleId = module->id;
+	h->oldModuleJ = toJson();
 
-	app()->history->push(complexAction);
+	app()->engine->randomizeModule(module);
 
-	app()->scene->rackWidget->removeModule(this);
-	delete this;
+	h->newModuleJ = toJson();
+	app()->history->push(h);
 }
 
-void ModuleWidget::bypassAction() {
-	// Push ModuleBypass history action
-	history::ModuleBypass *h = new history::ModuleBypass;
-	h->moduleId = module->id;
-	h->bypass = !module->bypass;
-	app()->history->push(h);
-	h->redo();
+static void disconnectActions(ModuleWidget *mw, history::ComplexAction *complexAction) {
+	// Add CableRemove action for all cables attached to outputs
+	for (PortWidget* output : mw->outputs) {
+		for (CableWidget *cw : app()->scene->rackWidget->getCablesOnPort(output)) {
+			if (!cw->isComplete())
+				continue;
+			// history::CableRemove
+			history::CableRemove *h = new history::CableRemove;
+			h->setCable(cw);
+			complexAction->push(h);
+		}
+	}
+	// Add CableRemove action for all cables attached to inputs
+	for (PortWidget* input : mw->inputs) {
+		for (CableWidget *cw : app()->scene->rackWidget->getCablesOnPort(input)) {
+			if (!cw->isComplete())
+				continue;
+			// Avoid creating duplicate actions for self-patched cables
+			if (cw->outputPort->module == mw->module)
+				continue;
+			// history::CableRemove
+			history::CableRemove *h = new history::CableRemove;
+			h->setCable(cw);
+			complexAction->push(h);
+		}
+	}
+}
+
+void ModuleWidget::disconnectAction() {
+	history::ComplexAction *complexAction = new history::ComplexAction;
+	disconnectActions(this, complexAction);
+	app()->history->push(complexAction);
+
+	disconnect();
 }
 
 void ModuleWidget::cloneAction() {
@@ -580,12 +626,38 @@ void ModuleWidget::cloneAction() {
 
 	app()->scene->rackWidget->addModuleAtMouse(clonedModuleWidget);
 
-	// Push ModuleAdd history action
+	// history::ModuleAdd
 	history::ModuleAdd *h = new history::ModuleAdd;
 	h->setModule(clonedModuleWidget);
 	app()->history->push(h);
 }
 
+void ModuleWidget::bypassAction() {
+	assert(module);
+	// history::ModuleBypass
+	history::ModuleBypass *h = new history::ModuleBypass;
+	h->moduleId = module->id;
+	h->bypass = !module->bypass;
+	app()->history->push(h);
+	h->redo();
+}
+
+void ModuleWidget::removeAction() {
+	history::ComplexAction *complexAction = new history::ComplexAction;
+	disconnectActions(this, complexAction);
+
+	// history::ModuleRemove
+	history::ModuleRemove *moduleRemove = new history::ModuleRemove;
+	moduleRemove->setModule(this);
+	complexAction->push(moduleRemove);
+
+	app()->history->push(complexAction);
+
+	// This disconnects cables, removes the module, and transfers ownership to caller
+	app()->scene->rackWidget->removeModule(this);
+	delete this;
+}
+
 void ModuleWidget::createContextMenu() {
 	Menu *menu = createMenu();
 	assert(model);

src/app/PortWidget.cpp

@@ -108,8 +108,6 @@ void PortWidget::onDragStart(const event::DragStart &e) {
 }
 
 void PortWidget::onDragEnd(const event::DragEnd &e) {
-	// FIXME
-	// If the source PortWidget is deleted, this will be called, removing the cable
 	CableWidget *cw = app()->scene->rackWidget->releaseIncompleteCable();
 	if (cw->isComplete()) {
 		app()->scene->rackWidget->addCable(cw);

src/app/RackWidget.cpp

@@ -148,6 +148,22 @@ void RackWidget::onHover(const event::Hover &e) {
 	mousePos = e.pos;
 }
 
+void RackWidget::onHoverKey(const event::HoverKey &e) {
+	OpaqueWidget::onHoverKey(e);
+	if (e.getConsumed() != this)
+		return;
+
+	if (e.action == GLFW_PRESS || e.action == GLFW_REPEAT) {
+		switch (e.key) {
+			case GLFW_KEY_V: {
+				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
+					pastePresetClipboardAction();
+				}
+			} break;
+		}
+	}
+}
+
 void RackWidget::onDragHover(const event::DragHover &e) {
 	OpaqueWidget::onDragHover(e);
 	mousePos = e.pos;
@@ -287,7 +303,7 @@ void RackWidget::fromJson(json_t *rootJ) {
 	}
 }
 
-void RackWidget::pastePresetClipboard() {
+void RackWidget::pastePresetClipboardAction() {
 	const char *moduleJson = glfwGetClipboardString(app()->window->win);
 	if (!moduleJson) {
 		WARN("Could not get text from clipboard.");
@@ -297,13 +313,14 @@ void RackWidget::pastePresetClipboard() {
 	json_error_t error;
 	json_t *moduleJ = json_loads(moduleJson, 0, &error);
 	if (moduleJ) {
-		ModuleWidget *moduleWidget = moduleFromJson(moduleJ);
+		ModuleWidget *mw = moduleFromJson(moduleJ);
 		json_decref(moduleJ);
-		addModule(moduleWidget);
-		// Set moduleWidget position
-		math::Rect newBox = moduleWidget->box;
-		newBox.pos = mousePos.minus(newBox.size.div(2));
-		requestModuleBoxNearest(moduleWidget, newBox);
+		addModuleAtMouse(mw);
+
+		// history::ModuleAdd
+		history::ModuleAdd *h = new history::ModuleAdd;
+		h->setModule(mw);
+		app()->history->push(h);
 	}
 	else {
 		WARN("JSON parsing error at %s %d:%d %s", error.source, error.line, error.column, error.text);
@@ -349,7 +366,9 @@ bool RackWidget::requestModuleBox(ModuleWidget *m, math::Rect requestedBox) {
 
 	// Check intersection with other modules
 	for (Widget *m2 : moduleContainer->children) {
-		if (m == m2) continue;
+		// Don't intersect with self
+		if (m == m2)
+			continue;
 		if (requestedBox.intersects(m2->box)) {
 			return false;
 		}
@@ -401,8 +420,10 @@ void RackWidget::clearCables() {
 	for (Widget *w : cableContainer->children) {
 		CableWidget *cw = dynamic_cast<CableWidget*>(w);
 		assert(cw);
-		if (cw != incompleteCable)
-			app()->engine->removeCable(cw->cable);
+		if (!cw->isComplete())
+			continue;
+
+		app()->engine->removeCable(cw->cable);
 	}
 	incompleteCable = NULL;
 	cableContainer->clearChildren();
@@ -415,7 +436,7 @@ void RackWidget::clearCablesAction() {
 	for (Widget *w : cableContainer->children) {
 		CableWidget *cw = dynamic_cast<CableWidget*>(w);
 		assert(cw);
-		if (cw == incompleteCable)
+		if (!cw->isComplete())
 			continue;
 
 		// history::CableRemove
@@ -429,23 +450,16 @@ void RackWidget::clearCablesAction() {
 }
 
 void RackWidget::clearCablesOnPort(PortWidget *port) {
-	assert(port);
-	std::list<Widget*> childrenCopy = cableContainer->children;
-	for (Widget *w : childrenCopy) {
-		CableWidget *cw = dynamic_cast<CableWidget*>(w);
-		assert(cw);
-
+	for (CableWidget *cw : getCablesOnPort(port)) {
 		// Check if cable is connected to port
-		if (cw->inputPort == port || cw->outputPort == port) {
-			if (cw == incompleteCable) {
-				incompleteCable = NULL;
-				cableContainer->removeChild(cw);
-			}
-			else {
-				removeCable(cw);
-			}
-			delete cw;
+		if (cw == incompleteCable) {
+			incompleteCable = NULL;
+			cableContainer->removeChild(cw);
+		}
+		else {
+			removeCable(cw);
 		}
+		delete cw;
 	}
 }
 
@@ -503,5 +517,18 @@ CableWidget *RackWidget::getCable(int cableId) {
 	return NULL;
 }
 
+std::list<CableWidget*> RackWidget::getCablesOnPort(PortWidget *port) {
+	assert(port);
+	std::list<CableWidget*> cables;
+	for (Widget *w : cableContainer->children) {
+		CableWidget *cw = dynamic_cast<CableWidget*>(w);
+		assert(cw);
+		if (cw->inputPort == port || cw->outputPort == port) {
+			cables.push_back(cw);
+		}
+	}
+	return cables;
+}
+
 
 } // namespace rack

src/app/Scene.cpp

@@ -124,12 +124,6 @@ void Scene::onHoverKey(const event::HoverKey &e) {
 					e.consume(this);
 				}
 			} break;
-			case GLFW_KEY_V: {
-				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
-					rackWidget->pastePresetClipboard();
-					e.consume(this);
-				}
-			} break;
 			case GLFW_KEY_Z: {
 				if ((e.mods & WINDOW_MOD_MASK) == WINDOW_MOD_CTRL) {
 					app()->history->undo();

src/engine/Engine.cpp

@@ -50,8 +50,6 @@ struct Engine::Internal {
 	float sampleTime;
 	float sampleRateRequested;
 
-	Module *resetModule = NULL;
-	Module *randomizeModule = NULL;
 	int nextModuleId = 1;
 	int nextCableId = 1;
 
@@ -93,16 +91,6 @@ static void Engine_step(Engine *engine) {
 		}
 	}
 
-	// Events
-	if (engine->internal->resetModule) {
-		engine->internal->resetModule->reset();
-		engine->internal->resetModule = NULL;
-	}
-	if (engine->internal->randomizeModule) {
-		engine->internal->randomizeModule->randomize();
-		engine->internal->randomizeModule = NULL;
-	}
-
 	// Param smoothing
 	{
 		Module *smoothModule = engine->internal->smoothModule;
@@ -273,11 +261,17 @@ void Engine::removeModule(Module *module) {
 }
 
 void Engine::resetModule(Module *module) {
-	internal->resetModule = module;
+	assert(module);
+	VIPLock vipLock(internal->vipMutex);
+	std::lock_guard<std::mutex> lock(internal->mutex);
+	module->reset();
 }
 
 void Engine::randomizeModule(Module *module) {
-	internal->randomizeModule = module;
+	assert(module);
+	VIPLock vipLock(internal->vipMutex);
+	std::lock_guard<std::mutex> lock(internal->mutex);
+	module->randomize();
 }
 
 static void Engine_updateActive(Engine *engine) {
@@ -344,7 +338,7 @@ void Engine::removeCable(Cable *cable) {
 }
 
 void Engine::setParam(Module *module, int paramId, float value) {
-	// TODO Make thread safe
+	// TODO Does this need to be thread-safe?
 	module->params[paramId].value = value;
 }
 

src/history.cpp

@@ -90,6 +90,24 @@ void ModuleBypass::redo() {
 }
 
 
+ModuleChange::~ModuleChange() {
+	json_decref(oldModuleJ);
+	json_decref(newModuleJ);
+}
+
+void ModuleChange::undo() {
+	ModuleWidget *mw = app()->scene->rackWidget->getModule(moduleId);
+	assert(mw);
+	mw->fromJson(oldModuleJ);
+}
+
+void ModuleChange::redo() {
+	ModuleWidget *mw = app()->scene->rackWidget->getModule(moduleId);
+	assert(mw);
+	mw->fromJson(newModuleJ);
+}
+
+
 void ParamChange::undo() {
 	ModuleWidget *mw = app()->scene->rackWidget->getModule(moduleId);
 	assert(mw);