Skip to content

Conversation

@myk002
Copy link
Member

@myk002 myk002 commented Jan 14, 2025

and return a proper error code

@myk002 myk002 merged commit b2ee13b into DFHack:develop Jan 14, 2025
14 checks passed
@myk002 myk002 deleted the myk_plug_fail branch January 14, 2025 10:37
else if (unload && !plug_mgr->unloadAll())
ret = CR_FAILURE;
else if (!plug_mgr->reloadAll())
ret = CR_FAILURE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, removing the early return here likely also broke the behavior of -a / -all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. we would also benefit from an else that skips the for loop when -a is specified:

diff --git a/library/Core.cpp b/library/Core.cpp
index 2b02c46fe..6e875648f 100644
--- a/library/Core.cpp
+++ b/library/Core.cpp
@@ -818,9 +818,11 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s
                     ret = CR_FAILURE;
                 else if (unload && !plug_mgr->unloadAll())
                     ret = CR_FAILURE;
-                else if (!plug_mgr->reloadAll())
+                else if (reload && !plug_mgr->reloadAll())
                     ret = CR_FAILURE;
             }
+            else
+            {
                 for (auto p = parts.begin(); p != parts.end(); p++)
                 {
                     if (!p->size() || (*p)[0] == '-')
@@ -832,6 +834,7 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s
                     else if (reload && !plug_mgr->reload(*p))
                         ret = CR_FAILURE;
                 }
+            }
             if (ret != CR_OK)
                 con.printerr("%s failed\n", first.c_str());
             return ret;

@myk002 myk002 mentioned this pull request Apr 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants