Forgottenserver: onMoveItem wrong behaviour

Created on 20 Jan 2017  路  6Comments  路  Source: otland/forgottenserver

Before creating an issue, please ensure:

  • [ x] This is a bug in the software that resides in this repository, and not a
    support matter (use https://otland.net/forums/support.16/ for support)
  • [ x] This issue is reproducible without changes to the code in this repository

Steps to reproduce (include any configuration/script required to reproduce)

  1. Equip a Backpack in inventory
  2. Move one item from inside the backpack to the inventory slot where the backpack is
  3. toCylinder returns the player instead of the backpack

Expected behaviour

toCylinder should be equal to backpack metatable

Actual behaviour

toCylinder is equal to player metatable

Environment

Latest source TFS 1.3

Most helpful comment

This is debatable.

It makes sense that, if you move an item to a slot, toCylinder should be the parent of that slot. Which, in this case, is the player. It also makes sense that toCylinder should be the parent cylinder of where the item ends it's move action.

Imagine you want to move a sword to a hand where there is a shield. You want toCylinder to be the hand slot or inside the shield?

This isn't a good argument because a shield will probably never be a cylinder, while a backpack usually is.

All 6 comments

I don't think this is actually a wrong behaviour. The toCylinder is accurate in this situation cause the player is moving it to the slot not to the backpack. (So the toCylinder is the player not the backpack.)

If you have to get the backpack you should use the toPosition.y, that will give you the slot that is being moved if its >= CONST_SLOT_FIRST and <= CONST_SLOT_LAST. (You have to do that check cause otherwise its => 64 for containers).

Yes i've already fixed my problem using position, but in my opinion it is a wrong behavior, because toCylinder should be the player only when the slot is empty otherwise the backpack

Imagine you want to move a sword to a hand where there is a shield. You want toCylinder to be the hand slot or inside the shield?

Now imagine the same, instead of a shield, a backpack. The server is just being consistent when it returns the slot instead of the container.

This is debatable.

It makes sense that, if you move an item to a slot, toCylinder should be the parent of that slot. Which, in this case, is the player. It also makes sense that toCylinder should be the parent cylinder of where the item ends it's move action.

Imagine you want to move a sword to a hand where there is a shield. You want toCylinder to be the hand slot or inside the shield?

This isn't a good argument because a shield will probably never be a cylinder, while a backpack usually is.

Therefore you could use getParent in the backpack to retrieve the player, so you would just need to make a simple check if toCylinder it's either a container or a player.

So I guess it works as designed. Closing.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

EPuncker picture EPuncker  路  5Comments

EPuncker picture EPuncker  路  4Comments

EPuncker picture EPuncker  路  3Comments

MillhioreBT picture MillhioreBT  路  4Comments

GoularPink picture GoularPink  路  4Comments