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.

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

Fix reloading plugins #835

Merged
merged 2 commits into from
Jan 18, 2025
Merged

Fix reloading plugins #835

merged 2 commits into from
Jan 18, 2025

Conversation

catornot
Copy link
Member

@catornot catornot commented Dec 31, 2024

Originally it probably worked if all plugins could be reloaded but it also just dropped all the plugins regardless of whether it can be reloaded or not.

So this pr attempts to amend that by individually reloading all the plugins instead of dropping then loading them all.

(this still doesn't really fix it since I can't get any plugin to be reloaded 💢 so that's why it's a draft)

was (wow crash)
image

now (wow still doesn't work!)
image

so basically I need some help but I think this pr is good first stepping stone. :)

ignore the screenshots before this

how to test

with this pr if upon reloading plugins nothing crashes then that means it worked as intended.

@catornot catornot requested a review from uniboi December 31, 2024 23:47
@github-actions github-actions bot 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 labels Dec 31, 2024
@catornot catornot marked this pull request as ready for review January 2, 2025 19:40
@catornot
Copy link
Member Author

catornot commented Jan 2, 2025

Seemingly it was only an issue of discord rpc not reloading, every other plugins works so this pr can be reviewed now :D

Copy link
Contributor

@uniboi uniboi left a comment

Choose a reason for hiding this comment

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

Makes sense to reload plugins in reverse load order. Should tbh the same for unloading as well

@uniboi uniboi added READY TO MERGE This mergeable right now and removed needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Jan 14, 2025
@GeckoEidechse
Copy link
Member

not sure if this was actually tested or not

@GeckoEidechse GeckoEidechse merged commit f98f6b9 into R2Northstar:main Jan 18, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
READY TO MERGE This mergeable right now
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants