search

LambdAurora / AurorasDecorations

Decorations-focused Quilt mod.
https://lambdaurora.dev/AurorasDecorations
GNU Lesser General Public License v3.0
50 stars 15 forks source link

Update to 1.18 #15

Closed binomial0 closed 2 years ago

binomial0 commented 2 years ago

Also update fabric api, mappings, java to 17 and gradle (because java 17 needs newer gradle).

This is my first contribution to this mod, so please let me know if you notice something I should do differently.

Most changes are quite mechanical. I don't fully understand the architecture for syncing block entities, but I get most of it, my changes make sense and I tested that they work. There may be possible code simplifications in this area.

I think my PR can only target existing branches, so this PR targets 1.17. This should ideally change before merging, but I either don't know how to do that or lack the required permissions.

Thank you for making this awesome mod, and also for your help with creating this PR!

binomial0 commented 2 years ago

CI failed because I forgot to add a license header to the two files I created. So I'm thinking about the license question again. I think I've got to the opinion that those 4 lines fall under fair use. (Noncommercial, not a substantial portion of FabricApi, AurorasDecorations does not compete with FabricApi). So I will just put a GPL header on it (which I think I'm allowed to anyway because Apache and GPL are compatible in this direction).

LambdAurora commented 2 years ago

Honestly just run the task updateLicenses, should be fine.

binomial0 commented 2 years ago

Thank you! I have classes tomorrow, so I have to go to bed. I will fix the server stuff tomorrow.

binomial0 commented 2 years ago

(duplicated from discord, sorry) I'm confused. Are sleeping bags supposed to break faster if you mine them with shears? Because the code looks like this might be the case, but it doesn't actually work in 1.17. The 1.17 code uses a method (breakByTool) that is deprecated in 1.18. So I changed it, and now it actually breaks faster with shears. I don't know if it's actually supposed to be that way.

binomial0 commented 2 years ago

Before you merge, please create a 1.18 branch so I can point the PR to that (it's still toward the 1.17 branch atm).

binomial0 commented 2 years ago

I'm at a point where I'm happy with the code. I've tested the current implementation of the readNbt method on a dedicated server (using gradlew runServer), and it definitely works, has the intended behavior and doesn't print any errors. I don't have enough background knowledge to be very surprised by this. What to do about the changes in AuroraUtil depends on your preference. I still like the current solution best, and I tried to make the explanation more clear. If you want, I can do it differently. When you have no further concerns, I consider this ready to merge.