-
Notifications
You must be signed in to change notification settings - Fork 426
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 #4164
base: v3.0
Are you sure you want to change the base?
Conversation
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.
You modified the helpers for old versions (for example replace ((NBTTagByte)val).h() with ((NBTTagByte)val).i()), was that intended? |
Yes. When I looked it up, .h() was 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 think it is good that you fixed it, I was just asking because that was not explicitly mentioned in the commit msg or pr. |
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. |
What is missing to release the corrected version? |
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. |
Mind dropping the .jar file? I tried compiling it but build keeps failing. |
Dynmap-3.7-beta-7_1.21.3_spigot.jar.zip |
Thank you very much, grateful! |
Thanks a lot |
It works for me too. Thank you ! ❤️ |
@LariOesch the problem with the build is that it is looking for |
thanks for the tip about BuildTools fixing the dependency error. Worked for me as well! |
You can easily make it now to always support async chunk loading For chunk loading just use After that don't forget to change |
@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. |
@jacob1 custom-perspectives.txt worlds.txt |
This didn't work for me. Here is my log https://i.imgur.com/dbj6FRi.png |
Dynmap-3.7-beta-7-fabric-1.21.3.zip Sharing my compiled version of fabric 1.21.3 |
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.
@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).
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. |
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 |
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. |
Reported to Spigot - https://hub.spigotmc.org/jira/browse/SPIGOT-7970 I tried checking fabric, but the PR doesn't compile because of all the chunk loading refactors Mojang made. |
This PR adds support for 1.21.3 (bundles of bravery update). It's my first time doing a plugin update like this, but I tested it on my server and it seems to be working.
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.3, Paper 1.21.3, and Spigot 1.21. Let me know of any changes / additionals / removals you want.