Shopkeepers

Shopkeepers

2M Downloads

Bugs and Feature Request

blablubbabcDEV opened this issue ยท 8 comments

commented

Migrated from: https://dev.bukkit.org/projects/shopkeepers/issues/315

Originally posted by MineCraftMyph (May 18, 2015):

[at]blablubbabc: Go
Feature Requests

  1. I would like to have shopkeepers in which only certain permissions can access.
  2. It would be nice if we could allow some shopkeepers to walk about, not being locked in position.
  3. Executing custom commands on right click from the player and console.
  4. Needs permissions to interact with separate shopkeepers: shopkeeper.1
  5. If the correct item is placed within the rewards slot, then items are given back. For example if it costs 1 diamond for a Wooden Pickaxe, we could put the Wooden Pickaxe into the end slot, and we'll receive 1 diamond in the first slot.

Other than that great plugin.
Bug
If the shop item has lore, and your item does not, it can not be traded.
http://u.cubeupload.com/Pleasured/47eUntitled.png

commented

Originally commented by ajisfree (May 19, 2015):

Lol Actually I am having the exact Opposite issue with this. I have a shop keeper with Items I want them to trade for but Lore/Name is being ignored. For example I have a custom recipe that creates a emerald named "Token" and Has a lore "Use in trade shops". I set that as the coast for the shopkeeper to give a diamond sword and It loads it up as the shop. But I noticed that it is not checking the Lore or Name, thus allowing any emerald to be traded rather then the "Token".
I am using Version 1.55
I am also using it with the Citizens Plugin.
Attached are the screenshots:

        Edited May 19, 2015
commented

Originally commented by MineCraftMyph (May 20, 2015):

[at]blablubbabc:
Fantastic, when can we expect the next build with these improvements?

commented

Originally commented by blablubbabc (May 20, 2015):

[at]MineCraftMyph:
If I don't run into greater problems you can expect the setting which lets you disable the name and lore comparison for traded items (including currency items as well) in the next few (1-2) days.

If I find some more time for this weekend I might look into adding a command for assigning admin shopkeepers a permisison node, which limits who can interact with the shopkeeper (either blocking the trading with the shopkeeper, or already the opening of the trading interface, not sure about that yet).
You can create separate tickets for your remianing suggestions (the command thing maybe). Though those don't have a high enough priority for me currently to work on them. You can however easily create a small addon plugin if you really need this feature for your server. Shopkeepers has methods which allow you to check if a given entity is a shopkeeper entity.

        Edited May 20, 2015
commented

Originally commented by blablubbabc (May 22, 2015):

[at]MineCraftMyph:
After implementing this setting now and doing the first tests I found out that it seems like minecraft's behavior has changed in 1.8.x:

Minecraft is now comparing item names and lores itself (and all other data as well), if the required item has a custom name/lore/etc.. and is blocking the trade, if the item names/lore/etc. don't match.
Because Shopkeepers relies on minecraft's default trading mechanics (we only add additional requirements, but we don't modify/remove the existing ones) adding this setting in the form I was thinking it would work is/would be more complicated as I expected it to be.
So basically: if minecraft isn't allowing the trade in the first place (because the item in the shop has a custom name for example, and the item offered does not), I can't simply change this behavior.
The only thing I can try is making it possible to trade an item with custom name/lore when the required item has no custom name/lore/data at all. Because this is something which minecraft currently would allow, but shopkeepers is blocking it.
However, the other way around (trading an item without or with different name/lore/etc. in place of a required item which has custom data (like a custom name or lore) is not something I currently think I could simply add.. :(

        Edited May 22, 2015
commented

Originally commented by blablubbabc (May 23, 2015):

I have made some changes to shopkeepers to better match minecraft's new trading logic (the one from MC 1.8.x) and released this for testing purposes as v1.56 here. Additionally I have added a setting (by default active) which lets you turn on the current, in certain cases more strict item comparison.
So maybe check that version out and disable this setting, and hopefully this behavior is what you are looking for.
If this is not what you are looking for, then I think I have to decline your request because of the already mentioned reasons of shopkeepers depending mostly on minecraft's trading logic and trying to modify that being too complicated / too much work for me.
I will close this ticket as fixed, because of the changes I made.

Regarding the permissions thing: the other ticket about this is still open, and I will try to look into it any time soon.
In case you find bugs/issues with v1.56 then please let me know by creating new tickets for those.

        Edited May 23, 2015
commented

Originally commented by MineCraftMyph (May 19, 2015):

[at]ajisfree:
I know it's annoying right.

commented

Originally commented by blablubbabc (May 19, 2015):

[at]MineCraftMyph:
Please create separate tickets for each of your feature requests and issues, so discussion about each of them is separated from each other.
I will comment on those here nevertheless:
Seems like a duplicate of this (long existing) ticket: http://dev.bukkit.org/bukkit-plugins/shopkeepers/tickets/254-permission-based-shopkeepers/ .
Will probably not be added because of the additional complexity it adds to the plugin. You can however look into the experimental citizens shopkeeper type. Citizens npcs have the ability to walk around.
What would that be needed for?
Same as first one?
I don't think this would work. The shopkeeper trading interfaces is mostly using the underlying mincraft trading mechanics. So you can't put an item into the result slot and expect an item in the trading slots to pop up. You could however setup additional trades in which you have simply switched the required and the result items.

Regarding the bug::

The commonly expected behavior is the current one, that's why it has been this way already from the first versions of shopkeepers, as far as I am aware.
The current and intended behavior is slightly different from minecraft's default behavior:

If the item offered and the item required for the trade don't perfectly match (ex. displayname or lore are different) the trade won't be processed.

This is because there are many plugins which introduce new items with new custom behavior, which simply differ in item name and/or lore. So if shopkeepers would consider all of the items to be the same, if only their type and data value matches (like minecraft does it by default), this would cause issues on most servers which use such plugins. As an example you can see [at]ajisfree: reply below: he wants that emeralds with custom name ("Token") are considered different than normal emerlads (without/different custom name). This is the behavior which most people expect from shopkeepers.
What I can do however is to add some setting(s) which let you specify globally (for all shopkeepers) that you want to skip the plugins name and/or lore comaprison.
[at]ajisfree:
Please create a separate ticket for your issue(s) in the future.

However, I was not able to reproduce your issue on my test server.

The result item is displayed because of minecraft's default trading behavior. Shopkeepers is modifying the trading behavior only where it is easily possible with the available api.

However, getting the displayed item out of the result slot, if the required items don't match, should be successfully blocked by shopkeepers. At least I was not able to get the item in my tests.

        Edited May 19, 2015
commented

Originally closed by blablubbabc (May 23, 2015)