ChestShop-authors / ChestShop-3

ChestShop - the chest & sign shop plugin for Minecraft Servers running Bukkit/Spigot/Paper
https://dev.bukkit.org/projects/chestshop
GNU Lesser General Public License v2.1
269 stars 175 forks source link

Please, allow configure additional signs #562

Open vgrynch opened 11 months ago

vgrynch commented 11 months ago

Is your feature request related to a problem? Please describe.

Would be good if, in addition to the list of supported containers, we could specify the list of signs allowed to use. Custom data packs and hybrid server (modded) content may add new signs.

Describe the solution you'd like

ChestShop works great with the custom modded or custom data pack chests added (with a custom namespace) when added to the list of supported containers. I hope, since there is already some support for custom namespace chests, it would not be hard to allow specifying additional signs for use.

Describe alternatives you've considered

No response

Agreements

Additional context

No response

Phoenix616 commented 11 months ago

With which datapack/mod do you have issues with? Because the current code should already work with any block which behaves like a proper sign.

vgrynch commented 11 months ago

I use BYG. Behaves - right, but, unfortunately, I doubt the plugin would always know well that certain custom items have the required behavior (belongs to certain types of items). This exactly happens in my case. That's why I propose adding a new config parameter to force ChestShop to know it is a sign. Just allow adding custom items into a list of supported signs, the same way as for chests. Custom chests work great!

To add some context (I'm Java developer, but do not know well internals of Minecraft): All modded items are exposed to the plugins on the server I'm using in the form of {namespace}_{id}. For example, for the sign on the image below, it will be "BYG_LAMENT_SIGN".

So, something in the config file like:

.............

# What signs are allowed to hold a shop?
SIGNS:
- "SIGN"
- "BYG_LAMENT_SIGN"

# What containers are allowed to hold a shop? (Only blocks with inventories work!)
SHOP_CONTAINERS: 
- "CHEST"
- "TRAPPED_CHEST"

...............

(Make sure you do not access custom items using "minecraft" namespace - some plugins fail to work with custom items exactly because of that.)

image

Phoenix616 commented 11 months ago

So if I read this right then there is no issue in you case? That's good to hear that the code indeed works as I expected.

As I said: Any custom block that behaves like a sign is supported. (As long as the mod and the server implementation implements the Bukkit API properly) The type of the sign block is not checked anywhere in the plugin.

The only reason why there is a config for containers is because the plugin is meant for chests in the first place and people might not want to allow specific container types/sizes, especially as they might have special behaviour different from a chest (e.g. a Dropper). That's not something that is part of signs in Minecraft so I personally don'f t see a need for this.

I'm of course open for a proper PR if you really want to add/get someone to add it.

vgrynch commented 11 months ago

No, it does not work in my case, you can see it in the last image. ChestShop simply does not recognize it as a sign. Maybe I can fix it somehow with a data pack or additional tags? Any known examples of making a "correct" custom sign item that works with ChestShop?

Phoenix616 commented 11 months ago

@vgrynch Ah, I see. The text was pretty hard to read on there.

Contact support of your server software then. They are not properly implementing the Bukkit/Spigot API, there is nothing I can do about your issue as there is no "list of allowed sign blocks" in ChestShop which would be blocking this.

vgrynch commented 11 months ago

You mean this event, right? https://github.com/ChestShop-authors/ChestShop-3/blob/9bbd6028d08126806cc5ab5b6e45b6a2374d42b7/src/main/java/com/Acrobot/ChestShop/Listeners/Block/SignCreate.java#L16-L26

I will contact them and check what they say. Please, do not close this ticket for now.

Phoenix616 commented 11 months ago

That's the main listener for shop creation, yes.