TFS 1.X+ NPC [ShopModule.onSell]

[ralke]

Hi again! I'm having a constant error, but I can't figure out what is triggering it. I use TFS 1.4 downgrade by nekiro (8.6), it says that players are attempting to sell a non-sellable item, but, don't know which NPC might have this problem, there's any way to fix this, or locate the error by printing something?.



Thanks in advance!
Regards

 

[ralke]

Still having the same issue, anyone knows how can I track where this error come from?

 

[Evil Puncker]

try changing this:

        if shopItem.sell == -1 then
            error("[ShopModule.onSell] attempt to sell a non-sellable item")
            return false
        end

to this:

        if shopItem.sell == -1 then
            error("[ShopModule.onSell: " .. Npc():getName() .. "] attempt to sell a non-sellable item")
            return false
        end

not tested but should show npc name

 

[ralke]

@Evil Puncker will do and see how it goes, replaced shopItem.buy with npc prints too
I'll answer again as soon I get results, thanks as always

 

[ralke]

Hi @Evil Puncker ! The npc print is working perfectly, it is triggering the error with mutiple NPCs.


Here's the NPCs attached

Jyscal.lua

<?xml version="1.0" encoding="UTF-8"?>
<npc name="Jyscal" script="runes.lua" walkinterval="0" floorchange="0">
    <health now="100" max="100" />
    <look type="154" head="119" body="77" legs="79" feet="114" addons="1"/>
    <parameters>
    </parameters>
</npc>

Spoiler: runes.lua

local keywordHandler = KeywordHandler:new()
local npcHandler = NpcHandler:new(keywordHandler)
NpcSystem.parseParameters(npcHandler)

function onCreatureAppear(cid)
    npcHandler:onCreatureAppear(cid)
end
function onCreatureDisappear(cid)
    npcHandler:onCreatureDisappear(cid)
end
function onCreatureSay(cid, type, msg)
    npcHandler:onCreatureSay(cid, type, msg)
end
function onThink()
    npcHandler:onThink()
end

local shopModule = ShopModule:new()
npcHandler:addModule(shopModule)

shopModule:addBuyableItem({'spellbook'}, 2175, 150, 'spellbook')
shopModule:addBuyableItem({'magic lightwand'}, 2163, 400, 'magic lightwand')

shopModule:addBuyableItem({'small health'}, 8704, 20, 1, 'small health potion')
shopModule:addBuyableItem({'health potion'}, 7618, 45, 1, 'health potion')
shopModule:addBuyableItem({'mana potion'}, 7620, 50, 1, 'mana potion')
shopModule:addBuyableItem({'strong health'}, 7588, 100, 1, 'strong health potion')
shopModule:addBuyableItem({'strong mana'}, 7589, 80, 1, 'strong mana potion')
shopModule:addBuyableItem({'great health'}, 7591, 190, 1, 'great health potion')
shopModule:addBuyableItem({'great mana'}, 7590, 120, 1, 'great mana potion')
shopModule:addBuyableItem({'great spirit'}, 8472, 190, 1, 'great spirit potion')
shopModule:addBuyableItem({'ultimate health'}, 8473, 310, 1, 'ultimate health potion')
shopModule:addBuyableItem({'antidote potion'}, 8474, 50, 1, 'antidote potion')
---- 12x potions
shopModule:addBuyableItem({'great mana'}, 12784, 438, 1, 'ultimate mana potion')
shopModule:addBuyableItem({'great spirit'}, 12786, 438, 1, 'ultimate spirit potion')
shopModule:addBuyableItem({'ultimate health'}, 12785, 625, 1, 'supreme health potion')


shopModule:addSellableItem({'normal potion flask', 'normal flask'}, 7636, 5, 'empty small potion flask')
shopModule:addSellableItem({'strong potion flask', 'strong flask'}, 7634, 10, 'empty strong potion flask')
shopModule:addSellableItem({'great potion flask', 'great flask'}, 7635, 15, 'empty great potion flask')

---shopModule:addBuyableItem({'instense healing'}, 2265, 95, 1, 'intense healing rune')
---shopModule:addBuyableItem({'ultimate healing'}, 2273, 175, 1, 'ultimate healing rune')
---shopModule:addBuyableItem({'magic wall'}, 2293, 350, 3, 'magic wall rune')
---shopModule:addBuyableItem({'destroy field'}, 2261, 45, 3, 'destroy field rune')
---shopModule:addBuyableItem({'light magic missile'}, 2287, 40, 10, 'light magic missile rune')
---shopModule:addBuyableItem({'heavy magic missile'}, 2311, 120, 10, 'heavy magic missile rune')
---shopModule:addBuyableItem({'great fireball'}, 2304, 180, 4, 'great fireball rune')
---shopModule:addBuyableItem({'explosion'}, 2313, 250, 6, 'explosion rune')
---shopModule:addBuyableItem({'sudden death'}, 2268, 350, 3, 'sudden death rune')
---shopModule:addBuyableItem({'death arrow'}, 2263, 300, 3, 'death arrow rune')
---shopModule:addBuyableItem({'paralyze'}, 2278, 700, 1, 'paralyze rune')
---shopModule:addBuyableItem({'animate dead'}, 2316, 375, 1, 'animate dead rune')
---shopModule:addBuyableItem({'convince creature'}, 2290, 80, 1, 'convince creature rune')
---shopModule:addBuyableItem({'chameleon'}, 2291, 210, 1, 'chameleon rune')
---shopModule:addBuyableItem({'desintegrate'}, 2310, 80, 3, 'desintegreate rune')

shopModule:addBuyableItem({'wand of vortex', 'vortex'}, 2190, 500, 'wand of vortex')
shopModule:addBuyableItem({'wand of dragonbreath', 'dragonbreath'}, 2191, 1000, 'wand of dragonbreath')
shopModule:addBuyableItem({'wand of decay', 'decay'}, 2188, 5000, 'wand of decay')
shopModule:addBuyableItem({'wand of draconia', 'draconia'}, 8921, 7500, 'wand of draconia')
shopModule:addBuyableItem({'wand of cosmic energy', 'cosmic energy'}, 2189, 10000, 'wand of cosmic energy')
shopModule:addBuyableItem({'wand of inferno', 'inferno'}, 2187, 15000, 'wand of inferno')
shopModule:addBuyableItem({'wand of starstorm', 'starstorm'}, 8920, 18000, 'wand of starstorm')
shopModule:addBuyableItem({'wand of voodoo', 'voodoo'}, 8922, 22000, 'wand of voodoo')

shopModule:addBuyableItem({'snakebite rod', 'snakebite'}, 2182, 500, 'snakebite rod')
shopModule:addBuyableItem({'moonlight rod', 'moonlight'}, 2186, 1000, 'moonlight rod')
shopModule:addBuyableItem({'necrotic rod', 'necrotic'}, 2185, 5000, 'necrotic rod')
shopModule:addBuyableItem({'northwind rod', 'northwind'}, 8911, 7500, 'northwind rod')
shopModule:addBuyableItem({'terra rod', 'terra'}, 2181, 10000, 'terra rod')
shopModule:addBuyableItem({'hailstorm rod', 'hailstorm'}, 2183, 15000, 'hailstorm rod')
shopModule:addBuyableItem({'springsprout rod', 'springsprout'}, 8912, 18000, 'springsprout rod')
shopModule:addBuyableItem({'underworld rod', 'underworld'}, 8910, 22000, 'underworld rod')

--- runes
shopModule:addBuyableItem({'intense healing'}, 2265, 95, 1, 'intense healing rune')
shopModule:addBuyableItem({'ultimate healing'}, 2273, 175, 1, 'ultimate healing rune')
shopModule:addBuyableItem({'magic wall'}, 2293, 350, 3, 'magic wall rune')
shopModule:addBuyableItem({'destroy field'}, 2261, 45, 3, 'destroy field rune')
shopModule:addBuyableItem({'light magic missile'}, 2287, 40, 10, 'light magic missile rune')
shopModule:addBuyableItem({'heavy magic missile'}, 2311, 120, 10, 'heavy magic missile rune')
shopModule:addBuyableItem({'great fireball'}, 2304, 180, 4, 'great fireball rune')
shopModule:addBuyableItem({'explosion'}, 2313, 250, 6, 'explosion rune')
shopModule:addBuyableItem({'sudden death'}, 2268, 350, 3, 'sudden death rune')
shopModule:addBuyableItem({'paralyze'}, 2278, 700, 1, 'paralyze rune')
shopModule:addBuyableItem({'animate dead'}, 2316, 375, 1, 'animate dead rune')
shopModule:addBuyableItem({'convince creature'}, 2290, 80, 1, 'convince creature rune')
shopModule:addBuyableItem({'chameleon'}, 2291, 210, 1, 'chameleon rune')
shopModule:addBuyableItem({'disintegrate'}, 2310, 80, 3, 'disintegrate rune')

--- added
shopModule:addBuyableItem({'energy bomb'}, 2262, 203, 2, 'energy bomb')
shopModule:addBuyableItem({'energy field'}, 2277, 38, 3, 'energy field')
shopModule:addBuyableItem({'energy wall'}, 2279, 85, 3, 'energy wall')

shopModule:addBuyableItem({'fire bomb'}, 2305, 147, 2, 'fire bomb')
shopModule:addBuyableItem({'fire field'}, 2301, 28, 3, 'fire field')
shopModule:addBuyableItem({'fire wall'}, 2303, 61, 3, 'fire wall')
shopModule:addBuyableItem({'fireball'}, 2302, 30, 5, 'fireball')

shopModule:addBuyableItem({'holy missile'}, 2295, 16, 5, 'holy missile')
shopModule:addBuyableItem({'icile'}, 2271, 30, 5, 'icicle')
shopModule:addBuyableItem({'avalanche'}, 2274, 57, 3, 'avalanche')
shopModule:addBuyableItem({'cure poison'}, 2266, 65, 1, 'cure poison')

shopModule:addBuyableItem({'poison bomb'}, 2286, 85, 2, 'poison bomb')
shopModule:addBuyableItem({'poison field'}, 2285, 21, 3, 'poison field')
shopModule:addBuyableItem({'poison wall'}, 2289, 52, 3, 'poison wall')

shopModule:addBuyableItem({'soulfire'}, 2308, 46, 3, 'soulfire')

shopModule:addBuyableItem({'stalagmite'}, 2292, 203, 10, 'stalagmite')
shopModule:addBuyableItem({'stone shower'}, 2288, 203, 3, 'stone shower')
shopModule:addBuyableItem({'thunderstorm'}, 2315, 203, 3, 'thunderstorm')

shopModule:addBuyableItem({'wild growth'}, 2269, 160, 2, 'wild growth')

--- ammy
shopModule:addBuyableItem({'dragon necklace'}, 2201, 1000, 200, 'dragon necklace')
shopModule:addBuyableItem({'protection amulet'}, 2200, 700, 250, 'protection amulet')
shopModule:addBuyableItem({'strange talisman'}, 2161, 100, 200, 'strange talisman')
shopModule:addBuyableItem({'silver amulet'}, 2170, 100, 200, 'silver amulet')


shopModule:addSellableItem({'wand of vortex', 'vortex'}, 2190, 250, 'wand of vortex')
shopModule:addSellableItem({'wand of dragonbreath', 'dragonbreath'}, 2191, 500, 'wand of dragonbreath')
shopModule:addSellableItem({'wand of decay', 'decay'}, 2188, 2500, 'wand of decay')
shopModule:addSellableItem({'wand of draconia', 'draconia'}, 8921, 3750, 'wand of draconia')
shopModule:addSellableItem({'wand of cosmic energy', 'cosmic energy'}, 2189, 5000, 'wand of cosmic energy')
shopModule:addSellableItem({'wand of inferno', 'inferno'},2187, 7500, 'wand of inferno')
shopModule:addSellableItem({'wand of starstorm', 'starstorm'}, 8920, 9000, 'wand of starstorm')
shopModule:addSellableItem({'wand of voodoo', 'voodoo'}, 8922, 11000, 'wand of voodoo')

shopModule:addSellableItem({'snakebite rod', 'snakebite'}, 2182, 250,'snakebite rod')
shopModule:addSellableItem({'moonlight rod', 'moonlight'}, 2186, 500, 'moonlight rod')
shopModule:addSellableItem({'necrotic rod', 'necrotic'}, 2185, 2500, 'necrotic rod')
shopModule:addSellableItem({'northwind rod', 'northwind'}, 8911, 3750, 'northwind rod')
shopModule:addSellableItem({'terra rod', 'terra'}, 2181, 5000, 'terra rod')
shopModule:addSellableItem({'hailstorm rod', 'hailstorm'}, 2183, 7500, 'hailstorm rod')
shopModule:addSellableItem({'springsprout rod', 'springsprout'}, 8912, 9000, 'springsprout rod')
shopModule:addSellableItem({'underworld rod', 'underworld'}, 8910, 11000, 'underworld rod')

function creatureSayCallback(cid, type, msg)
    if not npcHandler:isFocused(cid) then
        return false
    end

    local player = Player(cid)
    local vocationId = player:getVocation():getId()
    local items = {
        [1] = 2190,
        [2] = 2182,
        [5] = 2190,
        [6] = 2182
    }

    if msgcontains(msg, 'first rod') or msgcontains(msg, 'first wand') then
        if isInArray({1, 2, 5, 6}, vocationId) then
            if player:getStorageValue(3050) == -1 then
                selfSay('So you ask me for a {' .. ItemType(items[vocationId]):getName() .. '} to begin your advanture?', cid)
                npcHandler.topic[cid] = 1
            else
                selfSay('What? I have already gave you one {' .. ItemType(items[vocationId]):getName() .. '}!', cid)
            end
        else
            selfSay('Sorry, you aren\'t a druid either a sorcerer.', cid)
        end
    elseif msgcontains(msg, 'yes') then
        if npcHandler.topic[cid] == 1 then
            player:addItem(items[vocationId], 1)
            player:setStorageValue(3050, 1)
            selfSay('Here you are young adept, take care yourself.', cid)
        end
        npcHandler.topic[cid] = 0
    elseif msgcontains(msg, 'no') and npcHandler.topic[cid] == 1 then
        selfSay('Ok then.', cid)
        npcHandler.topic[cid] = 0
    elseif isInArray({"vial", "ticket", "bonus"}, msg) then
        if player:removeItem(7634, 100) or player:removeItem(7635, 100) or player:removeItem(7636, 100) then
            player:addItem(5957, 1)
            npcHandler:say("Alright, thank you very much! Here is your lottery ticket, good luck. Would you like to deposit more vials that way?", cid)
            npcHandler.topic[cid] = 0
        else
            npcHandler:say("Sorry, but you don't have 100 empty flasks or vials of the SAME kind and thus don't qualify for the lottery. Would you like to deposit the vials you have as usual and receive 5 gold per vial?", cid)
            npcHandler.topic[cid] = 0
        end
    end

    return true
end

npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback)
npcHandler:addModule(FocusModule:new())

And Lucil

<?xml version="1.0" encoding="UTF-8"?>
<npc name="Lucil" script="Lucil.lua" walkinterval="0" floorchange="0">
    <health now="100" max="100" />
    <look type="142" head="94" body="70" legs="2" feet="60" addons="0"/>
    <parameters>
        <parameter key="module_shop" value="1" />
        <parameter key="shop_buyable" value="
            white mushroom,2787,6;
            brown mushroom,2789,10;
            red mushroom,2788,12;
            banana, 2676, 3;
            blueberry, 2677, 1;
            carrot, 2684, 3;
            cherry, 2679, 1;
            cucumber, 8842, 5;
            grapes, 2681, 3;
            mango, 5097, 8;
            melon, 2682, 8;
            orange, 2675, 6;
            pear, 2673, 5;
            plum, 8839, 3;
            potato, 8838, 5;
            pumpkin, 2683, 10;
            raspberry, 8840, 1;
            red apple, 2674, 3;
            strawberry, 2680, 1;
            tomato, 2685, 4;
            meat, 2666, 4;
            ham, 2671, 6;
            bread, 2689, 4;
            cheese, 2696, 6;
            cookie, 2687, 5;
            egg, 2695, 2;
            ham, 2671, 8;
            meat, 2666, 5;" />
        <parameter key="shop_sellable" value="
            bamboo stick,12401,30;
            bundle of cursed straw,10605,800;
            carniphila seeds,11217,50;
            dark mushroom,2792,100;
            fire mushroom,2795,200;
            goat grass,2760,50;
            grave flower,2747,25;
            green mushroom,2796,100;
            lump of dirt,10609,10;
            lump of earth,11222,130;
            nettle blossom,11231,75;
            nettle spit,12432,25;
            orange mushroom,2790,150;
            powder herb,2803,10;
            seeds,7732,150;
            shadow herb,2804,20;
            sling herb,2802,10;
            swamp grass,10603,20;
            troll green,2805,25;
            trollroot,12471,50;
            wood mushroom,2791,15;
            empty honey glass,13045,270;" />
    </parameters>
</npc>

and Lucil.lua

local keywordHandler = KeywordHandler:new()
local npcHandler = NpcHandler:new(keywordHandler)
NpcSystem.parseParameters(npcHandler)

function onCreatureAppear(cid)            npcHandler:onCreatureAppear(cid)            end
function onCreatureDisappear(cid)        npcHandler:onCreatureDisappear(cid)            end
function onCreatureSay(cid, type, msg)        npcHandler:onCreatureSay(cid, type, msg)        end
function onThink()        npcHandler:onThink()        end

local voices = { {text = 'Selling various types of food, herbs and mushrooms, all picked under the light of the full moon!'} }
npcHandler:addModule(VoiceModule:new(voices))

keywordHandler:addKeyword({'offers'}, StdModule.say, {npcHandler = npcHandler, text = "I'm selling various types of food, mushrooms, and flowers. If you'd like to see my offers, ask me for a {trade}."})

npcHandler:setMessage(MESSAGE_GREET, "Greetings, traveller. Maybe you'd like to take a look at my {offers}...")
npcHandler:setMessage(MESSAGE_FAREWELL, "Goodbye, traveller.")
npcHandler:setMessage(MESSAGE_WALKAWAY, "Goodbye, traveller.")
npcHandler:setMessage(MESSAGE_SENDTRADE, "Of course, just browse through my wares.")
npcHandler:addModule(FocusModule:new())

What can be wrong here? I guess i'm missing something. Thanks in advance!

 

[Night Wolf]

This issue was already raised by me several years ago, it's a problem with otclient sell all function.
For the sake of your server, disable sell all button in otc. Tfs has not been made to handle this feature

Edit: found my original issue created

Sell All bugs · Issue #891 · edubart/otclient

There's three issues related to Sell All. Sell all doesn't work if you have more than 100 items (for stackable) The function that returns your items (onPlayerGoods) is unable to count more than 255...

github.com

 

[ralke]

This issue was already raised by me several years ago, it's a problem with otclient sell all function.
For the sake of your server, disable sell all button in otc. Tfs has not been made to handle this feature

Edit: found my original issue created

Sell All bugs · Issue #891 · edubart/otclient

There's three issues related to Sell All. Sell all doesn't work if you have more than 100 items (for stackable) The function that returns your items (onPlayerGoods) is unable to count more than 255...

github.com

Ohhh! Perfect, thought it was server-sided. Thanks for clarifying this been searching how to fix this
Taking this out of npctrade.otui will work?

  Button
    id: sellAllButton
    !text: tr('Sell All')
    width: 64
    anchors.right: next.left
    anchors.bottom: parent.bottom
    margin-right: 10
    visible: false
    @onClick: modules.game_npctrade.sellAll()

Haven't readed the issue yet, there's anything else should I look for before applying changes?
Or some way to actually make Sell all button work? Regards!

 

[Night Wolf]

Yes, you can remove the button, that's one way to do it.

But, bear in mind, this is beyond otclient functionality. The way most of our bases handles possible forged packets is absurdity. I can send a packet to the client saying I have bought a Magic Plate Armor for 1 gold coin and the server will attempt to process it. It will check that the item is not being sold for this value (we should be thankful for this one check) and return this message you're seeing.
The problem is that if someone spam packets your source will be spammed too and can even crash. We should have a limit or a proper validation channel to reject packets from users, especially when they seem to be malicious. Printing in console isn't the way to go in this case.
I've discussed this multiple times but apparently people were always more worried if it would break compatibility with previous tfs versions than if it would fix a major security flaw.

Post automatically merged: Feb 24, 2022

Even without the button you need to think someone might connect using another otclient or modules to your server. It isn't hard to fake your game RSA and encryption mode and people have been doing so for a couple of years now. Best to also remove the function, you can keep the signature of it but comment the code inside

Post automatically merged: Feb 24, 2022

Or some way to actually make Sell all button work? Regards!

I've tried that a couple of times, but it's hard. The source code inside tfs is very messy and each change breaks other stuff because things are too attached to eachother. In the issue you have all my comments on what I've tried and found, unfortunately no one helped me and back then I decided it was not worth to invest days and days of my precious time to fix a functionality most of players don't really care about.
We have been discussing how decouple the source code into something easier to insert testing but @skulls worked on that for months just to become frustrated by how hard it is to split the classes.

To be very frank, sometimes it seems that it would be easier to start fresh from scratch than to fix the issues we have in the architecture of tfs. The very least we can do is trying to apply punctual fixes and pray that it don't introduce another major critical issue that will be found 1 year from now.

 

[ralke]

@Night Wolf I get it! Thanks for the information is very valuble I guess that a player that floods this button, will be kicked by exceeding packets per second but that will still send too many bites to the server and that's not good. How about setting a delay or cooldown to the "sell all" function? Also have the question of where is this function coming from, just searched at npctrade.lua and was able to find:

This

    sellAllButton:setVisible(currentTradeType == SELL)

Also

    checkSellAllTooltip() ---- inside function refereshPlayerGoods()

And

function checkSellAllTooltip()
    sellAllButton:setEnabled(true)
    sellAllButton:removeTooltip()

    local total = 0
    local info = ''
    local first = true

    for key, amount in pairs(playerItems) do
        local data = getTradeItemData(key, SELL)
        if data then
            amount = getSellQuantity(data.ptr)
            if amount > 0 then
                if data and amount > 0 then
                    info =
                        info .. (not first and "\n" or "") .. amount .. " " ..
                            data.name .. " (" .. data.price * amount .. " gold)"

                    total = total + (data.price * amount)
                    if first then first = false end
                end
            end
        end
    end
    if info ~= '' then
        info = info .. "\nTotal: " .. total .. " gold"
        sellAllButton:setTooltip(info)
    else
        sellAllButton:setEnabled(false)
    end
end

Guess that's all, there's nothing else server-sided related to this or other client configurations I should have present?

Thanks again @Night Wolf
Regards!

 

[Night Wolf]

I guess that a player that floods this button, will be kicked by exceeding packets per second

I'm not so sure, I remember I was testing in my server and couldn't hit the limit to be kicked for this case. (I used max packets as 40) and I could even crash the source by spamming forged packets. Don't assume things are going to work as they should hahah

How about setting a delay or cooldown to the "sell all" function?

That could work but it's just like a band-aid, the issue is server side and putting delays on client side won't fix the real issue. Also anyone can edit your module and remove the delay

where is this function comming from, just searched at npctrade.lua and was able to find:

I pasted the function in the github discussion, sorry but it's been 4 years I don't really remember where this was exactly located. I'm not in the computer right now otherwise I could search for you

Post automatically merged: Feb 24, 2022

@ralke you actually have the function in your callback of the button you sent:

onClick: modules.game_npctrade.sellAll()

Just make sure the function is either deleted or commented (comment works better here for the sake of having a clear history on why this has been commented)