[Mangos Zero] Segfault calling Player::CanStoreNewItem against offline Player instance

[mangolassi]

Hello and hope you're all having a great holiday season!

I'm attempting to write a HandleCharacterAddItemCommand method in Level2.cpp which is capable of adding items to characters not currently logged in (accessible via CLI/SOAP).

In order to do this, I added an argument before the HItem/link/id is specified which takes the character name and attempts to fetch the appropriate instance of Player:

char* nameStr = ExtractOptNotLastArg(&args);

Player* plTarget;
ObjectGuid target_guid;
std::string target_name;
if (!ExtractPlayerTarget(&nameStr, &plTarget, &target_guid, &target_name))
{
   return false;
}

The rest of the method works nearly identically to the HandleAddItemCommand method in Level3.cpp.

problem:

When this line is executed, a segfault occurs:

uint8 msg = plTarget->CanStoreNewItem(NULL_BAG, NULL_SLOT, dest, itemId, count, &noSpaceForCount);

So I looked at the code for CanStoreNewItem in Player.h:

InventoryResult CanStoreNewItem(uint8 bag, uint8 slot, ItemPosCountVec& dest, uint32 item, uint32 count, uint32* no_space_count = NULL) const
{
   return _CanStoreItem(bag, slot, dest, item, count, NULL, false, no_space_count);
}

So then it's back to Player.cpp to look at "InventoryResult Player::_CanStoreItem(uint8 bag, uint8 slot, ItemPosCountVec& dest, uint32 entry, uint32 count, Item* pItem, bool swap, uint32* no_space_count) const". Near the bottom of the method, the crash is happening when the call to _CanStoreItem_InInventorySlots is made:

res = _CanStoreItem_InInventorySlots(INVENTORY_SLOT_ITEM_START, INVENTORY_SLOT_ITEM_END, dest, pProto, count, false, pItem, bag, slot);

So I look there near the top of the method and find the call to GetItemByPos where the crash seems to originate from:

Item* pItem2 = GetItemByPos(INVENTORY_SLOT_BAG_0, j);

So I look in GetItemByPos:

GetItemByPos segfaults when attempting to access m_items[slot] in the following code (conditions within if statement evaluate to true and the "return m_items[slot]" line is where the segfault occurs:

if (bag == INVENTORY_SLOT_BAG_0 && (slot < BANK_SLOT_BAG_END || (slot >= KEYRING_SLOT_START && slot < KEYRING_SLOT_END)))
{
   return m_items[slot];
}

Have I missed something while trying to fetch the player instance? Is it simply impossible to check player inventory when the player is not logged in? If not, is there a reasonable way to fake the player login for this purpose?

Thanks!

EDIT: This is based on the develop21 branch.

[mangolassi]

I believe the player object is already instantiated from a not logged in player via the ExtractPlayerTarget function, and I'm not familiar with the SMSG_xxx opcodes nor for what purpose they're passed around--if you mean that the opcodes are supposed to be passed to players (in some fashion), is it not possible to omit sending the opcode to players not logged in?

I wanted to avoid direct operations against the database in order to prevent unexpected behavior if parts of mangoszero are later modified upstream (I forked the project but would like to periodically merge upstream changes back in).

Thanks for the tip on the mail system, as it looks like it will provide the result I'm looking for. I'll give it a shot!

Also on a somewhat unrelated note, I wrote an ugly implementation of CLI/SOAP accessible character creation (https://github.com/mangoslassi/server/commit/fb97d3cab222ada089373ef86f02fb52d5018ccf). Would you know of a more "official" way to handle this without the modified code or is it best to stick with what I've written?

Thanks for all of your help!

[Olion]

I believe the player object is already instantiated from a not logged in player via the ExtractPlayerTarget function

No, AFAIK it become instantiated at character selection from the select character screen. Moreover, it should not be instantiated due to a lot of possible unwanted side effects like visible appearance of the character ingame.

The standard way to solve your task is sending the mail to the player with that item attached. I suppose such command is implemented in the TC.

Your way requires direct interaction with the DB with much more transactions than just sending mail and checking the method return state (IIRC it is possible).

I wanted to avoid direct operations against the database

It is correct idea (scripts, including commandscripts, must not use the DB directly), but that direct operations on the DB will be done by another core part for your task. The question is in the minimizing transaction number only.

[H0zen]

Let's assume you'll find a way to instantiate a player object from a not logged in player. Eventually, when you add the item, the server has to send a SMSG_xxx opcode to the...who? instead of torturing yourself, isn't easier to just add the item in the caracter database, or use the mail sistem?