MockbaTheBorg / MockbaModular

Mockba's plugin for VCV Rack
MIT License
15 stars 2 forks source link

1.1.3: Failed to load module panels #4

Closed cschol closed 4 years ago

cschol commented 4 years ago

Rack cannot load the module panels.

I think it has to do with the dynamic background loading. Is the configuration file MockbaModular.json supposed to exist by default? It does not.

MockbaTheBorg commented 4 years ago

The configuration should be created automagically by the module. I have tested here on Windows multiple times. Is it happening on Windows or other platform? Also, any change you can send me the log output? I am loading the "background" with the regular method of loading a panel:

        setModule(module);
        setPanel(APP->window->loadSvg(asset::plugin(pluginInstance, BGCOLOR)));

Then I load the rest of the panel as a regular Widget:

        SvgWidget* panel = createWidget<SvgWidget>(Vec(0, 0));
        panel->setSvg(APP->window->loadSvg(asset::plugin(pluginInstance, "res/Blank.svg")));
        addChild(panel);

So I am not sure why it wouldn't load. Does it not load the module at all? Loads partially? Or gives some error?

Thanks!

MockbaTheBorg commented 4 years ago

Ah ... and like even the Blank doesn't load? I have re-tested here and it loads fine. It is actually how I took the screenshots to put on the GitHub page.

cschol commented 4 years ago
[0.032 info src/plugin.cpp:154] Loaded plugin MockbaModular v1.1.3 from /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular
[2.762 warn src/window.cpp:75] Failed to load SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/i V
[2.762 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/Blank.svg
[2.762 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/screw.svg
[2.762 warn src/window.cpp:75] Failed to load SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/
[2.763 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/Feidah.svg
[2.763 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/knob.svg
[2.763 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/port.svg
[2.763 warn src/window.cpp:75] Failed to load SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/0̃
[2.763 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/FeidahS.svg
[2.763 warn src/window.cpp:75] Failed to load SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/.
[2.764 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/Filtah.svg
[2.764 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/HSW_0.svg
[2.764 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/HSW_1.svg
[2.764 info src/window.cpp:72] Loaded SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/res/HSW_2.svg
[2.783 warn src/window.cpp:75] Failed to load SVG /home/cschol/src/Rack-1.0-Home/plugins-v1/MockbaModular/m̃

As you can see the filename for the panel is completely botched.

The configuration file gets created in the user directory and its content is:

{
  "Background": "res/Empty_light.svg"
}
MockbaTheBorg commented 4 years ago

I just loaded it onto three different computer (running Windows), one of them didn't have VCV initially, and it worked fine on all of them. Tested with different panels and the MockbaModular.json file got created normally. @cschol Do you think it might be related to filenames on different operating systems? Like uppercase versus lowercase? So far I have not found nothing to fix. All seems to work fine.

cschol commented 4 years ago

Yes, I think it is platform related. I am looking into it now.

MockbaTheBorg commented 4 years ago

I would guess it is related to file names. Windows doesn't care, Linux does, Mac probably does too.

MockbaTheBorg commented 4 years ago

I think it might be happening because of the loadback() function, which is just defined on BGCOLOR. It might be returning some stray buffer. I do not have means to test on Linux or Mac though. Is it failing on both?

cschol commented 4 years ago

The question is rather why this is working on Windows.

In loadBack, you are returning a pointer ret to a character array that is allocated on the stack. That array will be invalid when the function returns. At worst this will crash with a segfault.

asset::plugin takes a std::string argument. Why are you working with character arrays here?

MockbaTheBorg commented 4 years ago

Yeah ... this is my suspicion, which I called stray buffer. Somehow Windows insists on working fine. For now a have disabled the feature for Mac and Linux and will push a commit soon. In the meantime I will rewrite the code to use std::string correctly, and a global buffer instead of a stack one.

cschol commented 4 years ago

Let's not risk this working by accident on Windows (or failing on some machines, when built by the plugin manager). Let's wait for this to be correctly implemented on all platforms.

MockbaTheBorg commented 4 years ago

Good point ... totally agree ... I will just take the feature off so we can release 1.1.3, and I get it fixed for 1.1.4.

MockbaTheBorg commented 4 years ago

Please try commit: dccec60ef63ff0e461a4edf2e9665a7f9774bbd5

cschol commented 4 years ago

I just tested by using strings in the function and it works correctly now.

diff --git a/src/MockbaModular.cpp b/src/MockbaModular.cpp
index c777b91..ad326f7 100644
--- a/src/MockbaModular.cpp
+++ b/src/MockbaModular.cpp
@@ -1,11 +1,11 @@
 #include "plugin.hpp"
 #include "MockbaModular.hpp"

-#define BACK "res/Empty_light.svg"
+const std::string BACK("res/Empty_light.svg");

-void saveBack(const char* Back) {
+void saveBack(const std::string& Back) {
        json_t* settingsJ = json_object();
-       json_object_set_new(settingsJ, "Background", json_string(Back));
+    json_object_set_new(settingsJ, "Background", json_string(Back.c_str()));
        std::string settingsFilename = asset::user("MockbaModular.json");
        FILE* file = fopen(settingsFilename.c_str(), "w");
        if (file) {
@@ -15,8 +15,8 @@ void saveBack(const char* Back) {
        json_decref(settingsJ);
 }

-const char* loadBack() {
-       const char* ret = BACK;
+std::string loadBack() {
+    std::string ret;
        std::string settingsFilename = asset::user("MockbaModular.json");
        FILE* file = fopen(settingsFilename.c_str(), "r");
        if (!file) {
@@ -42,4 +42,4 @@ const char* loadBack() {
        fclose(file);
        json_decref(settingsJ);
        return ret;
-}
\ No newline at end of file
+}
diff --git a/src/MockbaModular.hpp b/src/MockbaModular.hpp
index a3235e0..3719b30 100644
--- a/src/MockbaModular.hpp
+++ b/src/MockbaModular.hpp
@@ -3,7 +3,7 @@

 #define BGCOLOR loadBack()

-void saveBack(const char* Back);
-const char* loadBack();
+void saveBack(const std::string& Back);
+std::string loadBack();

-#endif
\ No newline at end of file
+#endif
diff --git a/src/UDPClockSlave.cpp b/src/UDPClockSlave.cpp
index 2303c6d..7836afc 100644
--- a/src/UDPClockSlave.cpp
+++ b/src/UDPClockSlave.cpp
@@ -167,4 +167,4 @@ struct UDPClockSlaveWidget : ModuleWidget {
        }
 };

-Model* modelUDPClockSlave = createModel<UDPClockSlave, UDPClockSlaveWidget>("UDPClockSlave");
\ No newline at end of file
+Model* modelUDPClockSlave = createModel<UDPClockSlave, UDPClockSlaveWidget>("UDPClockSlave");
MockbaTheBorg commented 4 years ago

Even better ... I will apply the fix ... thank you so much. Do I need to reupload 1.1.3 with the fix? Or are you able to use as is?

cschol commented 4 years ago

You need to commit the fix to your repo so I can pull it in.

MockbaTheBorg commented 4 years ago

Cool ... I will apply and upload it.

MockbaTheBorg commented 4 years ago

@cschol Commit cd639cf163344fb809c8e8fd367a5ccd3c68ebd8 has the fix.

cschol commented 4 years ago

Works. Thanks.