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

AddonModule#canLoad #7548

Open
wants to merge 5 commits into
base: dev/feature
Choose a base branch
from

Conversation

TheAbsolutionism
Copy link
Contributor

Description

This PR aims to add a default method #canLoad(SkriptAddon) within AddonModule allowing addon developers to specify conditions that need to be met in order for the addon to be loaded.
This ensures an addon is not loaded when it should not or can't.

Also, combines the two for loops of init and load into one loop because it simplifies it and there's no need to loop it twice.


Target Minecraft Versions: any
Requirements: none
Related Issues: #7539

Copy link
Contributor

@Fusezion Fusezion left a comment

Choose a reason for hiding this comment

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

Very nice, much needed, good words, #7541 also completed in this

@APickledWalrus
Copy link
Member

Also, combines the two for loops of init and load into one loop because it simplifies it and there's no need to loop it twice.

I'm against this simply because, if we desire to change this, it should be replaced with a most robust load order system. I do not think two loops is a major issue given it provides a basic load order system

@TheAbsolutionism
Copy link
Contributor Author

I'm against this simply because, if we desire to change this, it should be replaced with a most robust load order system. I do not think two loops is a major issue given it provides a basic load order system

So you're just against the combining the two loops? If so, I can just undo that part alone, but I would believe a canLoad would still need to be called within that loop as well.

@APickledWalrus
Copy link
Member

APickledWalrus commented Jan 29, 2025

So you're just against the combining the two loops? If so, I can just undo that part alone, but I would believe a canLoad would still need to be called within that loop as well.

Yeah, the canLoad is fine. I would filter a list of the addon modules that can load, and use that for the two loops

@TheAbsolutionism
Copy link
Contributor Author

Yeah, the canLoad is fine. I would filter a list of the addon modules that can load, and use that for the two loops

Okie dokie, all done

@Efnilite Efnilite added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jan 29, 2025
Copy link
Member

@cheeezburga cheeezburga left a comment

Choose a reason for hiding this comment

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

What do we think about shouldLoad instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants