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

Move Steve's Addons ASM to explicit compat #14

Merged
merged 9 commits into from
Jan 20, 2024

Conversation

ah-OOG-ah
Copy link
Member

@ah-OOG-ah ah-OOG-ah commented Jan 15, 2024

Move Steve's Addons ASM into code directly, with isModLoaded checks. Requires this PR in Steve's Addons.
Additionally, minor license cleanup: the tweet screenshot is now directly in the repository and the license is in text as well.
Also, moves to Tags.java instead of token replacement in a file.

This doesn't load in nightly 310, and I have no idea why. Also, needs testing by people who use SFM.

@Dream-Master
Copy link
Member

@ah-OOG-ah on mod need the other code to load that is the problem. I am not sure how to setup this?

@ah-OOG-ah
Copy link
Member Author

Merged into dev - seems to work now.

@ah-OOG-ah ah-OOG-ah closed this Jan 17, 2024
@Dream-Master Dream-Master reopened this Jan 17, 2024
@ah-OOG-ah ah-OOG-ah marked this pull request as ready for review January 18, 2024 01:35
@ah-OOG-ah
Copy link
Member Author

Should build and not break anything now.

@Dream-Master Dream-Master requested a review from a team January 18, 2024 09:32
@OneEyeMaker
Copy link

Instead of calling addon's methods from base mod (and so create circular dependency), it's better to create clear API that can be implemented/overridden in addon.
Also, don't use Loader.isModLoaded every time you want to call method from another mod: call it once and store result.

@ah-OOG-ah
Copy link
Member Author

It's not really an API, it's specifically for Steve's Addons. However, it doesn't require Steve's Addons to compile, so no circular dep.

@Dream-Master Dream-Master merged commit 44f2ab9 into GTNewHorizons:master Jan 20, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants