TFS 1.X+ getTopTopItem ignores corpses

[overdriven]

If I have a corpse and a pool of blood below it, getTopTopItem will get the pool of blood instead corpse.

player:say(tostring(tile:getTopTopItem():getType():getId()), TALKTYPE_MONSTER_SAY) -- returns 2016 (splash item)

I can move the corpse elsewhere, put it back on top of blood and getTopTopItem will still return pool of blood completely ignoring the corpse.

Why is it not returning the real item on top - the corpse?

Btw. what's the reason behind this silly function name "get Top Top"?

I'm using TFS 1.5.

Post automatically merged: Mar 15, 2022

I see now that getTopDownItem will get the corpse, but I have no idea what "TopDown" and "TopTop" means, it's counter-intuitive to be honest, I wish docs would explain what's the difference between these 2 methods.

 

[Night Wolf]

all I can tell you is that those functions are indeed counter-intuive and trying to understand their logic is a world of pain.
The whole stackpos system of our bases is badly designed and functions they have created are redundant and are also behaving wrongly.

I have raised an issue in TFS, but don't expect anything because it's been around for 2 years already:

tile:getThing(stack) not working properly · Issue #2957 · otland/forgottenserver

I don't know why and I can't confirm if this is the correct way of reproducing the issue but apparently when you have a stack of items in a single tile the function return the stacks in a wrong ord...

github.com

 

[Gesior.pl]

iterator getBeginDownItem() {
   return begin();
}
const_iterator getBeginDownItem() const {
   return begin();
}
iterator getEndDownItem() {
   return begin() + downItemCount;
}
const_iterator getEndDownItem() const {
   return begin() + downItemCount;
}
iterator getBeginTopItem() {
   return getEndDownItem();
}
const_iterator getBeginTopItem() const {
   return getEndDownItem();
}
iterator getEndTopItem() {
   return end();
}
const_iterator getEndTopItem() const {
   return end();
}

uint32_t getTopItemCount() const {
   return size() - downItemCount;
}
uint32_t getDownItemCount() const {
   return downItemCount;
}
inline Item* getTopTopItem() const {
   if (getTopItemCount() == 0) {
      return nullptr;
   }
   return *(getEndTopItem() - 1);
}
inline Item* getTopDownItem() const {
   if (downItemCount == 0) {
      return nullptr;
   }
   return *getBeginDownItem();
}

tile:getTopTopItem() is last element of top items list (last item on tile)
tile:getTopDownItem() is first element of down items list (first item on tile)
Names and orders are weird. Thanks to Tibia [client] items on tile implementation.
There are items with special attribute 'alwaysOnTop' (in .dat and .otb), which do not disappear when you put more than 10 items on tile.

It should be simpler to read, if you use 3 functions:

tile:getGround() - ground item, speed of walking is based on this, so it's allocated in special variable, not on list of 'items'
tile:getItems() - list of items on tile indexed from 1
tile:getCreatures() - list of creatures on tile indexed from 1

Read first item:

local items = tile:getItems()
local firstItem = items[1]

Read last item:

local items = tile:getItems()
local lastItem = items[#items]

of course it will waste much CPU to move all items to Lua, just to read one.
That's why there are functions getTopTopItem and getTopDownItem to get last and first item.

Function tile:getThing(index) is related to use of weird things order in tibia client - where ground, items and creatures are treated as one list with one 'index' (list in client is also limited to X elements, while on server you can put more things on tile [some of them disappear in client]).

Here is answer why it won't be 'fixed':

tile:getThing(stack) not working properly · Issue #2957 · otland/forgottenserver

I don't know why and I can't confirm if this is the correct way of reproducing the issue but apparently when you have a stack of items in a single tile the function return the stacks in a wrong ord...

github.com

tile:getThing(index) is in Lua to read item from tile using stackpos sent by Tibia Client.
Thing can be Item or Creature, so it's not function to read items.

 

[Nekiro]

if you want to get the first top thing on a stack use tile:getTopVisibleThing()

 

[Gesior.pl]

Just remember that tile:getTopVisibleThing() MAY also return Creature (Player/Monster/NPC), not only Item.

 

[Night Wolf]

Here is answer why it won't be 'fixed':

I can't really reply you in the issue but I'll explain why what you're saying makes no sense


1.
Items order on server side is related to weird items and creatures on tile implementation in Tibia Client, and Tibia communication protocol. There are items with 'alwaysOnTop' attribute, which does not disappear, even if you put 10+ items on given tile (tile items limit in client).
You cannot put items on tile in any order. When you add them in wrong order, server takes care of it and put them on valid stack [index].

I don't care about client at all, I have the items created by me in the specific order I wanted, my question is why stackpos (SERVER side) is returning in the right order for items and in the reverse order for borders.

2.
tile:getItems() return list of items, which in real contains 2 list of items. 'top item' and 'down items'. They are on one list to make it execute faster 'is item on tile'.
If you want to read this in some other 'order' than Tibia implementation, use tile:getDownItemCount() and tile:getTopItemCount() to get information on which index start second (reversed) list.

Now, what's the relevance of speed in this context? You're literally throwing out words without understanding the problem. How can getDownItemCount and getTopItemCount would help me? All I'm expecting is to get list of items in a given tile in the order they are server side, no matter how they appear in client or how fast they are created....

3.
Item structure is optimized to send tiles and changes on tiles to client as fast as possible.
You should check tile.cpp and protocolgame.cpp implementation of items and creatures on tile communication.

Again, what's the point of talking about optimizations when the logic is clearly wrong? If you don't know the answer don't ask me to go check the logic and solve myself when you started your statement saying the issue should be closed.

4.
MillhioreBT is not doing anything special. He just reads stackpos sent from Tibia client, which is not position on list in items in tile.cpp on server side. On server side you can put 1000 items on tile, in client only 10 will be visible and transferred from server.
It's server job to decide which 10 should be visible - it takes a lot of calculation and is executed 100k times per second on popular servers, so data structures are designed to handle this as fast as possible.

If stackpos is not server side, how it gets updated then? Do server ask the client what is the stackpos for the given items? In case I have 10k+ items in a tile how many of them will be iterable through tile:getItems()? Shouldn't be all of them? Why are we relying on client response to know which items are in a given position if I already have this information server side?

 

[Boy67]

iterator getBeginDownItem() {
return begin();
}
const_iterator getBeginDownItem() const {
return begin();
}
iterator getEndDownItem() {
return begin() + downItemCount;
}
const_iterator getEndDownItem() const {
return begin() + downItemCount;
}
iterator getBeginTopItem() {
return getEndDownItem();
}
const_iterator getBeginTopItem() const {
return getEndDownItem();
}
iterator getEndTopItem() {
return end();
}
const_iterator getEndTopItem() const {
return end();
}

uint32_t getTopItemCount() const {
return size() - downItemCount;
}
uint32_t getDownItemCount() const {
return downItemCount;
}
inline Item* getTopTopItem() const {
if (getTopItemCount() == 0) {
return nullptr;
}
return *(getEndTopItem() - 1);
}
inline Item* getTopDownItem() const {
if (downItemCount == 0) {
return nullptr;
}
return *getBeginDownItem();
}

This has to be some of the worst variable/function naming I've seen. I understand that English is not everyone's main language, but that's the sort of VB naming fucklogic I saw back in school 15 years ago.

 

[Night Wolf]

This has to be some of the worst variable/function naming I've seen. I understand that English is not everyone's main language, but that's the sort of VB naming fucklogic I saw back in school 15 years ago.

if could have been named "getTopTopDownLeftLeftRightR1SquareTriangle" and I wouldn't mind if it worked.
The problem I see is that every issue discovered in the repository you have always people with 0 background and that didn't even bothered to replicate the issue talking "this is inteded behavior, you don't know how to code".

Now you have also some kids with moderator functionalities in TFS repository, one of them banned me 30d because I politely asked him to give credits to a code he cherry picked from a project he is constantly snobbing.

It's really hard to keep a sanity level of discussing with adults that behave like they were in kindergarten.

 

[Gesior.pl]

I don't care about client at all, I have the items created by me in the specific order I wanted

You don't understand. You can't add items to tile in order 'you want'. Server will sort them anyway. If server does not sort items to 'valid order', it will debug client.

All I'm expecting is to get list of items in a given tile in the order they are server side

That's EXACTLY what you get with tile:getItems(). Items in order in which they are stored on server side.

Again, what's the point of talking about optimizations when the logic is clearly wrong?

EVERYONE knows that order is wrong and it should be done other way, but Tibia Client accepts data only in this fucked-up order.
Sorry, but OTS cannot change Tibia Client, it can only reproduce communication that Tibia Client does with official Tibia Servers.

In case I have 10k+ items in a tile how many of them will be iterable through tile:getItems()? Shouldn't be all of them?

There will be all of them, but if you use tile:getThing(index) you will get strange results, because tile:getThing(index) returns what Tibia Client see on tile. Why? Read below.

If stackpos is not server side, how it gets updated then? Do server ask the client what is the stackpos for the given items?

Communication goes other way. CLIENT sends something like use item on tile x,y,z, stackpos 10, even if on server side there are 5k items on tile.
Client sends it, because when server sent tile to client, there were only 10 items on list. Why 10 items, not 5k? Because client does not accept more (debug, if you send). This limit must be there, because otherwise walking into screen with 5k items on tile would transfer 5k items to client, which would take a lot of CPU time and transfer. Remember that Tibia was made in 1997 when people had 10 kb/s connections and it's protocol DID NOT change.

There are invisible GMs, that are visible to other GMs, but not to players. Now imagine that 'stackpos' of items and items visible in client may be different depending on that you can see or not GM.
Server has to take it all into consideration and keep track of 'what each player see on screen'.

getTopTopItems or getTopDownItems may sound strange to someone who does not understand how items work, but name says everything:

• TopTop - top item of top items, it's top from alwaysOnTop attribute from Tibia Client
• TopDown - top item of down items, items that are not alwaysOnTop, so they go down (below top items) and disappear when you add more items to tile

Final point:
OTS REPRODUCES WHAT OFFICIAL TIBIA SERVERS SEND/RECEIVE WHEN COMMUNICATE WITH TIBIA CLIENT.
Sorry, that communication protocol is fucked-up, but that's how it works.

 

[Night Wolf]

You don't understand. You can't add items to tile in order 'you want'. Server will sort them anyway. If server does not sort items to 'valid order', it will debug client.

I understand man but you're trying to defend the indefensable here... if I have 5 items ordered and I iterate them, the expectation is to get the item ids in order. If you need another function for client to work, then those two logics need to be split. Isn't it OBVIOUS?

You're trying to say that every user of the base should know in advance this mystical information about how tibia client organizes internally items "not always on top" in a specific order and reverses that order in items "always on top".. which by the way is also false because neither the border nor the items have this flag. So you're once again talking about something that you didn't even had the effort to go there and replicate.

That's EXACTLY what you get with tile:getItems(). Items in order in which they are stored on server side.

another reason why this is wrong, on server side the order is wrong.

EVERYONE knows that order is wrong and it should be done other way, but Tibia Client accepts data only in this fucked-up order.
Sorry, but OTS cannot change Tibia Client, it can only reproduce communication that Tibia Client does with official Tibia Servers.

I'm not asking anyone to change tibia client, all I'm saying is that the function in LuaScript shouldn't be adapted to show information on how it is going to be passed to client side, we expect a function that gives the right order. If we know we are merging two different arrays and the second one is on the reverse order of indexing, why we just don't iterate the second array in the reverse order inside the cpp function?
You're attaching too much in the details of the architecture implementation and forgetting the basics...

Communication goes other way. CLIENT sends something like use item on tile x,y,z, stackpos 10, even if on server side there are 5k items on tile.
Client sends it, because when server sent tile to client, there were only 10 items on list. Why 10 items, not 5k? Because client does not accept more (debug, if you send). This limit must be there, because otherwise walking into screen with 5k items on tile would transfer 5k items to client, which would take a lot of CPU time and transfer. Remember that Tibia was made in 1997 when people had 10 kb/s connections and it's protocol DID NOT change.

got it, but once again I'm not talking about number of items that big. I was testing with 3 items and could reproduce the issue still.

TL;DR:
For me, all the discussion shows what I've said earlier: you seem to understand on a very basic level what is the issue and is trying to convince someone that has spent days investigating those functions before opening an issue. Give it a try and replicate the issue first, you'll see what is wrong and you'll also see that the logic is strange and going against the common sense of what you would expect for such function. I was kind enough to provide all the details, stacks and items and explain where it was going in reverse order plus the code I was using (which has also commented the topDown function that I also tried)
Now, any proposal that you have to workaround this in Lua side, do the same logic and cpp and create a PR. That's all what is takes for the function to behave correctly and as expected.

If in my server the items are created in order 1, 2, 3, 4, 5, I need a function that can return me the list of items in the order they are and not any other order depending on attributes of those items.

 

[Nekiro]

You're trying to say that every user of the base should know in advance this mystical information about how tibia client organizes internally items "not always on top" in a specific order and reverses that order in items "always on top".. which by the way is also false because neither the border nor the items have this flag. So you're once again talking about something that you didn't even had the effort to go there and replicate.

forgottenserver/src/tile.cpp at master · otland/forgottenserver

A free and open-source MMORPG server emulator written in C++ - otland/forgottenserver

github.com

 

[Night Wolf]

forgottenserver/src/tile.cpp at master · otland/forgottenserver

A free and open-source MMORPG server emulator written in C++ - otland/forgottenserver

github.com

What does it has to do with the issue? Can you please try to replicate before you paste random links here?

 

[overdriven]

getTopTopItems or getTopDownItems may sound strange to someone who does not understand how items work, but name says everything:

• TopTop - top item of top items, it's top from alwaysOnTop attribute from Tibia Client
• TopDown - top item of down items, items that are not alwaysOnTop, so they go down (below top items) and disappear when you add more items to tile

@Gesior.pl Ok, I get it now. So it's because of client's alwaysOnTop attribute which splashed blood has. But anyhow:
1. It's not obvious for everyone and this TopTop function name is IMO bad and should be documented.
2. Since there are already TWO functions that care about this alwaysOnTop attribute why there isn't any function that disregard this attribute?
So instead of having to pull all items first like you suggest:

local items = tile:getItems()
local firstItem = items[1]

one would just do i.e.

local firstItem = tile:getTopItem() -- gets top item regardless if it has alwaysOnTop attribute or not

Going further, instead of having to come up with weird TopTop names, it would be even better if it was not three, but ONE generic parametrized function with optional alwaysOnTop attribute, i.e. tile:getTopItem(Boolean alwaysOnTop), where alwaysOnTop true=getItemTopTop, false=getItemTopDown, null=getItemTop. It would be so straightforward...

Not sure if possible as I'm not a Lua coder, but if it was JS I'd pass an object parameter which can potentially have more attributes in future:

local firstItem = tile:getTopItem() -- gets top item regardless if it has alwaysOnTop attribute or not
local firstItem = tile:getTopItem({alwaysOnTop: true}) -- gets top item with alwaysOnTop=true
local firstItem = tile:getTopItem({alwaysOnTop: false}) -- gets top item with alwaysOnTop=false

 

[Gesior.pl]

@overdriven
Add this to your data/lib/core/tile.lua:

function Tile.getTopItem(self)
    local items = self:getItems()
    if items and #items > 0 then
        return items[#items]
    end

    return self:getGround()
end

function Tile.getAllItems(self)
    local items = {}

    local tileItems = self:getItems()
    if tileItems and #tileItems > 0 then
        items = tileItems
    end

    local ground = self:getGround()
    if ground then
        table.insert(items, ground)
    end

    return items
end


function Tile.getAllItemsReversed(self)
    local items = {}

    local tileItems = self:getAllItems()
    for i = #tileItems, 1, -1 do
        table.insert(items, tileItems[i])
    end

    return items
end

Test tile (there are 3 sand borders):


How they work:
tile:getTopItem() - get top item visible in game (yellow bag)
tile:getAllItems() - get all tile items, including ground item, in order from top visible in game (yellow bag at index 1)
tile:getAllItemsReversed() - get all tile items, including ground item, in order from bottom visible in game (stone floor [ground] at index 1)

Results:

TEST getItems
getItems    1    1992    yellow bag
getItems    2    1988    backpack
getItems    3    9081    parcel
getItems    4    10640    sand
getItems    5    10638    sand
getItems    6    10639    sand

TEST allTileItems
allTileItems    1    1992    yellow bag
allTileItems    2    1988    backpack
allTileItems    3    9081    parcel
allTileItems    4    10640    sand
allTileItems    5    10638    sand
allTileItems    6    10639    sand
allTileItems    7    413    stone floor

TEST getAllItemsReversed
getAllItemsReversed    1    413    stone floor
getAllItemsReversed    2    10639    sand
getAllItemsReversed    3    10638    sand
getAllItemsReversed    4    10640    sand
getAllItemsReversed    5    9081    parcel
getAllItemsReversed    6    1988    backpack
getAllItemsReversed    7    1992    yellow bag

topTop    10639    sand
topDown    1992    yellow bag
topItem    10639    sand

Just remember that they waste some CPU and often tile:getTopDownItem() is what you really need.

Talkaction to test all functions:

function onSay(player, words, param)
    print('')
    print('TEST getItems')
    local tile = Tile(player:getPosition())
    local tileItems = tile:getItems()
    for k, v in pairs(tileItems) do
        local itemType = v:getType()
        print('getItems', k, v.itemid, itemType:getName())
    end

    print('')
    print('TEST allTileItems')
    local allTileItems = tile:getAllItems()
    for k, v in pairs(allTileItems) do
        local itemType = v:getType()
        print('allTileItems', k, v.itemid, itemType:getName())
    end

    print('')
    print('TEST getAllItemsReversed')
    local reversedTileItems = tile:getAllItemsReversed()
    for k, v in pairs(reversedTileItems) do
        local itemType = v:getType()
        print('getAllItemsReversed', k, v.itemid, itemType:getName())
    end

    print('')

    local topTop = tile:getTopTopItem()
    if topTop then
        local itemType = topTop:getType()
        print('topTop', topTop.itemid, itemType:getName())
    else
        print('no toptop')
    end

    local topDown = tile:getTopDownItem()
    if topDown then
        local itemType = topDown:getType()
        print('topDown', topDown.itemid, itemType:getName())
    else
        print('no topDown')
    end

    local topItem = tile:getTopItem()
    if topItem then
        local itemType = topItem:getType()
        print('topItem', topItem.itemid, itemType:getName())
    else
        print('no topItem')
    end

    return false
end

 

[Night Wolf]

@Gesior.pl
Now that you have checked the issue, can you confirm the borders order? is it still printing reverse?

 

[Gesior.pl]

@Gesior.pl
Now that you have checked the issue, can you confirm the borders order? is it still printing reverse?

I created these 3 sand borders in 4 different orders for test on 1.4.1. Function tile:getItems() always returned them in right order - starting from 'top visible' = last created.
If you still got problem with some order of items, post scenario how to reproduce it, because I can't get what is wrong reading your github issue.

To make it clear for all Lua scripters:
stackpos is not for managing items on tile. It's virtual value used in communication client-server.
It's not stored anywhere in Tile, it's not stored anywhere in Item. It's part of Position, but you can create Position without stackpos, as you often do in scripts like Position(123, 346, 7).
stackpos is generated on-fly, when it's required to write/read some data from client-server communication.
So, if your Lua script does not read/write NetworkPacket to communicate with Tibia Client, do not use stackpos.


TILE TESTS
Tile:

/i 10638
/i 10639
/i 10640
/i 2000

Result:

TEST getItems
getItems    1    2000    red backpack
getItems    2    10640    sand
getItems    3    10639    sand
getItems    4    10638    sand

Tile:

/i 10640
/i 10639
/i 10638
/i 2000

Result:

TEST getItems
getItems    1    2000    red backpack
getItems    2    10638    sand
getItems    3    10639    sand
getItems    4    10640    sand

 

[Night Wolf]

local items = tile:getItems()
        for i, v in ipairs(items) do print(i, v:getName(), v:getId()) end

should iterate in the item order all correct items regardless of its type specified. That was not the case when I created the issue but you're saying its working from your side...

I'm still without computer so can't re-test the issue on my side, but if it's really returning the correct order of creation then I agree with the issue being closed. Just make sure to add relevant explanations on the issue itself (or link this post instead)

 

[Azakelis]

Going further, instead of having to come up with weird TopTop names, it would be even better if it was not three, but ONE generic parametrized function with optional alwaysOnTop attribute, i.e. tile:getTopItem(Boolean alwaysOnTop), where alwaysOnTop true=getItemTopTop, false=getItemTopDown, null=getItemTop. It would be so straightforward...

rocket science

 

[btc]

How to get the current item? For example when a player moves an item covered by other item (for older tibia versions).
We should have a function like:

        iterator getCurrentItem() {
            return func();
        }
        const_iterator getCurrentItem() const {
            return func();
        }

So I could change in stackpos_move the function getTopDownItem to getCurrentItem:

case STACKPOS_MOVE: {
                //Item* item = tile->getTopDownItem();
                Item* item = tile->getCurrentItem();
                if (item && item->isMoveable()) {
                    thing = item;
                } else {
                    thing = tile->getTopVisibleCreature(player);
                }
                break;
            }

This happens when clientID != spriteID (when item is covered by an another one):

if (item->getClientID() != spriteId) {
        //player->sendCancelMessage(RETURNVALUE_NOTPOSSIBLE);
        //return;
    }

The GIF below shows that the Demon Shield has been moved instead of the Giant Sword, because it always moves the first thing on top from function getTopDownItem:

 

[btc]

up.
It is probably some of those weird microsoft functions from "Vector" file or something from

Thing* Tile::getThing(size_t index) const

I've noticed there is also a function:

getStackposOfItem

any ideas?