Removed unused (de)serialization methodes #147
Conversation
removed java.io.Serializable interface from classes removed serialization method from classes (or changed to `toString` if the method was used) removed deserialization method from classes (or changed to `fromString` if the method was used)
|
Were these changes tested? Removing serializable and not replacing it with something else is a significant change and needs to be reasonably tested to make sure that the data output isn't changing/breaking shops that previously existed. |
|
Yes i have ran it on a new paper server. The config and Data is both saved and loaded properly. I already mentioned in issue #145 that if you want custom JSON de(serialization) you'll have to implement the |
Update this with 2.6 since I had to make an adapter for itemstacks. As long as it works without the serializable extensions then I'm fine with those being removed. Changing (de)serializable methods feels unnecessary and even if they aren't being used by gson they roll in and out of usage for debugging data output. toString should focus more on a readable format for debugging(with an exception for smaller objects that are easily readable in their serialized form in which case it can redirect to that) I didn't make any of these changes myself to avoid stealing credit for your work/testing. |
|
i wont be able to update it today, ill probably have it finished tomorrow. I might consider making proper Serialization classes if that something you're interested in |
That's fine, it's not looking like I'll get a release out before the end of the week anyways. I already added on for Configuration Serializable. I don't think any of our stuff needs it since the normal gson handles it pretty well. If you really want to it's fine as long as the output and input don't change, I just don't see any benefits that would justify the work needed for it. |
|
Merge Conflicts have been resolved. Tested on 1.19.2. Maybe do another test yourself, but i would say you're ready for release with 1.19 support |
There was a problem hiding this comment.
@SparklingComet Can you look this over too and let me know if you don't agree.
| public static ShopItemStack deserialize(String serialized) { | ||
| ShopItemStack item = new GsonProcessor().fromJson(serialized, ShopItemStack.class); | ||
| item.loadLegacyData(); | ||
| item.buildMap(); | ||
| return item; | ||
| } | ||
|
|
||
| public String serialize() { | ||
| return new GsonProcessor().toJson(this); | ||
| } | ||
|
|
There was a problem hiding this comment.
Removing Serializable is good if it's not being used, but I see no point in removing the (de)serialize methods since at worst they just waste space. They do occasionally get used when I need to add debug to a section. In most cases when I do that they have to be removed soon after for performance like in the Hopper code(where no debug can stay since it runs so often with each hopper)
| public static PlayerSetting deserialize(String serialized) { | ||
| PlayerSetting playerSetting = new GsonProcessor().fromJson(serialized, PlayerSetting.class); | ||
| playerSetting.load(); | ||
| return playerSetting; | ||
| } | ||
|
|
| public String serialize() { | ||
| return new GsonProcessor().toJson(this); | ||
| } | ||
|
|
| public static ShopUser deserialize(String serialized) { | ||
| ShopUser shopUser = new GsonProcessor().fromJson(serialized, ShopUser.class); | ||
| shopUser.player = Bukkit.getOfflinePlayer(UUID.fromString(shopUser.playerUUID)); | ||
| return shopUser; | ||
| } | ||
|
|
| public String serialize() { | ||
| return new GsonProcessor().toJson(this); | ||
| } | ||
|
|
| * Deserializes the object to Json using Gson | ||
| * | ||
| * @param serialized Shop GSON to be deserialized | ||
| * | ||
| * @return Shop object from file | ||
| */ | ||
| public static Shop deserialize(String serialized) { | ||
| Shop shop = new GsonProcessor().fromJson(serialized, Shop.class); | ||
| shop.fixAfterLoad(); | ||
|
|
||
| return shop; | ||
| } | ||
|
|
||
| /** |
| /** | ||
| * Serializes the object to Json using Gson | ||
| * | ||
| * @return serialized string | ||
| */ | ||
| public String serialize() { | ||
| return new GsonProcessor().toJson(this); | ||
| } | ||
|
|
| public static ShopType deserialize(String serialized) { | ||
| return new GsonProcessor().fromJson(serialized, ShopType.class); | ||
| } | ||
|
|
| public String serialize() { | ||
| return new GsonProcessor().toJson(this); | ||
| } | ||
|
|
Pull Request Etiquette
There are several guidelines you should follow in order for your Pull Request to be merged.
Replace
[ ]with[x]to cross the checkbox.Changes
Removed unused (de)serialization methodes
toStringif the method was used)fromStringif the method was used)