Skip to content

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

Closed
wants to merge 1 commit into from

Conversation

cg2121
Copy link
Contributor

@cg2121 cg2121 commented Jun 16, 2025

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

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@cg2121 cg2121 added the Bug Fix Non-breaking change which fixes an issue label Jun 16, 2025
@cg2121 cg2121 requested a review from PatTheMav June 16, 2025 20:23
@cg2121 cg2121 force-pushed the modules-backward-search branch from 48043de to 93004f5 Compare June 16, 2025 22:13
@cg2121
Copy link
Contributor Author

cg2121 commented Jun 16, 2025

Updated to change the module load order instead.

@cg2121 cg2121 changed the title libobs: Search through modules backwards frontend: Change order of loading modules Jun 16, 2025
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.
@cg2121 cg2121 force-pushed the modules-backward-search branch from 93004f5 to e1f4521 Compare June 16, 2025 22:58
@WizardCM
Copy link
Member

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.

@WizardCM
Copy link
Member

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.

@PatTheMav
Copy link
Member

PatTheMav commented Jun 23, 2025

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.

@gxalpha
Copy link
Member

gxalpha commented Jun 24, 2025

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?

@Fenrirthviti
Copy link
Member

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.

@cg2121
Copy link
Contributor Author

cg2121 commented Jun 24, 2025

Closing, in favor of #12311

@cg2121 cg2121 closed this Jun 24, 2025
@cg2121 cg2121 deleted the modules-backward-search branch June 24, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants