SpongePowered / SpongeAPI

A Minecraft plugin API
http://www.spongepowered.org/
MIT License
1.14k stars 342 forks source link

"Offline" Location #1932

Open randombyte-developer opened 6 years ago

randombyte-developer commented 6 years ago

Some months ago I stumbled upon this:

A plugin was serializing a Location to the config. The serialized structure holds the Worlds UUID and the position. When deserializing the config, the default Location TypeSerializer built into Sponge tries to construct the location off of the world UUID and the position. An admin had deleted a world which resulted in errors when the config was loaded. The world UUID couldn't be found anymore. The Location wasn't invalid or "offline", it just couldn't be constructed.

I solved it by building my own serializer. Once I mentioned it on the Sponge Discord server it was proposed to create a concept of an "offline Location". Nothing more was discussed, it just came to my mind recently and thought that with https://github.com/SpongePowered/SpongeAPI/pull/1896 it would be a great chance (or even the only one) to introduce such a concept.

I don't have a strong opinion on that. I think the only use-case would be for the config stuff, which could be simply solved by changing the default serializer a bit. I don't have the big picture of the API to think of any other use-cases. Maybe someone else has a good idea about it.

ryantheleach commented 6 years ago

Semi-related: https://github.com/SpongePowered/SpongeAPI/issues/1884

Cybermaxke commented 5 years ago

I added the world UUID to the Location in , and additional constructors/methods, for 1.13 https://github.com/SpongePowered/SpongeAPI/commit/40904f2eeab4e9b42b8e6047f4b754e983eec3b5#diff-6d94dc0de1d7083caa6f3758f6b904e2R89

gabizou commented 5 years ago

@cybermaxke, I had a thought, add a method to Location#isValid. If the world is not valid, return false, better for devs to be able to check instead of the object potentially throwing exceptions later. And basically, it’d be better to maybe even add ifValid(consumer)

dualspiral commented 5 years ago

@gabizou Would an unloaded but valid world be classed as invalid? If so, I’d also suggest isAvailable and ifAvailable. It might be a valid location, just not on a loaded world.

gabizou commented 5 years ago

@gabizou Would an unloaded but valid world be classed as invalid? If so, I’d also suggest isAvailable and ifAvailable. It might be a valid location, just not on a loaded world.

Right, there's a key here that if the world cannot be loaded because it never existed (registered), then the location would be invalid entirely. But, if a world can be loaded, isValid would likely be a new method to verify that.

It reminds me of the subclassing guava used for Optional to where we could have three subclassed Location extensions, one for offline, one for unregistered and one for valid. Thoughts?

ryantheleach commented 5 years ago

I've seen multi-world plugin authors struggle with unloading and reloading worlds in Sponge as well, while on the topic of loaded vs registered.

Not sure of the details, But recall there being problems with world uuids even after unloading.

Is it possible to "unregister" a world? Should it be?

Cybermaxke commented 5 years ago

The only way to "unregister" a world is by removing it.