Config Creation causes issues

[BoyStijn]

Expected behavior

The config creates itself and gets loaded without issue

Observed/Actual behavior

some pre-comments aren't properly commented out

Steps/models to reproduce

run the latest dev version

Plugin list

TradeShop EssentialsX

TradeShop version and Minecraft version (CraftBukkit/Spigot/Paper/...)

1.19.2 TradeShop-2.6.0-DEV-2022-11-25T03.51.47

Agreements

• [X] I have searched for and ensured there isn't already an open issue regarding this.
• [ ] My version of Minecraft and TradeShop are supported by the TradeShop Dev Team (when in doubt, first ask on the Discord).

Other

adding a space after \n in the language file fixes the issue

[BoyStijn]

After fixing the config the following error appeared when trying to finalize a shop

java.lang.NullPointerException: Cannot invoke "org.bukkit.inventory.ItemStack.getType()" because the return value of "org.shanerx.tradeshop.item.ShopItemStack.getItemStack()" is null at org.shanerx.tradeshop.shop.Shop.lambda$fixSide$9(Shop.java:971) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?] at java.util.ArrayList.forEach(ArrayList.java:1511) ~[?:?] at org.shanerx.tradeshop.shop.Shop.fixSide(Shop.java:971) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?] at org.shanerx.tradeshop.shop.Shop.fixAfterLoad(Shop.java:300) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?] at org.shanerx.tradeshop.data.storage.Json.JsonShopConfiguration.load(JsonShopConfiguration.java:92) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?] at org.shanerx.tradeshop.data.storage.DataStorage.loadShopFromSign(DataStorage.java:58) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?] at org.shanerx.tradeshop.shop.Shop.loadShop(Shop.java:162) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?] at org.shanerx.tradeshop.shop.ShopChest.getShop(ShopChest.java:316) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?] at org.shanerx.tradeshop.shop.listeners.ShopProtectionListener.onBlockBreak(ShopProtectionListener.java:239) ~[TradeShop-2.6.0-DEV-2022-11-25T03.51.47.jar:?]

[BoyStijn]

above issue can be fixed by adding unloadData(); on all ShopItemStack changes and creation (ItemStack wasn't being serialized)

[BoyStijn]

Also userEditableKey contains a captital E, but the methods getMapped... use .toLowerCase() so it can never retrieve if a value is user editable

[KillerOfPie]

Can you send me the config file with errors so I can try to find why those comments aren't appearing. Adding here or send on discord is fine.

[KillerOfPie]

above issue can be fixed by adding unloadData(); on all ShopItemStack changes and creation (ItemStack wasn't being serialized)

I think I fixed this, added unload before serializing s it should always run wen it's needed.

[BoyStijn]

Can you send me the config file with errors so I can try to find why those comments aren't appearing. Adding here or send on discord is fine.

No longer have the broken config but: global-options: chest-directions: pre-comment: "Directions an allowed shop can be from a sign. Multiple directions can be chained together allowing all 26 blocks surrounding a block to be checked. \nTo Chain directions just add a '+' between each one. i.e 'Back+Left' or 'Back+Left+Up' \nAllowed directions are:\n Up, Down, Left, Right, Back, Front"

Theres is no space after the \n like in the other pre-comments. this for some reason causes the parser to forget to add a # at the start of a new line

[KillerOfPie]

Maybe fixed, changed userEditable to user-editable

[KillerOfPie]

above issue can be fixed by adding unloadData(); on all ShopItemStack changes and creation (ItemStack wasn't being serialized)

I think I fixed this, added unload before serializing s it should always run wen it's needed.

Maybe you did locally but its not in the public build

itll be up once im done with all the fixes

[BoyStijn]

above issue can be fixed by adding unloadData(); on all ShopItemStack changes and creation (ItemStack wasn't being serialized)

I think I fixed this, added unload before serializing s it should always run wen it's needed.

Maybe you did locally but its not in the public build

itll be up once im done with all the fixes

misread that. My bad

[KillerOfPie]

Got them all updated, going to change the jar on the last dev release. You'll probably need to regenerate the config files. If there re still uncommented lines then send me the file before you make changes to it.

[KillerOfPie]

updated https://github.com/Tradeshop/TradeShop/releases/tag/TradeShop-2.6.0-DEV-2022-11-25T19.56.03

[BoyStijn]

serialize() never gets called so it still doesn't serialize itemstacks

[BoyStijn]

adding it here: public ShopItemStack(ItemStack itemStack, HashMap<ShopItemStackSettingKeys, ObjectHolder<?>> itemSettings) { this.itemStack = itemStack; this.itemSettings = itemSettings; buildMap(); } Seems to work

[BoyStijn]

you also missed the userEditableKey in ShopSettingKeys

[KillerOfPie]

serialize() never gets called so it still doesn't serialize itemstacks

Serialize should be called by gson when this object is stored in another object being serialized. @SparklingComet Correct me if im wrong on that.

adding it here: public ShopItemStack(ItemStack itemStack, HashMap<ShopItemStackSettingKeys, ObjectHolder<?>> itemSettings) { this.itemStack = itemStack; this.itemSettings = itemSettings; buildMap(); } Seems to work

Not sure what you mean by that seeming to work?

Edit: Never mind I see what you're saying. gonna look again

you also missed the userEditableKey in ShopSettingKeys

I had them both open too....fixed now

[BoyStijn]

IntelliJ cant find any usage and the item doesn't get serialized

Not sure what you mean by that seeming to work?

i mean putting unloadData() at the end of the constructor serializes the item properly

[BoyStijn]

public static class Serializer implements JsonDeserializer<MutableComponent>, JsonSerializer<Component> { This is how minecraft does custom serialization

[KillerOfPie]

IntelliJ cant find any usage and the item doesn't get serialized

Not sure what you mean by that seeming to work?

i mean putting unloadData() at the end of the constructor serializes the item properly

Yea that's normal. We don't reference serialize, its something that GSON calls on objects it's serializing.

It does but it's unnecessary until the object is getting stored, making it a waste of resource(shouldn't be a noticeable amount so im going to do it anyways, but not my preferred method of doing it).

public static class Serializer implements JsonDeserializer<MutableComponent>, JsonSerializer<Component> { This is how minecraft does custom serialization

We don't use minecraft serialization in any cases outside of debugging and ItemStacks(starting in 2.6). We use GSON for serializing and deserializing any data we store as json.

[KillerOfPie]

dev release updated

[BoyStijn]

We don't use minecraft serialization in any cases outside of debugging and ItemStacks(starting in 2.6). We use GSON for serializing and deserializing any data we store as json.

Minecraft also uses GSON. Unless GSON uses reflection to get serialization and deserialization you would have to implement the interfaces

other website also show it when making custom serialization

[BoyStijn]

Anyway all seems to work now. Keep up the good work, the plugin is amazing

[KillerOfPie]

We don't use minecraft serialization in any cases outside of debugging and ItemStacks(starting in 2.6). We use GSON for serializing and deserializing any data we store as json.

Minecraft also uses GSON. Unless GSON uses reflection to get serialization and deserialization you would have to implement the interfaces

other website also show it when making custom serialization

Doesn't matter what Minecraft uses, we Serialize directly through GSON and not through Minecrafts interfaces.

Anyway all seems to work now. Keep up the good work, the plugin is amazing

Let me know if you find anything, want to release the beta asap.

[BoyStijn]

Doesn't matter what Minecraft uses, we Serialize directly through GSON and not through Minecrafts interfaces.

so does minecraft JsonDeserializer<T> and JsonSerializer<T> are from com.google.gson

[KillerOfPie]

JsonDeserializer and JsonSerializer may be from GSON but we don't use them. We use Serializable from java.io to mark objects and basic GSON serialization rather than making custom serializers.

Minecraft serialization is pretty inconsistent. Using ItemStack as an example, serializing the object(using its serialize method) returns a map rather than a directly readable output like JSON meaning it needs to be serialized/deserialized twice unpack(renamed unload since it isn't unloading but unpacking the ItemStack) and trying to directly (de)serialize it through GSON didn't work. Not sure why the unpack in serialize doesn't work since I know the deserialize method does.

I guess I could look at making a run through of this stuff again while we also make the changes for sql since most of the code that handles this stuff has been copied around since we started with it a couple years ago... Tis a problem for later though, as long as it works and is consistent it doesn't matter how it gets there.

Good finds and you are free to make your own pull requests if you want to fix something. Bigger changes may need discussions so we know how they work and affect other code but you're still welcome to do so.

[BoyStijn]

Minecraft serialization is pretty inconsistent. Using ItemStack as an example, serializing the object(using its serialize method) returns a map rather than a directly readable output like JSON meaning it needs to be serialized/deserialized twice unpack(renamed unload since it isn't unloading but unpacking the ItemStack) and trying to directly (de)serialize it through GSON didn't work. Not sure why the unpack in serialize doesn't work since I know the deserialize method does.

This is because thats not minecraft serialization, but rather bukkits one. Minecraft stores most data as NBT and has custom (de)serializers for that. JSON is mainly used for parsing datapacks and chat messages

[BoyStijn]

Just to test my theory i removed the java.io.Serializable interface as well as the (de)serialization methods from the Shop and ShopItemStack class. The saved shop data that resulted from this { "l::world::-12.0::87.0::-16.0": { "owner": { "player": "554fadce-4a58-42e7-91e9-f6bf2a02a546", "role": "OWNER" }, "managers": [], "members": [], "shopType": "TRADE", "shopLoc": { "div": "::", "worldName": "world", "x": -12.0, "y": 87.0, "z": -16.0 }, "product": [ { "itemStackB64": null, "serialItemStack": { "v": 3120.0, "type": "OAK_SIGN" }, "itemSettings": { "COMPARE_UNBREAKABLE": { "value": true }, "COMPARE_SHULKER_INVENTORY": { "value": true }, "COMPARE_DURABILITY": { "value": 1.0 }, "COMPARE_NAME": { "value": true }, "COMPARE_ATTRIBUTE_MODIFIER": { "value": true }, "COMPARE_BOOK_AUTHOR": { "value": true }, "COMPARE_ENCHANTMENTS": { "value": true }, "COMPARE_ITEM_FLAGS": { "value": true }, "COMPARE_BOOK_PAGES": { "value": true }, "COMPARE_BUNDLE_INVENTORY": { "value": true }, "COMPARE_FIREWORK_DURATION": { "value": true }, "COMPARE_LORE": { "value": true }, "COMPARE_FIREWORK_EFFECTS": { "value": true }, "COMPARE_CUSTOM_MODEL_DATA": { "value": true } } } ], "cost": [ { "itemStackB64": null, "serialItemStack": { "v": 3120.0, "type": "CHEST" }, "itemSettings": { "COMPARE_UNBREAKABLE": { "value": true }, "COMPARE_SHULKER_INVENTORY": { "value": true }, "COMPARE_DURABILITY": { "value": 1.0 }, "COMPARE_NAME": { "value": true }, "COMPARE_ATTRIBUTE_MODIFIER": { "value": true }, "COMPARE_BOOK_AUTHOR": { "value": true }, "COMPARE_ENCHANTMENTS": { "value": true }, "COMPARE_ITEM_FLAGS": { "value": true }, "COMPARE_BOOK_PAGES": { "value": true }, "COMPARE_BUNDLE_INVENTORY": { "value": true }, "COMPARE_FIREWORK_DURATION": { "value": true }, "COMPARE_LORE": { "value": true }, "COMPARE_FIREWORK_EFFECTS": { "value": true }, "COMPARE_CUSTOM_MODEL_DATA": { "value": true } } } ], "chestLoc": { "div": "::", "worldName": "world", "x": -12.0, "y": 87.0, "z": -15.0 }, "status": "OPEN", "shopSettings": { "NO_COST": { "value": false }, "HOPPER_EXPORT": { "value": false }, "HOPPER_IMPORT": { "value": false } }, "availableTrades": 1 } }

Is properly serialized as you can see. This is due to the fact that java.io.Serializable and the (de)serialization methods aren't used by GSON

[BoyStijn]

Good finds and you are free to make your own pull requests if you want to fix something

made a pull request to cleanup the unused (de)serialization methods Tested it on a clean paper server. Save Data is the same, no errors as well