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

GH-307 Implement Fabric platform support #376

Merged
merged 31 commits into from
Feb 17, 2024
Merged

GH-307 Implement Fabric platform support #376

merged 31 commits into from
Feb 17, 2024

Conversation

huanmeng-qwq
Copy link
Contributor

Closes #307

@huanmeng-qwq
Copy link
Contributor Author

Temporary draft status, but I am not sure if there is still a need to add or modify content

@Rollczi
Copy link
Owner

Rollczi commented Feb 14, 2024

Hey @huanmeng-qwq nice to see another PR grom you.
I am afraid that translations of brigadier will not work properly in some cases e.g. when I will use a multilevel argument etc.

@Rollczi
Copy link
Owner

Rollczi commented Feb 14, 2024

Also I have problem with this issue: #349

I think we can resolve this as in the minestom platform (one full string argument represents all arguments)

@huanmeng-qwq
Copy link
Contributor Author

Also I have problem with this issue: #349

I think we can resolve this as in the minestom platform (one full string argument represents all arguments)

It does look good, let me give it a try.

Also, it seems that Fabric does not support directly introducing sub projects as mod dependencies, which has led to the failure of CI

@Rollczi
Copy link
Owner

Rollczi commented Feb 14, 2024

Also, it seems that Fabric does not support directly introducing sub projects as mod dependencies, which has led to the failure of CI

I think it is an issue with JDK version. I have tried to index your project with JDK 17 and it also failed.
image

On JDK 8 it works fine. (CI)
image

@huanmeng-qwq
Copy link
Contributor Author

huanmeng-qwq commented Feb 14, 2024

I think it is an issue with JDK version. I have tried to index your project with JDK 17 and it also failed.

The reason why the problem only occurs on jdk17 is because the project requires 17 to run, so it was ignored in jdk8

@Rollczi
Copy link
Owner

Rollczi commented Feb 14, 2024

I think it is an issue with JDK version. I have tried to index your project with JDK 17 and it also failed.

The reason why the problem only occurs on jdk17 is because the project requires 17 to run, so it was ignored in jdk8

Ah okay, you are right

@huanmeng-qwq
Copy link
Contributor Author

It seems that my writing style is incorrect. I obtained the correct way through reporting issues

@huanmeng-qwq
Copy link
Contributor Author

It should be possible now

@Rollczi
Copy link
Owner

Rollczi commented Feb 14, 2024

It seems that my writing style is incorrect.

Don't worry, I will review your code :P

huanmeng-qwq and others added 3 commits February 14, 2024 19:30
…leFabric.java

Co-authored-by: Norbert Dejlich <ndejlich5@gmail.com>
Co-authored-by: Norbert Dejlich <ndejlich5@gmail.com>
@huanmeng-qwq huanmeng-qwq marked this pull request as ready for review February 15, 2024 08:13
@huanmeng-qwq
Copy link
Contributor Author

require assets/litecommands/icon.png
@Rollczi

@huanmeng-qwq
Copy link
Contributor Author

require assets/litecommands/icon.png @Rollczi

How can I test the icon?

exampes/fabric/run/mods: ModMenu mod examples:fabric:runClient

I mean how to check icon

ModMenu supports you to view the icons of installed mods in the game

@huanmeng-qwq
Copy link
Contributor Author

also work when I publish the litecommands to maven local.

implementation(project(path = ":litecommands-fabric", configuration = "namedElements")) It directly references local sub projects, supports debugging hot modifications, and can ensure that the project code is the latest

@Rollczi
Copy link
Owner

Rollczi commented Feb 15, 2024

also work when I publish the litecommands to maven local.

implementation(project(path = ":litecommands-fabric", configuration = "namedElements")) It directly references local sub projects, supports debugging hot modifications, and can ensure that the project code is the latest

Yes you arę right.

But I compared
modImplementation("dev.rollczi:litecommands-fabric:3.3.4+1.20.4")
with
modImplementation(include("dev.rollczi:litecommands-fabric:3.3.4+1.20.4")!!)

@huanmeng-qwq
Copy link
Contributor Author

But I compared modImplementation("dev.rollczi:litecommands-fabric:3.3.4+1.20.4") with modImplementation(include("dev.rollczi:litecommands-fabric:3.3.4+1.20.4")!!)

include means that the dependency will be packaged into the META-INF/jars directory of the jar

@Rollczi
Copy link
Owner

Rollczi commented Feb 15, 2024

Okay, I think all is done. But last issue. LiteCommands should be displayed in the mod menu?. (Currently it is only displayed example mod)
image

@huanmeng-qwq
Copy link
Contributor Author

Okay, I think all is done. But last issue. LiteCommands should be displayed in the mod menu?. (Currently it is only displayed example mod)

Because of this paragraph, it will not be displayed by default

    "custom": {
        "modmenu": {
            "badges": [
                "library"
            ]
        }
    }

open:
image
image

@Rollczi
Copy link
Owner

Rollczi commented Feb 15, 2024

Okay, I think all is done. But last issue. LiteCommands should be displayed in the mod menu?. (Currently it is only displayed example mod)

Because of this paragraph, it will not be displayed by default

thanks <3

@Rollczi
Copy link
Owner

Rollczi commented Feb 15, 2024

@huanmeng-qwq I added a few changes. Please check if they suit you and everything is okay.

@huanmeng-qwq
Copy link
Contributor Author

Okay, it can be merged

@Rollczi Rollczi changed the title Implement Fabric platform support GH-307 Implement Fabric platform support Feb 17, 2024
@Rollczi Rollczi merged commit e353cdf into Rollczi:master Feb 17, 2024
6 checks passed
@Rollczi
Copy link
Owner

Rollczi commented Feb 17, 2024

Thank you again ❤️

@creatorfromhell
Copy link

Looks awesome, can't wait to test.

@Rollczi
Copy link
Owner

Rollczi commented Feb 17, 2024

Looks awesome, can't wait to test.

@creatorfromhell You can checkout new snapshot 3.4.0-SNAPSHOT in the repository with snapshots ;P

maven("https://repo.eternalcode.pl/snapshots")

@creatorfromhell
Copy link

Looks awesome, can't wait to test.

@creatorfromhell You can checkout new snapshot 3.4.0-SNAPSHOT in the repository with snapshots ;P

maven("https://repo.eternalcode.pl/snapshots")

Cheers. The project I need it for I still have to port over from the previous lib I used xD

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.

Support Fabric Platform
3 participants