Please add a placeholder or dummy first trade
ryantheleach opened this issue · 9 comments
I currently prefer the approach of adding placeholder items for empty slots, instead of a dummy initial trade. The dummy trade could still be something to considered in the future, but there should be less need for it once there are labeled placeholders in empty trade slots inside the editor.
The latest snapshot version adds these placeholder items to all empty trade slots inside the editor (bad998c). It is possible to specify different items for colums that don't contain any trade, as well as empty slots of partially set up trades. These items now have default display names and lore that describe their meaning and usage.
I have chosen to use gray stained glass pane items for slots of completely empty columns by default, since those are quite subtle. When using light gray stained glass panes it is even possible to make these items completely invisible inside the inventory (when using the default Minecraft textures).
Please let me know what you think about these changes, including the default item types and messages.
Give me a few days to take a look, I will be 100% using these changes immediately, (but am currently on a weekend vacation)
I went to acquire a snapshot build from the maven server, but I do not see any 2.15 snapshots listed. I'm unable to build from source atm, so it will need to wait until I get back home.
I've grabbed a copy from github actions, was mildly confused at why Shopkeepers and ShopkeepersMain were both listed. Not sure of the difference.
This looks pretty good.
The only thing is, is that the barrier blocks look like something bad or an error when I created a shop with a chest already containing items, and it wasn't immediately obvious that the barrier blocks were a placeholder.
This could be easily remedied IMO if the lore text when a barrier block was present explained a little more.
e.g. highlighting would give "No Trade Item Set" or something, in addition to the placeholder text.
It's more straightforward when a shop is created using an empty chest, as it defaults to stained glass however.
I'd be more tempted to use gray stained glass for the job barrier currently has.
This is especially driven home by the dissonance caused because there appears to be a difference between the stained glass, and the barrier, when I believe they fulfil the same function, except one appears in the case of a partial trade, even when the trade is valid.
As a user, I would have expected a valid trade to have no "NO" signs.
They are probably somewhat appropriate when a trade has a problem, like being invalid or out of stock.
Additionally, adding the barrier blocks & placeholders gives the impression that it should be possible to set up a 'free' trade.
e.g. giving a specific item away for free, inside the chest, or accepting donations of a specific item only.
Because a trade item of 'Nothing' currently has 2 valid representations, either an unset trade, or unset item, it would feel like having a purposely set 'nothing' with no item or placeholder would be possible.
So whilst I appreciate the change and will use it immediately, I fear it's trading one UX problem for another without some small tweaks.
Placeholder items have been added in v2.15.1, and the empty trade slot items are configurable.
If you are currently too busy to implement this, would you be willing to accept a PR?
Would you want it fully configurable?
Would you want it to apply to all shop types, or only the trade shop?
Yeah, I am too buisy currently (and the next weeks). I am also not sure if I find the time to review a PR any time soon.
And I am also not too sure about having an 'example trade'. Instead an alternative could be to have placeholder items in all slots of the editor's trades area whenever the slot is considered 'empty' (eg. when a user changes the item's amount or the cost to 0), These placeholder items would explain the slot's purpose (similar to your image, but for all slots) and maybe also the (shop type specific) instructions on how to setup the trade (eg. "Click to increase cost", "Click and place an item from your inventory here", ..). And whenever the slot is considered empty again it fills in that placeholder item again.
However, the admin shopkeeper would require some changes for this. I was thinking about having it behave similar to the player trading shopkeeper anyways. But if you create a PR for something like this maybe limit it to the player shops for now. The items and messages would ideally be fully configurable. Items specified in the config can already contain message for display names and lore, however, these messages should ideally be part of the 'messages' section, so that external language files can be created for those.
I think there are already 'placeholder' items for selling, buying and books shops (zero currency items). However, I think they are currently only inserted for columns which contain a result item. And they have no default messages with instructions currently.
One option could be to reuse these zero currency items for the trading shop as well (and just have them use different messages). So then only a new 'empty-result-item' or similar would be required, which could be used for empty top row slots.
Personally I would probably go with a barrier block for it as default as well, but if you have a better idea (to visually separate it from the 'cost' slots, without making the impression of it representing an actually traded item) let me know. Other options to represent the empty result slot could be: painting, sign, minecart, empty bucket, armor stand, banner pattern, paper, ..
Cheers for the fast reply!
I was wanting this change for myself anyway, so was likely going to fork ShopKeepers until the feature got pulled for my own server, So don't sweat if the PR stays open for some time.
I feel like once the code's in place, coming up with defaults that people are happy with is just a matter of getting the default config right, and can come afterwards, that said, I'll take your comments into account.
The screenshots above were just to communicate my intent more then anything, so I'm not heavily attached to anything there.
Placeholders were actually my preference then a dummy trade, but I figured this would be faster to implement and demonstrate in a ticket.