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

Support Spigot/Paper 1.21.3 and 1.21.4 #4164

Open
wants to merge 4 commits into
base: v3.0
Choose a base branch
from
Open

Conversation

jacob1
Copy link

@jacob1 jacob1 commented Nov 10, 2024

This PR adds support for 1.21.3 (bundles of bravery update) and 1.21.4 (The Garden Awakens). It's my first time doing a plugin update like this, but I tested it on my server and it seems to be working. For 1.21.4, it just gets it running, it doesn't support rendering the new pale garden / wood blocks.

The biggest difference in this compared to base 1.21 is I removed async chunk loading on paper (#3725). Paper did some massive overhauls of chunk loading on their end (PaperMC/Paper@730882fca9cf , PaperMC/Paper@7bd22b1835af). I think they did this prior to 1.21, but it wasn't caught, because it falls back to sync chunk loading if the classes aren't there. I only noticed because Mojang did their own big refactors to chunk loading in 1.21.2.
It would be worth keeping that ... but unless I had a lot of guidance I think that's way outside what I'm capable of redoing. It was already broken since 1.21 anyway.

I checked the mappings docs to get the right mappings. I found a few mistakes in the older helper classes, mostly inconsequential. I went back and fixed those too.

Tested on Spigot 1.21.4, Spigot 1.21.3, Paper 1.21.3, and Spigot 1.21. Let me know of any changes / additionals / removals you want.

Jar Download

Edit: Lots of people are asking for jars. If you just want a download, I compiled it and posted it here. This is the new one with 1.21.4 support / nether ceiling fix.

Leaves fix was applied to 1.21 as well. getMaterial() was removed in 1.20 so has been removed from that version onwards as well.
@Brokkonaut
Copy link

Brokkonaut commented Nov 10, 2024

You modified the helpers for old versions (for example replace ((NBTTagByte)val).h() with ((NBTTagByte)val).i()), was that intended?

@jacob1
Copy link
Author

jacob1 commented Nov 10, 2024

Yes. When I looked it up, .h() was getAsShort, and .i() was getAsByte. That method is supposed to get a byte, not a short, so I fixed it and also all the other versions it affected.

That one is definitely inconsequential though, so I can remove it if it causes too much noise. It's not the only change I made to old versions though.

@Brokkonaut
Copy link

I think it is good that you fixed it, I was just asking because that was not explicitly mentioned in the commit msg or pr.

@jacob1
Copy link
Author

jacob1 commented Nov 10, 2024

Yeah, I didn't mention that one, only the other two. As for the other two:

Fixed DynmapBlockState.isLeaves. In 1.21 it was using RAILS instead of LEAVES. Seems to be used for some lighting calculations.

Removed DynmapBlockState.setMaterial call in 1.20 onwards. The function it was using to get the material was removed. It's been using getSoundType() since then (obviously incorrect). As best I could tell this might be used in some connected texture pack code, but I couldn't tell if the material is really needed.

@SilviodaSilva1972
Copy link

What is missing to release the corrected version?

@jacob1
Copy link
Author

jacob1 commented Nov 13, 2024

It's done from my perspective, it just needs a maintainer to merge it.

You can compile it yourself using the guide in the readme. I'm using it on my "live" server now and haven't run into any issues.
(I could post my .jar it but better to wait for an official build)

@LariOesch
Copy link

Mind dropping the .jar file? I tried compiling it but build keeps failing.

@Tsoccerguy3
Copy link

Dynmap-3.7-beta-7_1.21.3_spigot.jar.zip
Here is the compiled jar for 1.21.3 , Tested and working

@SilviodaSilva1972
Copy link

Thank you very much, grateful!

@ThomasGITH
Copy link

Thanks a lot

@Selunie
Copy link

Selunie commented Nov 15, 2024

It works for me too. Thank you ! ❤️

@mblaze
Copy link

mblaze commented Nov 16, 2024

@LariOesch the problem with the build is that it is looking for org.spigotmc:spigot:1.21.3-R0.1-SNAPSHOT but it won't be able to find it. Usually this dependency is hosted in https://repo.mikeprimm.com/org/spigotmc/spigot/ but since it is maintained by the author of the plugin (assuming from the name), it is not yet there. Fortunately Gradle looks for dependencies in many places, including your local repository. This means that it should be enough to build Spigot for 1.21.3 with BuildTools (instructions here) to get the missing dependency in your local Maven repository. At least this is what made it work for me. I'd strongly advise against downloading JARs from unofficial sources.

@SomethingGeneric
Copy link

@LariOesch the problem with the build is that it is looking for org.spigotmc:spigot:1.21.3-R0.1-SNAPSHOT but it won't be able to find it. Usually this dependency is hosted in https://repo.mikeprimm.com/org/spigotmc/spigot/ but since it is maintained by the author of the plugin (assuming from the name), it is not yet there. Fortunately Gradle looks for dependencies in many places, including your local repository. This means that it should be enough to build Spigot for 1.21.3 with BuildTools (instructions here) to get the missing dependency in your local Maven repository. At least this is what made it work for me. I'd strongly advise against downloading JARs from unofficial sources.

thanks for the tip about BuildTools fixing the dependency error. Worked for me as well!

This was referenced Nov 24, 2024
@masmc05
Copy link
Contributor

masmc05 commented Nov 25, 2024

You can easily make it now to always support async chunk loading

For chunk loading just use CompletableFuture<NBTTagCompound> nbt = cw.getHandle().m().a.d(new ChunkCoordIntPair(chunk.x, chunk.z)).thenApply(op -> op.orElse(null)); instead of the provider's completable future.
For chunk saving just make getLoadedChunk as a method of MapChunkCache121_3, replace there getAsyncSaveData.invoke(null, world.getHandle(), c) with SerializableChunkData.a(world.getHandle(), c) and save.invoke(null, world.getHandle(), c, o) with o.a() (rename the o variable). Also remove the if (!Bukkit.isPrimaryThread()) return null; as it's not needed anymore

After that don't forget to change isUnsafeAsync to always false. Idk how to deal with sync methods, but you most probably can just throw UnsupportedOperationException as they'll become unused

@jacob1
Copy link
Author

jacob1 commented Nov 26, 2024

@masmc05 Thanks for the info. I sort of see where you mean. I don't have the time to try it out for a while though. If I ever have time to test and get it working I may submit another PR.

If you're willing you could also send in a PR, or just a branch.

Will that work on Spigot too? If so, that would be great. It's best not to rely on Paper's internal api anyway like the old code did.

@shou692199
Copy link

shou692199 commented Nov 26, 2024

@jacob1
Seems like nether roof rendering is broken after this PR.
error log
https://pastebin.com/FSYF8nFH

custom-perspectives.txt
https://pastebin.com/RrZ1nqKK

worlds.txt
https://pastebin.com/BrHaZnaW

@Teerack
Copy link

Teerack commented Nov 27, 2024

This didn't work for me. Here is my log https://i.imgur.com/dbj6FRi.png

@jnvb
Copy link

jnvb commented Nov 28, 2024

Dynmap-3.7-beta-7-fabric-1.21.3.zip

Sharing my compiled version of fabric 1.21.3
I can PR if you guys want.

DynmapWorld.worldheight is reporting as 255 in 1.21.3, instead of 256 like it did before. (Overworld is now 319 -> 320). Unsure if this is a bug, since it seems right ...

Either way, do a Math.ceil to ensure there's enough sections even when the amount of blocks doesn't evenly line up with a 16 block section.
@jacob1
Copy link
Author

jacob1 commented Nov 29, 2024

@jacob1 Seems like nether roof rendering is broken after this PR. error log https://pastebin.com/FSYF8nFH

custom-perspectives.txt https://pastebin.com/RrZ1nqKK

worlds.txt https://pastebin.com/BrHaZnaW

@shou692199 Seems like it's broken in 1.21.3, yeah. The nether's max world height is being reported as 255 now instead of 256. Unsure if this is a spigot bug or just an upstream Minecraft change. I think 255 is more accurate ... ? You can build the 0th block but not the 256th.

Anyway, I made a fix. Here's a new jar if you want it. Keep the max height as 256 in custom-perspectives.txt (or else the top block won't render).
Edit: Updated this jar to remove the debug print.

This didn't work for me. Here is my log https://i.imgur.com/dbj6FRi.png

Looks like it loaded fine to me. You can ignore the all.png error. I get that one too. It's a separate bug from before, unrelated to this PR.

@Brokkonaut
Copy link

Brokkonaut commented Nov 30, 2024

@shou692199 Seems like it's broken in 1.21.3, yeah. The nether's max world height is being reported as 255 now instead of 256. Unsure if this is a spigot bug or just an upstream Minecraft change. I think 255 is more accurate ... ? You can build the 0th block but not the 256th.

Anyway, I made a fix. Here's a new jar if you want it. Keep the max height as 256 in custom-perspectives.txt (or else the top block won't render).

At least in paper this will probably be changed back to 256 soon: PaperMC/Paper#11675

But I think your solution is good to have it working in every implementation

@jacob1
Copy link
Author

jacob1 commented Nov 30, 2024

Thanks for the links. That seems like a confusing disaster to have the api differ in Spigot and Paper. I see though, the api clearly defines what should happen. I'll report it upstream to Spigot and see if it can get fixed there, or at least change the docs.

I didn't test in fabric. There is a 1.21.3 fabric PR now, which I bet is also affected. My fix should apply there too in theory.

@jacob1
Copy link
Author

jacob1 commented Nov 30, 2024

Reported to Spigot - https://hub.spigotmc.org/jira/browse/SPIGOT-7970
Edit: fixed now in Spigot

I tried checking fabric, but the PR doesn't compile because of all the chunk loading refactors Mojang made.

(ability to run only, not the ability to render any of the new pale garden blocks)
@jacob1 jacob1 changed the title Support Spigot/Paper 1.21.3 Support Spigot/Paper 1.21.3 and 1.21.4 Dec 4, 2024
@jacob1
Copy link
Author

jacob1 commented Dec 4, 2024

I added support for Spigot 1.21.4

There were more nms mapping changes, so I created a new copy of the helper classes. (TileEntity.getBlockPos changed, and so did a bunch more NBT methods).

This doesn't add support for rendering the pale garden. Only letting dynmap run. If you try to render the new biome, everything just shows up black. No plans to try to add support right now. That would be a big time commitment to understand the texture_1 / models_1 format and then add all the new blocks.

@luismayme
Copy link

I'm really noob to ask this, but I have never cloned a PR. How can I compile this?

@jacob1
Copy link
Author

jacob1 commented Dec 5, 2024

I'm really noob to ask this, but I have never cloned a PR. How can I compile this?

You can clone my fork with git clone https://github.com/jacob1/dynmap.git.

From there, just follow the build steps in the readme

Or you can just download the jar in the first post, I put it there too :)

@CosminPerRam
Copy link

Hey, are there any blockers on this PR? Looking forward to an official update.

@DivineJimmi
Copy link

The update works, but pale oak is not rendering correctly. Textures are missing.
Screenshot 2024-12-10 131732
Screenshot 2024-12-10 131725

@jacob1
Copy link
Author

jacob1 commented Dec 10, 2024

Yeah, all the new blocks are missing. I won't have more free time until the end of the month to check that. On my server I'm just dealing with it, it's annoying but at least it runs.

I wouldn't consider it a blocker to merging this PR, only a blocker to making a new release.

@DavidDoesCode
Copy link

Hi @jacob1 I used your latest commit and added the textures. I got them from the 1.21.4 minecraft jar. There were other textures missing, but I didn't want to bloat up the commit with unneeded textures.

https://github.com/DavidDoesCode/dynmap/tree/jacob1/v3.0

@DavidDoesCode
Copy link

Yes. When I looked it up, .h() was getAsShort, and .i() was getAsByte. That method is supposed to get a byte, not a short, so I fixed it and also all the other versions it affected.

That one is definitely inconsequential though, so I can remove it if it causes too much noise. It's not the only change I made to old versions though.

I am not sure this is correct. You modified OLD version mappings.
https://mappings.dev/1.18.1/net/minecraft/nbt/ByteTag.html
https://mappings.dev/1.21.3/net/minecraft/nbt/ByteTag.html

So if this is to be "compatible" with 1.21.3, it doesn't make sense to do.

@DivineJimmi
Copy link

Hi @jacob1 I used your latest commit and added the textures. I got them from the 1.21.4 minecraft jar. There were other textures missing, but I didn't want to bloat up the commit with unneeded textures.

https://github.com/DavidDoesCode/dynmap/tree/jacob1/v3.0

Can you upload a fixed .jar to test this?

@jacob1
Copy link
Author

jacob1 commented Dec 11, 2024

Hi @jacob1 I used your latest commit and added the textures. I got them from the 1.21.4 minecraft jar. There were other textures missing, but I didn't want to bloat up the commit with unneeded textures.

https://github.com/DavidDoesCode/dynmap/tree/jacob1/v3.0

Cool, that's a good start. textures_1.txt and models_1.txt also need to be modified to make them show up properly, I think.

For your other comment, this is compatible with all versions of 1.21. I have separate mappings for 1.21.3 and 1.21.4. I fixed the older mappings which were using the wrong functions. I used that site and validated that everything I changed is pointing to the right one. So 1.18.1 still uses .h(), 1.21.3 uses .i().

@DavidDoesCode
Copy link

DavidDoesCode commented Dec 11, 2024

@jacob1 - I'm struggling with gradle builds but hopefully I can get passed it soon, test, and share a jar. Thanks for your note on textures_1 and models_1 I will have a look.

@DorianMAG
Copy link

@jacob1
Thx for this work !
Tested with spigot 1.21.4, plugin start now

@buddythugginz
Copy link

thankyou so much for this!

@Viech
Copy link

Viech commented Dec 19, 2024

Compiled this today for spigot 1.21.4 from the source branch of this PR: https://dl.viech.name/Dynmap-3.7-beta-7-spigot.jar

% md5sum Dynmap-3.7-beta-7-spigot.jar
63333b7868d677c862af042b9af58f7b  Dynmap-3.7-beta-7-spigot.jar

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.