Anything that implements java.io.Serializeable should be a valid value for a global variable
gerzytet opened this issue ยท 6 comments
We will need more design discussions about this issue.
First of all, allowing Serializable as Global Variable value posses number of problems:
- Because the Bukkit API's ConfigurationSerializable doesn't support the data stream, the best way would serialize the data into String and vice versa.
- This is problematic because once we serialize the data into String, we no longer have a method to identify that the input is either raw String or a serialized object.
- Perhaps one possible solution is to discard the raw String and treat both raw String and serialized object as String, so there would be no ambiguity. The caveat of this method will be that the users will be unable to modify the var.yml directly just for simple String (technically they can, but it may corrupt the serialized data if not done it correctly), and serialization through Serializable is known to be heavy performance-wise compared to ConfigurationSerializable (cause it has to count on all the class metadata, not just String).
- Or as an alternative, we may consider implementing custom ConfigurationSerializable which supports various objects that are not currently supported (such as List)
- The problem is that other than the List, ConfigurationSerializable only support List, so we have to convert any other data structure, like Set, into List, yet if we do that, we do not know whether the original data structure was List, Set, or something else after the List is deserialized. Also, it will be quite a lot of work if we have to support each object manually one by one.
- Or to make things super simple and compatible well with the ConfigurationSerializable, just only allow List to be savable as GlobalVariable, then the ConfigurationSerializable will handle it.
@wysohn
you should look into gson
It's a well known library for converting objects to json, even if the objects aren't serializable
My biggest concern is backwards compatibility. Raw strings will probably go away if we store everything as json, so we need a way to handle old var.yml files.
The easiest way I can think of is randomly deciding to rename var.yml into something else, like global.yml, and storing all the variables there. If trg sees var.yml, it will know that it's an old file and convert to to the newer format
@gerzytet
That will be quite painful to migrate everything but will be a lot better since we can combine Bukkit side and Sponge side with just gson. Our work will be much easier in the future using gson.
One thing to note is that the Spigot jar itself has gson in it, so we will have to rename the package or something in order to use the latest gson. They literally never update gson and some latest serialization/deserialization doesn't work in the old gson.
I haven't looked through the Sponge side, but they have pretty solid API to handle lots of cases, so I wouldn't worry about it.
Handled in #167