search

CloudburstMC / Cloudburst

Cloudburst Server is a Bedrock first server software. Originally forked from Cloudburst Nukkit.
178 stars 70 forks source link

Rename level -> world #108

Closed lukeeey closed 3 years ago

lukeeey commented 3 years ago

I don't expect this to be merged as it breaks almost every plugin but it does make it easier to understand. Plus, with the API being worked on plugins wont need to rely on the internals of Cloudburst Server anyway.

I've renamed all references except in the config.

TheShermanTanker commented 3 years ago

I second this PR, I feel that Worlds would be easier to understand. What's causing the build failures though?

TheShermanTanker commented 3 years ago

@lukeeey I've found the problem:

Results :

Failed tests: 
  ServerPropertiesMappingTest.writingWorks:81 expected: <motd=A Cloudburst Powered Server
sub-motd=https://cloudburstmc.org
server-ip=0.0.0.0
server-port=19132
view-distance=10
white-list=false
achievements=true
announce-player-achievements=true
spawn-protection=16
max-players=20
allow-flight=false
spawn-animals=true
spawn-mobs=true
gamemode=0
force-gamemode=false
hardcore=false
pvp=true
difficulty=1
default-world=world
allow-nether=true
enable-query=true
auto-save=true
force-resources=false
xbox-auth=true
generate-structures=true
> but was: <motd=A Cloudburst Powered Server
sub-motd=https://cloudburstmc.org
server-ip=0.0.0.0
server-port=19132
view-distance=10
white-list=false
achievements=true
announce-player-achievements=true
spawn-protection=16
max-players=20
allow-flight=false
spawn-animals=true
spawn-mobs=true
gamemode=0
force-gamemode=false
hardcore=false
pvp=true
difficulty=1
default-level=world
allow-nether=true
enable-query=true
auto-save=true
force-resources=false
xbox-auth=true
generate-structures=true
>

You've changed the ServerPropertiesMappingTest to check if the server.properties has default-world, but you forgot to change the default server.properties to default-world as well, you left it as default-level instead. Change that and it'll be able to build

TheShermanTanker commented 3 years ago

Just going to query, any progress on this PR so far? @lukeeey

Sleepybear commented 3 years ago

Closing this as it's not going to be merged. BDS code calls them Level and we're going to stay with that.

TheShermanTanker commented 3 years ago

Closing this as it's not going to be merged. BDS code calls them Level and we're going to stay with that.

Wait some of the code from the official Bedrock Server executable is public?

lukeeey commented 3 years ago

@TheShermanTanker No, but the binary ships with symbols so you can see the class and method names

TheShermanTanker commented 3 years ago

Interesting, though I still think this would be a good change. PM has also recently renamed Level to World due to "the misleading nature of the name "level". It's also confusing when trying to do things like getting the XP level of the player or such, and also does not translate well to other languages." https://github.com/pmmp/PocketMine-MP/commit/3cd6e12e718a6d846c1c2e75ae2667a148e2c613

TheShermanTanker commented 3 years ago

I guess all we can do now is just hope this decision changes in the future. Just because the official Mojang implementation does it doesn't mean it's the best way, it's not uncommon for third party implementations to surpass the code Mojang has written (The PaperMC server for Java Edition is a great example). It would be good if this specific patch was cached somewhere in the future