NPC Trading

[kaldec]

I've been trying to work out some kinks I've had with NPC trading. The first problem I had was getting a bunch of "GetItemAt(X)" messages spamming my chat window. I looked through the code and found this in client.cpp:

bool Client::MoveItem(int16 to_slot, int16 from_slot, uint8 quantity){
    	if ( to_slot >=3000 && to_slot <= 3016){// Item Trade

			TradeList[to_slot-3000] = GetItemAt(0);

			TradeCharges[to_slot-3000] = pp.invitemproperties[0].charges;

////////////////////////////////////////////////////////////////
			for (int j = 0;j != 10;j++) {

				TradeList[((to_slot-2999)*10)+j] = GetItemAt(80+j);

				TradeCharges[((to_slot-2999)*10)+j] = GetItemPropAt(80+j).charges;

			}
////////////////////////////////////////////////////////////////

I can't make sense of the portion of the code between the comments, it is what generates the spam. The GetItemAt call must fail as 80-89 are invalid item slots for a player profile, and all of these calls will spit out the error and return 0. It seems that either this chunk should be removed, or the values set to 0 directly, or the error message removed from Client::GetItemAt.

I've noticed that at least one server doesn't have this problem, so I'm not sure if something is dorked in my setup or if that admin has altered the code.

I'm also seeing a problem with items traded to NPCs remaining tied to the cursor so that they reapear if I camp and come back. While this is great for quests, it's not ideal. Hopefully I can figure this out by comparing the code for selling which seems to remove the item okay.

[Bigpull]

Ok first off if you ever come across an "#if 0" or "#if 1" Odds are it's me reimplenting something for whatever reason struck me (didn't make sense, didn't work, just plain felt like it..)

Onto OP_MoveItem, theres a bug in the client_proccess file with cursored items. The trade code in MoveItem() is a direct copy and paste from this original code, including all it's problems.

    	if ( to_slot >=3000 && to_slot <= 3016){// Item Trade
			TradeList[to_slot-3000] = GetItemAt(0); // Put item number from cursor into Tradelist
			TradeCharges[to_slot-3000] = pp.invitemproperties[0].charges; // Get charge count from cursor into TradeList
			// Missing bag flag check?
			for (int j = 0;j != 10;j++) { // Process bag items into out half of trade window
				TradeList[((to_slot-2999)*10)+j] = GetItemAt(250+j); // Item number from cursor bag to Tradelist
				TradeCharges[((to_slot-2999)*10)+j] = GetItemPropAt(250+j).charges; // Item charge from cursor bag to TradeList
			}

[kaldec]

Thanks for the response! I can make sense of having an offset of 250 instead of 80, I would assume it's to account for placing a container in the trade window. I'll look more closely at the trade code, to be sure, but it seems like this should be changed back to point into the container space.