Make Module always deserialize "id", "leftModuleId", and "rightModuleId". Due to this, make pasting and cloning modules delete these properties from the JSON before `fromJson()` is called.

include/helpers.hpp

@@ -26,7 +26,7 @@ plugin::Model* createModel(const std::string& slug) {
 			return m;
 		}
 		app::ModuleWidget* createModuleWidget(engine::Module* m) override {
-			TModule *tm = NULL;
+			TModule* tm = NULL;
 			if (m) {
 				assert(m->model == this);
 				tm = dynamic_cast<TModule*>(m);

src/app/ModuleWidget.cpp

@@ -611,8 +611,6 @@ PortWidget* ModuleWidget::getOutput(int portId) {
 
 json_t* ModuleWidget::toJson() {
 	json_t* moduleJ = APP->engine->moduleToJson(module);
-	// When serializing ModuleWidget, don't include the ID. This ID is only meaningful when serializing the entire rack.
-	json_object_del(moduleJ, "id");
 	return moduleJ;
 }
 
@@ -622,13 +620,9 @@ void ModuleWidget::fromJson(json_t* rootJ) {
 
 void ModuleWidget::copyClipboard() {
 	json_t* moduleJ = toJson();
-	DEFER({
-		json_decref(moduleJ);
-	});
+	DEFER({json_decref(moduleJ);});
 	char* moduleJson = json_dumps(moduleJ, JSON_INDENT(2) | JSON_REAL_PRECISION(9));
-	DEFER({
-		free(moduleJson);
-	});
+	DEFER({std::free(moduleJson);});
 	glfwSetClipboardString(APP->window->win, moduleJson);
 }
 
@@ -645,9 +639,12 @@ void ModuleWidget::pasteClipboardAction() {
 		WARN("JSON parsing error at %s %d:%d %s", error.source, error.line, error.column, error.text);
 		return;
 	}
-	DEFER({
-		json_decref(moduleJ);
-	});
+	DEFER({json_decref(moduleJ);});
+
+	// Don't use IDs from JSON
+	json_object_del(moduleJ, "id");
+	json_object_del(moduleJ, "leftModuleId");
+	json_object_del(moduleJ, "rightModuleId");
 
 	// history::ModuleChange
 	history::ModuleChange* h = new history::ModuleChange;
@@ -665,9 +662,7 @@ void ModuleWidget::load(std::string filename) {
 	FILE* file = std::fopen(filename.c_str(), "r");
 	if (!file)
 		throw Exception(string::f("Could not load patch file %s", filename.c_str()));
-	DEFER({
-		std::fclose(file);
-	});
+	DEFER({std::fclose(file);});
 
 	INFO("Loading preset %s", filename.c_str());
 
@@ -675,9 +670,12 @@ void ModuleWidget::load(std::string filename) {
 	json_t* moduleJ = json_loadf(file, 0, &error);
 	if (!moduleJ)
 		throw Exception(string::f("File is not a valid patch file. JSON parsing error at %s %d:%d %s", error.source, error.line, error.column, error.text));
-	DEFER({
-		json_decref(moduleJ);
-	});
+	DEFER({json_decref(moduleJ);});
+
+	// Don't use IDs from JSON
+	json_object_del(moduleJ, "id");
+	json_object_del(moduleJ, "leftModuleId");
+	json_object_del(moduleJ, "rightModuleId");
 
 	fromJson(moduleJ);
 }
@@ -729,7 +727,7 @@ void ModuleWidget::loadDialog() {
 		// No path selected
 		return;
 	}
-	DEFER({free(pathC);});
+	DEFER({std::free(pathC);});
 
 	try {
 		loadAction(pathC);
@@ -744,17 +742,13 @@ void ModuleWidget::save(std::string filename) {
 
 	json_t* moduleJ = toJson();
 	assert(moduleJ);
-	DEFER({
-		json_decref(moduleJ);
-	});
+	DEFER({json_decref(moduleJ);});
 
-	FILE* file = fopen(filename.c_str(), "w");
+	FILE* file = std::fopen(filename.c_str(), "w");
 	if (!file) {
 		WARN("Could not write to patch file %s", filename.c_str());
 	}
-	DEFER({
-		fclose(file);
-	});
+	DEFER({std::fclose(file);});
 
 	json_dumpf(moduleJ, file, JSON_INDENT(2) | JSON_REAL_PRECISION(9));
 }
@@ -786,7 +780,7 @@ void ModuleWidget::saveDialog() {
 		// No path selected
 		return;
 	}
-	DEFER({free(pathC);});
+	DEFER({std::free(pathC);});
 
 	std::string path = pathC;
 	if (system::getExtension(path) == "")
@@ -872,15 +866,18 @@ void ModuleWidget::cloneAction() {
 	// history::ComplexAction
 	history::ComplexAction* h = new history::ComplexAction;
 
-	// Clone Module
-	engine::Module* clonedModule = model->createModule();
 	// JSON serialization is the obvious way to do this
 	json_t* moduleJ = toJson();
+	// Don't use IDs from JSON
+	json_object_del(moduleJ, "id");
+	json_object_del(moduleJ, "leftModuleId");
+	json_object_del(moduleJ, "rightModuleId");
+
+	// Clone Module
+	engine::Module* clonedModule = model->createModule();
 	// This doesn't need a lock (via Engine::moduleFromJson()) because the Module is not added to the Engine yet.
 	clonedModule->fromJson(moduleJ);
 	json_decref(moduleJ);
-	// Reset ID so the Engine automatically assigns a new one
-	clonedModule->id = -1;
 	APP->engine->addModule(clonedModule);
 
 	// Clone ModuleWidget

src/app/RackWidget.cpp

@@ -22,8 +22,9 @@ namespace app {
 
 
 /** Creates a new Module and ModuleWidget */
-ModuleWidget* moduleWidgetFromJson(json_t* moduleJ) {
+static ModuleWidget* moduleWidgetFromJson(json_t* moduleJ) {
 	plugin::Model* model = plugin::modelFromJson(moduleJ);
+	assert(model);
 	engine::Module* module = model->createModule();
 	assert(module);
 	module->fromJson(moduleJ);
@@ -287,10 +288,12 @@ void RackWidget::fromJson(json_t* rootJ) {
 		}
 
 		CableWidget* cw = new CableWidget;
-		// Legacy: Before v1, cable colors were not serialized. So we need to initialize the color here.
-		cw->setNextCableColor();
 		cw->setCable(cable);
 		cw->fromJson(cableJ);
+		// In <=v1, cable colors were not serialized, so choose one from the available colors.
+		if (cw->color.a == 0.f) {
+			cw->setNextCableColor();
+		}
 		addCable(cw);
 	}
 }
@@ -308,20 +311,19 @@ void RackWidget::pastePresetClipboardAction() {
 		WARN("JSON parsing error at %s %d:%d %s", error.source, error.line, error.column, error.text);
 		return;
 	}
-	DEFER({
-		json_decref(moduleJ);
-	});
+	DEFER({json_decref(moduleJ);});
+
+	// Because we are creating a new module, we don't want to use the IDs from the JSON.
+	json_object_del(moduleJ, "id");
+	json_object_del(moduleJ, "leftModuleId");
+	json_object_del(moduleJ, "rightModuleId");
 
 	try {
-		plugin::Model* model = plugin::modelFromJson(moduleJ);
-		engine::Module* module = model->createModule();
-		assert(module);
-		module->fromJson(moduleJ);
-		// Reset ID so the Engine automatically assigns a new one
-		module->id = -1;
-		APP->engine->addModule(module);
-
-		ModuleWidget* mw = module->model->createModuleWidget(module);
+		ModuleWidget* mw = moduleWidgetFromJson(moduleJ);
+		assert(mw);
+		assert(mw->module);
+
+		APP->engine->addModule(mw->module);
 		addModuleAtMouse(mw);
 
 		// history::ModuleAdd

src/engine/Module.cpp

@@ -152,13 +152,10 @@ void Module::fromJson(json_t* rootJ) {
 		}
 	}
 
-	// Only set ID if unset
-	if (id < 0) {
-		// id
-		json_t* idJ = json_object_get(rootJ, "id");
-		if (idJ)
-			id = json_integer_value(idJ);
-	}
+	// id
+	json_t* idJ = json_object_get(rootJ, "id");
+	if (idJ)
+		id = json_integer_value(idJ);
 
 	// params
 	json_t* paramsJ = json_object_get(rootJ, "params");
@@ -176,16 +173,15 @@ void Module::fromJson(json_t* rootJ) {
 	if (bypassedJ)
 		internal->bypassed = json_boolean_value(bypassedJ);
 
-	// These do not need to be deserialized, since the module positions will set them correctly when added to the rack.
-	// // leftModuleId
-	// json_t *leftModuleIdJ = json_object_get(rootJ, "leftModuleId");
-	// if (leftModuleIdJ)
-	// 	leftExpander.moduleId = json_integer_value(leftModuleIdJ);
+	// leftModuleId
+	json_t *leftModuleIdJ = json_object_get(rootJ, "leftModuleId");
+	if (leftModuleIdJ)
+		leftExpander.moduleId = json_integer_value(leftModuleIdJ);
 
-	// // rightModuleId
-	// json_t *rightModuleIdJ = json_object_get(rootJ, "rightModuleId");
-	// if (rightModuleIdJ)
-	// 	rightExpander.moduleId = json_integer_value(rightModuleIdJ);
+	// rightModuleId
+	json_t *rightModuleIdJ = json_object_get(rootJ, "rightModuleId");
+	if (rightModuleIdJ)
+		rightExpander.moduleId = json_integer_value(rightModuleIdJ);
 
 	// data
 	json_t* dataJ = json_object_get(rootJ, "data");

src/logger.cpp

@@ -101,9 +101,7 @@ bool isTruncated() {
 	FILE* file = std::fopen(asset::logPath.c_str(), "r");
 	if (!file)
 		return false;
-	DEFER({
-		std::fclose(file);
-	});
+	DEFER({std::fclose(file);});
 
 	if (fileEndsWith(file, "END"))
 		return false;

src/network.cpp

@@ -4,6 +4,7 @@
 #include <curl/curl.h>
 
 #include <network.hpp>
+#include <system.hpp>
 #include <asset.hpp>
 
 
@@ -160,7 +161,7 @@ static int xferInfoCallback(void* clientp, curl_off_t dltotal, curl_off_t dlnow,
 bool requestDownload(const std::string& url, const std::string& filename, float* progress, const CookieMap& cookies) {
 	CURL* curl = createCurl();
 
-	FILE* file = fopen(filename.c_str(), "wb");
+	FILE* file = std::fopen(filename.c_str(), "wb");
 	if (!file)
 		return false;
 
@@ -182,10 +183,10 @@ bool requestDownload(const std::string& url, const std::string& filename, float*
 	CURLcode res = curl_easy_perform(curl);
 	curl_easy_cleanup(curl);
 
-	fclose(file);
+	std::fclose(file);
 
 	if (res != CURLE_OK)
-		remove(filename.c_str());
+		system::remove(filename);
 
 	return res == CURLE_OK;
 }

src/patch.cpp

@@ -273,9 +273,7 @@ void PatchManager::loadAutosave() {
 		// TODO Load template without causing infinite recursion
 		return;
 	}
-	DEFER({
-		std::fclose(file);
-	});
+	DEFER({std::fclose(file);});
 
 	json_error_t error;
 	json_t* rootJ = json_loadf(file, 0, &error);

src/settings.cpp

@@ -260,9 +260,7 @@ void save(const std::string& path) {
 	FILE* file = std::fopen(path.c_str(), "w");
 	if (!file)
 		return;
-	DEFER({
-		std::fclose(file);
-	});
+	DEFER({std::fclose(file);});
 
 	json_dumpf(rootJ, file, JSON_INDENT(2) | JSON_REAL_PRECISION(9));
 	json_decref(rootJ);
@@ -273,9 +271,7 @@ void load(const std::string& path) {
 	FILE* file = std::fopen(path.c_str(), "r");
 	if (!file)
 		return;
-	DEFER({
-		std::fclose(file);
-	});
+	DEFER({std::fclose(file);});
 
 	json_error_t error;
 	json_t* rootJ = json_loadf(file, 0, &error);