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 issues with shaded dependencies #62

Open
wants to merge 1 commit into
base: fabric/1.20
Choose a base branch
from

Conversation

solonovamax
Copy link

  • Rely on fabric language kotlin to provide kotlinx.serialization
  • Relocate all shaded dependencies into subpackage

Fixes #61.

Similar to #59.

- Rely on fabric language kotlin to provide kotlinx.serialization
- Relocate all shaded dependencies into subpackage

Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
@fengshuo2004
Copy link

By the look of things, this PR would also fix my issue #65 .
@jenchanws Please have a look. Thank you very much!

@solonovamax
Copy link
Author

Hi, @fengshuo2004, this PR is only for the fabric 1.20 branch, as that was the version I was using. However, applying the same to the other branches should be quite easy, and I can do so if you'd like, as welll as providing instructions for how to compile it yourself so you can fix the issue.

However, mohist is not exactly the best and has many issues, so I also recommend giving this and this a read. Mohist can often be the cause for issues encountered, however I don't believe it is the issue here. But, often mohist is the cause of the issue.

@fengshuo2004
Copy link

Hi, @fengshuo2004, this PR is only for the fabric 1.20 branch, as that was the version I was using. However, applying the same to the other branches should be quite easy, and I can do so if you'd like, as welll as providing instructions for how to compile it yourself so you can fix the issue.

However, mohist is not exactly the best and has many issues, so I also recommend giving this and this a read. Mohist can often be the cause for issues encountered, however I don't believe it is the issue here. But, often mohist is the cause of the issue.

@solonovamax Thanks for the reply. I am not using Mohist, I'm using Magma.

I'll clone your repo and make the same changes to the Forge branch. I have some Java dev experience so should figure this stuff out no problem. I'll let you know if it fixes my issue.

@solonovamax
Copy link
Author

the same thing that was said about mohist applies to magma.

@fengshuo2004
Copy link

Just coming back to say that after making my own build, this does indeed fix #65 .
(Obviously the removal of the shadowDep line is not needed for Forge build since the line doesn't exist there anyway)

My opinion is, a patch is always nice to have as long as the patch doesn't break something else. But if Jen isn't willing to accept it, I'll just report the issue to Magma instead, no big deal.

In any case, thanks for the PR @solonovamax !

@solonovamax
Copy link
Author

My opinion is, a patch is always nice to have as long as the patch doesn't break something else. But if Jen isn't willing to accept it, I'll just report the issue to Magma instead, no big deal.

in this particular instance, it is not an issue with magma

however, magma can cause many other issues, so it's basically the first thing you should check for issues.

lietblue added a commit to lietblue/create-track-map that referenced this pull request May 10, 2024
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.

Crash when interacting with other mods that use kotlinx.serialization because classes are not relocated
2 participants