pablo67340 / GUIShop

A Virtual item shop for your Minecraft server!
GNU General Public License v3.0
41 stars 60 forks source link

An End to All the Madness #58

Closed A248 closed 3 years ago

A248 commented 4 years ago

Without a doubt the vast majority of GUIShop's work is parsing items from the config, including various details such as enchantments, spawner types, lore display, etc. Currently the logic can be summed up as follows:

  1. Have a configuration section for each detail of the item
  2. Parse the configuration and add the attributes to the item.
  3. Display the item in the shop, give the item to the player, whatever.

However, this system is problematic. Item attributes which aren't hard-coded into GUIShop can't be added to the item. Potion types, for example, are not currently implemented. There's no way I know of to add unbreakability to tools. Not to mention custom NBT support, custom item qualities, and any sort of upcoming Minecraft updates which might add more features.

Furthermore, since GUIShop looks up the sell price of an item by its ID, multiple differing items of the same ID, but different data values or other attributes, such as enchanted books and potions, can't be sold separately. This could be solved by implementing a more robust lookup strategy. Nevertheless, ultimately the problem lies in the fact that GUIShop has hard-coded these attributes into its sell GUI.

That GUIShop takes this approach also makes configuration a hassle. A mistake means the shops.yml won't load an item. While the error is mitigated by printing an error to the server console, it still makes adding items take a considerable amount of time. Creator mode helps server owners in this regard, but it is its own monstrosity of which still relies on the same configuration framework for parsing items and their myriad qualities.

There is a way to avoid all of this. I wondered if it would be possible to serialise a raw item on server stop and load the exact item after restart. GUIShop would no longer need to store so many strings corresponding to item material or mob type, lists relating to buy and sell lore, or arrays representing enchantments. Server owners would place the item into an editor GUI, similarly to creator, but much improved in its style and backend framework.

I took a look at PlayerVaultsX, a premium plugin open-sourced under the GNU GPL v3. I use it on my own server since I am more than happy to compile from source. The original, freely-distributed PlayerVaults was reportedly riddled with many of the errors regarding saving and loading item qualities that GUIShop faces. However, the premium PlayerVaultsX implements a new data saving system by encoding the contents of an Inventory in base64 and saving it to disk. It is reported that this system saves all sorts of item qualities, including custom NBT or anything which Mojang could imagine to dream up. PlayerVaultsX's serialisation does not hard-code every individual item aspect; rather, it relies on BukkitObjectOutputStream and BukkitObjectInputStream to do all of the work. More info here

GUIShop could adapt a similar technique for saving and loading all of the qualities of an item. This would greatly improve its reliability and consistency and vanquish all of the hassles regarding item parsing which are borne on developers and server owners.

What I propose with regards to GUIShop 8.0 (or GUIShop 2 if you wish) is a directory structure representing the entire buy GUI. Specifically, it would have a sub-directory for each subshop. This way, players could also create unlimited, nested subshops how so ever they choose. The structure would be as follows:

plugins/GUIShop/shops:
  blocks/:
    # ...
  raiding/:
    # ...
  more/:
    misc/:
      # ...
    tools/:
      info.yml
      item1/:
        # ...
      item2/:
        info.yml
        item.gshop

In this filesystem structure each directory represents a menu, submenu, shop, or item, depending on the contents. info.yml would be a readable yml file which denotes the type of object repesented by the directory. To guarantee items do not overlap in a shop, their directory is named by their slot – meaning the item in the first slot would be "item1". The item.gshop files would contain the results obtained by the described itemstack serialisation technique.

Moreover, since each shop would be independent of another, GUIShop could maximise efficiency by asynchronously loading shops and their respective items. Essentially, GUIShop would load all of the shops and items into memory as it currently does.

A huge ConcurrentHashMap (concurrent because it would be loaded async) would store the sell prices of each items so that the sell GUI can easily determine the lookup of each item. This map would not be keyed by "item strings" or any sort of string representation of an item. It would be keyed by itemstacks themselves.

The hardest part of this idea would not be loading or saving shops and items. By far the most difficult task would be the creation of an in-game GUI for server administrators to add items. Regarding saving the contents of the in-game editor, GUIShop would of course save files asynchronously for performance and use a lock to prevent concurrency issues.

pablo67340 commented 4 years ago

I think this honestly would making the saving and loading incredibly difficult for both GUIShopPlus and the creator mode. In terms of adding Item Unreliability, the Unbreaking enchantment? Would it not be easier to implement new configuration section keys as new item features rollout or features get requested?

Similar to mobType for spawners, potionType for potions (Using PostionEffectType enum)

Custom NBT could easily be a list of objects that will be iterated/applied to the item's NBT upon purchase

https://pastebin.com/NJnKv8w1

This would adapt into the current loading system, with only have 2 configuration files and a whack more item support. In terms of async loading, we could still implement some async features across the current loading system to take weight off the main thread.

One thing I am debating is if we should drop support for pre 1.9. Most of the bullshittery workarounds in this plugin are designed to maximize compatibilty across all versions, frankly, I am getting tired of needing to make a version with with 99 other Minecraft versions, and I am seeing more developers pursue the same. With Mojang releasing new updates frequently now, I think it would probably be best to offer 1.9+ support only, and gut the rest. Even if you feel 1.9 is too low, I'd even be willing to go to 1.10 or 1.12 (but I feel it might be too new for some server owners).

Let me know what you think, and again, I appreciate all the work you've done for this plugin Immensely. Looking forward to hearing that you think.

A248 commented 4 years ago

I do not think it would be as hard as you made it out to be. There are a lot of flatfiles, yes, but that's similar to how there are a lot of entries in the shops.yml curerntly; also, it's intended that users will use the editor instead. By the way, I didn't mean to make it seem like async loading was so overrated, that was my fault.

Now that I think of it, this is probably something which would be better for me to experiment with / test out on the side. When I have more free time after I finish exams in May, I will try this out and see what comes of it, and let you know how it went.

Regarding legacy support, it may come as a surprise to you that I myself run 1.8.8 on my own server. I do so regretfully; I would very much like to upgrade to an updated version, but there are a few reasons which hold me back. As a result, I often find myself forking other projects/plugins which have dropped 1.8 support.

From contributing to GUIShop I know that legacy support requires a lot of compatibility workarounds. However, 1.8 support is definitely something I'm willing to put in the effort for, so long as my circumstances are this way.

pablo67340 commented 4 years ago

Okay so we leave 1.8 support in, works for me 💯

What do you think about implementing some of these new features such as potions or nbt within our current loading methods?

A248 commented 4 years ago

It shouldn't be too hard to do. We can just add the 'potionType' tag as you describe and add it to potion items.

IMO, I would prefer a more compact approach to NBT, instead of branching the NBT vertically, consider this:

Raiding:
  '0':
    type: SHOP
    id: IRON_SWORD
    buy-price: 20.0
    sell-price: 10.0
    custom-nbt: '{Unbreakable:1,ench:[{id:16,lvl:1000}]}'

We might have to semi-parse the NBT this way, but with a JSON library it shouldn't be too hard. This approach would certainly help users with lots of NBT tags, so they don't have to convert it to key-value yml form.

pablo67340 commented 3 years ago

Config format took a massive change. All of this has been resolved!