uskyblock / uSkyBlock

The best skyblock bukkit plugin around...
http://dev.bukkit.org/bukkit-plugins/uskyblock/
GNU General Public License v3.0
15 stars 12 forks source link

Update region limits to reflect the new 1.18 world height #36

Closed janchaloupka closed 2 years ago

janchaloupka commented 2 years ago

Some of the players on our server are complaining that they cannot build below Y0 after the new 1.18 update. I briefly looked into the code and found that the minimum and maximum region height values are hard coded to the old world limits.

I propose to change these values to reflect the new world height limits.

(moved to uskyblock/uSkyBlock repo as per request https://github.com/rlf/uSkyBlock/pull/1288#issuecomment-1023485736)

BlackBeltPanda commented 2 years ago

Should probably be using WorldInfo.getMinHeight() and WorldInfo.getMaxHeight() for world height checks.

Muspah commented 2 years ago

Thanks again! Might need to think about a way to update existing regions.

janchaloupka commented 2 years ago

Should probably be using WorldInfo.getMinHeight() and WorldInfo.getMaxHeight() for world height checks.

Thanks for the suggestion. I updated the code and now it should be more future-proof.

Thanks again! Might need to think about a way to update existing regions.

No problem! I propose to create new command (something like /usb wg updateall or /usb wg update all) that will update all island regions.

Of course this would require the admins to run this command. Maybe the plugin could also check its version number on startup and if the config version is older than this version it could run the command automatically.

theleteron commented 2 years ago

It would be a good idea to also check other parts of the code for hardcoded world limits (ex. https://github.com/uskyblock/uSkyBlock/blob/c96543beb006838f2f3b1d220a52402dd05d8f7e/uSkyBlock-Core/src/main/java/us/talabrek/ultimateskyblock/island/level/ChunkSnapshotLevelLogic.java#L80). For example, everything that is built below or above old limits is not counted towards island level or biome change is not applied below and above old limits.

janchaloupka commented 2 years ago

I checked all parts of the source code and (hopefully) fixed every hardcoded world height reference.

What's fixed:

Muspah commented 2 years ago

Thanks very much all, will review and merge when I'm back at the PC with some spare time. Somewhere this week :)

janchaloupka commented 2 years ago

The problem with the failing build is because of the outdated reference to FAWE dependency. I have it fixed in my local repository. I can push it if it's not already fixed in another branch.

Muspah commented 2 years ago

Feel free to push.

Muspah commented 2 years ago

Looks good to me, updating a single region in my test server (/usb wg update) changed the coordinates correctly:

image

Lets merge... Build might fail, there seems to be a new method in OfflinePlayer that we need to expand in our NullPlayer. Will commit afterwards.