otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.6k stars 1.06k forks source link

sell Items with subtype [2 bugs] #2475

Open GoularPink opened 6 years ago

GoularPink commented 6 years ago

Bug 1: How is it: The vials that have subtype are appearing to sell in npc, but when we click to sell, npc says that we do not have the item.

Fix would be: Or do not show these items or be able to sell them.

Print: https://imgur.com/vR7fhTT

script inside some seller/buyer npc: <parameter key="shop_sellable" value="Spellwand,7735,299; Vial,2006,5" />

Bug 2: How it is: The item appears in npc for sell, but we don't have any items in our backpack

print: http://prntscr.com/k2vuow

script: `<?xml version="1.0" encoding="UTF-8"?>

`

andersonfaaria commented 6 years ago

can confirm this. The error is in npc buy/sell module, I don't recall exactly but it checks for specific type if it's given, otherwise it only buys for type = 0

Another bug is that if you put a golden cup in a monster's drop, it will drop with type = 1 (water). When you try to sell to the npc, since the golden cup isn't empty, it will say you don't have the item.

GoularPink commented 6 years ago

The second bug no need put in monster, just create this npc script and say trade without any item in your backpack.

andersonfaaria commented 6 years ago

yea, that was exactly what I've said, that's why I put

Another bug is that if you put a golden cup in a monster's drop, it will drop with type = 1 (water)`

DSpeichert commented 6 years ago

Do you intend to submit a PR to fix this?

andersonfaaria commented 6 years ago

@DSpeichert I don't have the solution, I found this bug on my own server but didn't fixed it yet, just cataloged it. But as far as I tracked it, it's an error in buy/sell module because it have a weird logic when it comes to subtype:

if the subtype is given in npc xml, everything ok, it will only buy/sell from the specific subtype. else, it just buy/sells from subtype 0 (empty).

what should be changed is for the else case, it accepts the item regardless of subtype but I didn't know if it could have any side-effect.

andersonfaaria commented 6 years ago

And the other bug I've mentioned: items in monster's drop behave a little differently, when the subtype is not specified they are added like "doCreateItem(item, count/subtype)" and therefore when there's no subtype specified they come with subtype 1 (water) because the count is always 1 when not specified.

This has to be re-made in source (I don't really know where) to check if item can hold a fluid, then the subtype is always 0 unless specified otherwise.

Zbizu commented 3 years ago

It's a client limitation bound to client id of the items. There isn't much that can be done besides writing a new protocol for npc trading in otclient.

Zbizu commented 2 years ago

did the merged PR fix the bugs?

gesior commented 2 years ago

@Zbizu Not really. After these changes, it shows that you have '20 might rings' on sell window, when you have 0 of them. I don't know why there is different if, when we check for 5+ items.

There is really a lot of code in NPC Lua lib not compatible with C++ code of Trade window. It looks like it's impossible to restrict items with charges to be sellable only with given charges count (ex. not used might ring). Even, if you fix it in C++, you can still sell used rings using OTC with modified packet, as Lua code ignores subType for items that are not fluids: image

I've replaced:

                uint32_t count;
                if (itemType.isFluidContainer() || itemType.isSplash()) {
                    count = player->getItemTypeCount(shopInfo.itemId, subtype); // This shop item requires extra checks
                } else {
                    count = subtype;
                }

with:

                uint32_t count = player->getItemTypeCount(shopInfo.itemId, subtype); // This shop item requires extra checks

and it looks like it works fine now on C++ side [count of sellable items on Trade window].

Other Lua trading problem: 'chat trade' totally ignores subType in 'sell'. It's not even passed as parameter. With that C++ fix above. I can set might ring to by sellable by Trade Window only with 20 charges:

shopModule:addSellableItem({'might ring'}, 2164, 5000, 'might ring', 20)

but if I say sell might ring, it will still accept might ring with 1 charge. [spoofed 'sell by trade window' packet will also let me sell 1 charge might ring, as it's not fluid and subType is ignored]

// WORKING ON FIX