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

remove usage of fabric internals #145

Closed
wants to merge 2 commits into from
Closed

Conversation

moehreag
Copy link

@moehreag moehreag commented Feb 5, 2023

This change allows LFAPI to be used with Quilt loader on 1.8.9 (and possibly other versions), and increases stability across updates since Fabric's internals should not be considered a stable API.

Technical changes:
Replaced the usage of Fabric's LogCategory with calls to Log4j directly. This should not compromise compatibility since on all versions this API is targeting Log4j is used instead of slf4j.

@thecatcore
Copy link
Member

The thing is, we plan to expand this api to earlier versions of Minecraft, which included neither slf4j and log4j.
The changes to logger in FabricLoader was asked by us to Player to help handling this case, which he kindly implemented.

@moehreag
Copy link
Author

moehreag commented Feb 5, 2023

The thing is, we plan to expand this api to earlier versions of Minecraft, which included neither slf4j and log4j. The changes to logger in FabricLoader was asked by us to Player to help handling this case, which he kindly implemented.

Oh okay, will find a different solution to this then.

@moehreag moehreag closed this Feb 5, 2023
@Wyvest
Copy link
Contributor

Wyvest commented Feb 5, 2023

maybe use the upstream method as a fallback to the PR?

@moehreag
Copy link
Author

moehreag commented Feb 5, 2023

maybe use the upstream method as a fallback to the PR?

(afaik) There is no upstream solution, but making a small mod just containing these few necessary classes from fabric loader works as well.

@Wyvest
Copy link
Contributor

Wyvest commented Feb 5, 2023

maybe use the upstream method as a fallback to the PR?

(afaik) There is no upstream solution, but making a small mod just containing these few necessary classes from fabric loader works as well.

wait no I meant like use the changes in this PR if the one that currently is in legacy fabric API doesn't work

@moehreag
Copy link
Author

moehreag commented Feb 5, 2023

maybe use the upstream method as a fallback to the PR?

(afaik) There is no upstream solution, but making a small mod just containing these few necessary classes from fabric loader works as well.

wait no I meant like use the changes in this PR if the one that currently is in legacy fabric API doesn't work

oh, well that's up to you and the other maintainers/contributors to decide.

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.

None yet

3 participants