Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Load ASI recursive in scripts #140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

sndth
Copy link

@sndth sndth commented Feb 19, 2025

My PR introduces the ability to load ASI plugins recursively from the scripts/plugins/update directories and adds the option to enable this feature in the global.ini file (it is disabled by default). This allows for better organization of the plugin folder, but only when the LoadFromScriptsRecursive variable is set to 1.

Simple presentation:

I want to load several plugins that belong to a modification called MySuperMod, so I create a MySuperMod folder in scripts/plugins/update and then place several ASI plugins there. This way, I know that in case of inconsistent plugin names, we can easily find them.

It doesn't matter whether LoadFromScriptsRecursive is set to 0 or 1, the plugins located in scripts/plugins will still be loaded, which is obvious.


SetCurrentDirectoryW(szSelfPath);

if (SetCurrentDirectoryW((i.path().wstring() += L"\\").c_str()))
Copy link
Contributor

Choose a reason for hiding this comment

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

The final character before the null character must be a backslash (''). If you don't specify the backslash, then it will be added for you. Therefore, specify >MAX_PATH-2 characters for the path unless you include the trailing backslash; in which case, specify MAX_PATH-1 characters for the path.

This concatenation is then not necessary.

Copy link
Author

Choose a reason for hiding this comment

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

Indeed! I was confused by the original input, such as scripts\\, being passed as an argument to SetCurrentDirectoryW.

@ThirteenAG
Copy link
Owner

Don't forget to add error handling and symlinks like here:

std::error_code ec;
constexpr auto perms = std::filesystem::directory_options::skip_permission_denied | std::filesystem::directory_options::follow_directory_symlink;
for (const auto& it : std::filesystem::recursive_directory_iterator(path, perms, ec))

Also, if this doesn't break anything, perhaps better to leave it enabled by default.
And maybe rename options to something like

LoadFromFromRoot=1
LoadRecursively=1

If `nWantsToLoadFromScriptsOnly` was not set, ASI plugins in the root directory were not loaded.
@sndth
Copy link
Author

sndth commented Feb 20, 2025

Don't forget to add error handling and symlinks like here:

std::error_code ec;
constexpr auto perms = std::filesystem::directory_options::skip_permission_denied | std::filesystem::directory_options::follow_directory_symlink;
for (const auto& it : std::filesystem::recursive_directory_iterator(path, perms, ec))

Also, if this doesn't break anything, perhaps better to leave it enabled by default. And maybe rename options to something like

LoadFromFromRoot=1
LoadRecursively=1

I made changes that fulfill the requests! I'm not entirely sure what the LoadFromFromRoot option refers to, but I think it's almost everything now.

@ThirteenAG
Copy link
Owner

I just meant as a replacement for LoadFromScriptsOnly, though probably not worth to do it right now, as I'm not sure what the backwards compatibility impact might be.

@sndth
Copy link
Author

sndth commented Feb 20, 2025

So that's probably it!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants