Qveshn / LightAPI

Bukkit Library for create invisible light source
Other
29 stars 12 forks source link

Support for 1.17 #28

Closed LOOHP closed 3 years ago

LOOHP commented 3 years ago

This PR adds support for 1.17.

Here are the changes:

As those are some rather large changes, the way I did it might not be the way maintainers wanted, so changes are welcome and this can just be used as a reference.

ErythroCraft commented 3 years ago

Have you got an test version?

Link here ...

Qveshn commented 3 years ago

Hi, Great job! You shocked me! 😃

Performed superficially the first test for 1.17 and code review. Overall, it works great at first glance.

Found one minor bug: light sources can also be set in blocks of one section below the world and one above the world. Accordingly, the data of the chunk section should also be able to be updated on the client side.

    @Override
    public boolean isValidSectionY(World world, int sectionY) {
        //return sectionY >= (world.getMinHeight() >> 4) && sectionY < (world.getMaxHeight() >> 4);
        return sectionY >= (world.getMinHeight() >> 4) - 1 && sectionY <= (world.getMaxHeight() >> 4);
    }

Here are details for 1.17: https://wiki.vg/Protocol#Update_Light (Two extra sections for lights started with version 1.14)

Probably, I will also need to add a similar y-validation to createLight/deleteLight/collectChunks to prevent unnecessary actions and possible errors. And I will also need to add some public functions like isValidY or isValidLocation or getMinLightHeight/getMaxLightHeight to LightAPI itself.

I'll test it for a couple of days. Then I will take PR and also add corrections and additions.

Thank you so much for the work done!

Qveshn commented 3 years ago

PS.

I will also think about what to do with BitSet. I don't know if it's a good idea to move the whole project to java 1.8. Spigot/bukkit 1.8-1.11 has been compiled by java 1.6. So they mean working under java 1.6. On the other hand, according to statistics, almost no one starts servers 1.8-1.11 under Java 1.6/1.7. https://bstats.org/global/bukkit#javaVersion

Maybe implement custom BitSet analog, and do the conversion into 1.8 BitSet already in module 1.17 or something like that?

Also, I noticed that the maximum height of the world at 1.17 according to the wiki is 384 blocks (from -64 to 319). And two more additional sections (32 blocks). The total is 416 blocks or 26 sections (26 bits). If the maximum height of the world is not planned to increase in future versions, then int is enough to store the mask. It seems to me that operations with the int variable will be faster than with the BitSet class. It will be necessary to reflect on this...

LOOHP commented 3 years ago

The maximum height of the world in 1.17 is 256, it is planned to increase to -64 - 319 in 1.18. But this is just the default. Through datapacks, you are able to extend the world much higher/lower. During the snapshots, I remember playing with the settings and creating a world that is 1024 blocks from bottom to top. I'm unsure if you can go beyond that, but it's clear that the world height is something that Mojang wants to be customizable. So an int (or even a long) might not be enough.

Also, the Java 8 switch is also due to Java 16 unable to compile with

<maven.compiler.source>1.6<maven.compiler.source>
<maven.compiler.target>1.6<maven.compiler.target>

but without Java 16 you can't compile with 1.17 NMS classes. It'll have a class version problem. Pretty much every active plugin right now at least requires Java 8, I think switching should be fine.

BeYkeRYkt commented 3 years ago

Drop Java 1.6 and old NMS versions (<1.12.2). According to statistics (bStats), the old versions are in the minority, so it does not make sense to support them.

Qveshn commented 3 years ago

Found another little bug 😃 All previous server versions produce error in ru/beykerykt/lightapi/server/nms/NmsHandlerBase.java sectionsMaskSky.toLongArray()[0] should be replaced with something like sectionsMaskSky.isEmpty() ? 0 : (int) sectionsMaskSky.toLongArray()[0] And also for sectionsMaskBlock I will fix it myself

Qveshn commented 3 years ago

@BeYkeRYkt @LOOHP Who knows what "Trust Edges" mean in update light packet? https://wiki.vg/Protocol#Update_Light

Trust Edges Boolean If edges should be trusted for light updates.

What does it mean exactly when is true and when is false? (This is the last parameter in PacketPlayOutLightUpdate since 1.16.1) I always use "true" but don't know which is right.

LOOHP commented 3 years ago

I'm not really sure, on my other project I also had to deal with light update packets, I used true as well. I guess it has something to do with light on chunk borders, but again, I'm not really sure. I guess it'll be fine leaving it on true.

Qveshn commented 3 years ago

I think I understood what "Trusted Edges" means 😃 I have decompiled the client and looked at the code. IMHO: If set to true, the client should trust the incoming NibbleArray (array with light data) in the sense that all light spreading was correctly calculated there. If false, the client should recalculate light spreading by himself (NibbleArray contains only light sources without spreading). Bukkit, spigot, paper and LightAPI always recalculate light spreading on the server side, therefore client should trust NibbleArray and we can send Trusted Edges = true

Edges - these are the edges of the block and not chunk (adjacent blocks around the block with light)

LOOHP commented 3 years ago

Ah, so essentially it's just whether the array only contain sources.

Qveshn commented 3 years ago

@LOOHP, what's the best way to add your name to the MIT license, LOOHP or/and another name/email?

LOOHP commented 3 years ago

I guess this will do LOOHP <jamesloohp@gmail.com>

Qveshn commented 3 years ago

License is updated. Almost everything is ready for a new release. I found some bugs and fixed them. I hope there won't be others. ATM the version should be fully functional. All changes are committed. Now I am going on vacation, and tonight I will publish a new release (maybe I will make a couple of minor changes). If you wish, you can test the last commit for now.

Qveshn commented 3 years ago

Hi Release is ready https://github.com/Qveshn/LightAPI/releases/tag/3.5.0 https://www.spigotmc.org/resources/lightapi-fork.48247/ If everything is ok, then the issue can be closed.

LOOHP commented 3 years ago

New release seems to be working great so far, works well if existing code from my plugin. Thanks!