-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
frontend: Change order of loading modules #12290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
48043de
to
93004f5
Compare
Updated to change the module load order instead. |
Plugins are now required to be installed in %PROGRAMDATA%. Since its module path is added after the default module paths, the plugins in there will be loaded last. With the load modules once PR (obsproject#12285), the module added in the default path will be the only one loaded. We want the one in %PROGRAMDATA% to be the one loaded instead.
93004f5
to
e1f4521
Compare
Please split the two changes into two commits, one that moves the code from OBSBasic to OBSApp, and one that changes the order of loading. |
My biggest concern with changing the load order without any additional protections, is that it'd allow core modules to be overridden, with no way for the user to know or be able to fix it. I don't think this is the right solution. |
I agree with @WizardCM that this change has quite a few known (and possibly unknown) side-effects that might be a nightmare to handle for support volunteers. I'd like to think that not loading a module from the "new" default location if a module of the same name exists in the "old" location (as enabled by #12285) is the better of the bad choices. We have far too many popups already, so this one hurts, but... maybe we need to somehow inform a user that there were some plugins that weren't loaded because duplicates were found and then it's up to the user to remove outdated/old versions from the internal plugins directory (as the log message is bound to be drowned out by all the other log messages). An automatic migration is out of the question because the directory structure differs between both, so it's the least bad option to let users resolve this before OBS stops loading third party modules from the internal location. |
We know from the safe mode check which of the modules are first-party and which aren’t, could that help? E.g., first load all first-party modules from the old location, then everything from the new location, and lastly the third-party from the old location? |
After discussion today, while I appreciate this is an approach, I don't think it's the right one. What we should be doing is starting to enforce core modules loaded by explicitly loading those first, check by hash, from the OBS Studio program directory. Eventually, we will not be loading anything else from the program directory, but I feel that this kind of enforcement of core modules by hash to validate they have not been tampered/replaced is a better approach than just trying to blanket load things and hope it all works out. Twitch's TEB beta builds has plugin module hash functions, we should take a look at that to save some work on our end. IMO, if we're going to fix module loading, let's do it right rather than just different with as many problems as the current methods. |
Closing, in favor of #12311 |
Description
Plugins are now required to be installed in %PROGRAMDATA%. Since its module path is added after the default module paths, the plugins in there will be loaded last. With the load modules once PR (#12285), the module added in the default path will be the only one loaded. We want the one in %PROGRAMDATA% to be the one loaded instead.
Motivation and Context
To be used with #12285
How Has This Been Tested?
Loaded OBS and made sure the plugins in %PROGRAMDATA%, were the ones loaded.
Types of changes
Checklist: