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 documentation for addClientSideModDependency and add new modIsInstalled function with more sensible naming and semantics #51

Merged
merged 4 commits into from Dec 3, 2022

Conversation

atampy25
Copy link
Contributor

@atampy25 atampy25 commented Dec 3, 2022

The addClientSideModDependency function used to do what it says; it would check if the mod was installed and abort the plugin loading if it wasn't. At some point, the semantics changed to supposedly "returning whether the mod is available", but even that has been broken for a long time, as it actually returns whether the mod is UNavailable. This has had a knock-on effect to the overrideFrameworkChecks flag, which is meant to make it so that checks always succeed; however, because a check succeeding means returning false in this broken function, the flag being set actually makes all plugins with mod dependencies abort initialisation.

This PR deprecates the old function and updates its documentation to describe the actual behaviour of the function. It also introduces a new function with a more sensible name that describes its more sensible semantics.

@atampy25 atampy25 added the plugins Related to plugins. label Dec 3, 2022
@atampy25 atampy25 requested a review from RDIL December 3, 2022 03:27
components/controller.ts Outdated Show resolved Hide resolved
components/controller.ts Outdated Show resolved Hide resolved
atampy25 and others added 2 commits December 3, 2022 13:30
Co-authored-by: Reece Dunham <me@rdil.rocks>
Signed-off-by: atampy25 <24306974+atampy25@users.noreply.github.com>
Co-authored-by: Reece Dunham <me@rdil.rocks>
Signed-off-by: atampy25 <24306974+atampy25@users.noreply.github.com>
Copy link
Member

@RDIL RDIL left a comment

Choose a reason for hiding this comment

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

LGTM

@RDIL RDIL merged commit 9969168 into thepeacockproject:v5 Dec 3, 2022
RDIL added a commit that referenced this pull request Dec 3, 2022
…Installed` function with more sensible naming and semantics (#51)

* Fix documentation and add new function

Signed-off-by: atampy25 <24306974+atampy25@users.noreply.github.com>
Co-authored-by: Reece Dunham <me@rdil.rocks>
scrungofan pushed a commit to scrungofan/Peacock that referenced this pull request Dec 6, 2022
…Installed` function with more sensible naming and semantics (thepeacockproject#51)

* Fix documentation and add new function

Signed-off-by: atampy25 <24306974+atampy25@users.noreply.github.com>
Co-authored-by: Reece Dunham <me@rdil.rocks>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins Related to plugins.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants