Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add unload and load commands for plugins #836

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add unload and load commands for plugins #836

wants to merge 7 commits into from

Conversation

catornot
Copy link
Member

@catornot catornot commented Jan 4, 2025

this pr adds concommands to reload and unload plugins so that they be more easily reloaded on windows. windows will prevent the dll from being modified if it's loaded so it needs to be unload then loaded :P

also this already has #835

@catornot catornot added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code depends on another PR Blocked until the PR it depends on is merged labels Jan 4, 2025
@catornot catornot removed the depends on another PR Blocked until the PR it depends on is merged label Jan 21, 2025
Alystrasz
Alystrasz reviewed Feb 1, 2025
View reviewed changes
primedev/plugins/pluginmanager.cpp
if (args.ArgC() >= 2 && !strcmp(name.c_str(), args.Arg(1)))
continue;

if (plugin.Unload())
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be wise to print if unloading fails, wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah it be wise to add a log call there

@Alystrasz
Copy link
Contributor

Alystrasz commented Feb 1, 2025 •
edited
Loading

@catornot can you provide me with a protocol to test your change?
Describing what currently does not work, but should work with this PR?

@catornot
Copy link
Member Author

catornot commented Feb 2, 2025 •
edited
Loading

why

rn everything should just work if you try using the reload_plugins command

this pr adds the ability to unload(unload_plugins) then load(load_plugins) individual plugins since reloading all the plugins is sometimes buggy (for like no apparent reason and since it's a dev feature, I don't think it's a big issue)

reloading plugins isn't really useful on windows since it's too fast to replace the plugins and replacing them before reloading is not possible on windows which is why manual unload and loading is required.

how

  • get a plugin that be actually reloaded (ex: octbots or recanim)
  • start game
  • unload it with unload_plugins <plugin name>
  • load it load_plugins
  • profit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alystrasz Alystrasz Alystrasz left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
needs code review Changes from PR still need to be reviewed in code needs testing Changes from the PR still need to be tested
Projects
Northstar Roadmap
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@catornot @Alystrasz