Jump to content

  •  

Bug Tracker Migration

June 3rd
Good news everyone! The staff has decided that it is time to slowly kill off this Bug Tracker. We will begin the process of slowly migrating from this Bug Tracker over to our Github Issues which can be found here: https://github.com/HerculesWS/Hercules/issues

Over the next couple of days, I will be closing off any opportunity to create new reports. However, I still will keep the opportunity to reply to existing Bug Reports. Doing this will allow us to slowly fix any bug reports we have listed here so that we can easily migrate over to our Issue Tracker.

Update - June 7th 2015: Creating new bug posts has been disabled. Please use our https://github.com/HerculesWS/Hercules/issues tracker to post bugs. Users are still able to reply to existing bug posts.

- Administration

Issue Information

  • #008410

  • 4 - High

  • Fixed

Issue Confirmations

  • Yes (2)No (0)
Photo

@autotrade/@at issue

Posted by Garr on 19 October 2014 - 11:55 PM

I've stumbled upon an issue lately, and it's related to @at. After some testing here's what I found:

Issue itself:
Some items are missing from the vend after using @at.

How to reproduce:
Log on a character with cart, empty cart, relog, fill it with items that you'll be vending, set up vend, @at.

Result:
Some (or even all, which will result in trader going offline) items will go missing. Well, only new items that you'll put after relog will go missing.

Extra info:
Downloaded latest Hercules from git, compiled it with MSVC2010 to check if it's reproducable. It is.

Screens:
Posted Image
Posted Image
Posted Image

Some in-depth view:
Problem lies in autotrade_populate.
		for(j = 0; j < MAX_CART; j++) {
			if( !memcmp((char*)(&data->list[i]) + sizeof(data->list[0].id), (char*)(&sd->status.cart[j]) + sizeof(data->list[0].id), sizeof(struct item) - sizeof(data->list[0].id)) ) {
				if( cursor ) {
					ARR_FIND(0, cursor, k, sd->vending[k].index == j);
					if( k != cursor )
						continue;
				}
				break;
			}
		}
Specifically, in this line:
!memcmp((char*)(&data->list[i]) + sizeof(data->list[0].id), (char*)(&sd->status.cart[j]) + sizeof(data->list[0].id), sizeof(struct item) - sizeof(data->list[0].id))
If you don't relog it fails comparison for new items added to cart.

Please check.

I experience this for long time and keep ignoring it, also sometimes when you @autotrade/@at you just get logged out*

Actually, when you just get logged out (aka vend doesn't appear), it's the result of same error. It actually DOES create at vendor (you can check it with logs), but due to it not finding ANY items, vend_num is zeroed, so the vend goes offline.

Okay now i understand it so the main problem is the missing items when vending, and sometimes it does not vend at all thats why the vendors gone hope they'll fix it asap.

Yes is quite old issue i also experience, this should get fixed,..

More info: Everything seems to be in order. Out of curiousity I compared every data->list item with sd->status.cart items through logs, and after repeating the steps above I found complete matches in both, but they did not trigger the !memcmp.

Edited by Garr, 20 October 2014 - 09:14 PM.


Okay, playing with it a bit further, here are more findings:
Memory copy is taken from the autotrade_prepare, where it copies cart items into data:
ea 02 00 00 
72 33 
01 00 
00 00 00 00 
01 
00 
00 b7 - attribute
00 00 
00 00 
00 00 
00 00 
00 00 00 00 
00 
00 e3 09 - bound
81 00 00 00 f2 49 02 00

New lines are taken from adresses shown by logs, which, as it can be seen, makes attribute and bound take 2 and 3 bytes respectively, even though in the struct it's char and unsigned char respectively, so should be 1 byte?
sd->status.cart[0].id adress = 356489240
sd->status.cart[0].nameid adress = 356489244
sd->status.cart[0].amount adress = 356489246
sd->status.cart[0].equip adress = 356489248
sd->status.cart[0].identify adress = 356489252
sd->status.cart[0].refine adress = 356489253
sd->status.cart[0].attribute adress = 356489254
sd->status.cart[0].card[0] adress = 356489256
sd->status.cart[0].card[1] adress = 356489258
sd->status.cart[0].card[2] adress = 356489260
sd->status.cart[0].card[3] adress = 356489262
sd->status.cart[0].expire_time adress = 356489264
sd->status.cart[0].favorite adress = 356489268
sd->status.cart[0].bound adress = 356489269
sd->status.cart[0].unique_id adress = 356489272
So, these extra bytes are what's causing memcmp to fail, as seen below:
mem1(data) =
ea 02 00 00
72 33
01 00
00 00 00 00
01
00
00 b7 - attribute
00 00
00 00
00 00
00 00
00 00 00 00
00
00 e3 09 - bound
81 00 00 00 f2 49 02 00

mem2(cart) =
8d 00 00 00
72 33
01 00
00 00 00 00
01
00
00 00 - attribute
00 00
00 00
00 00
00 00
00 00 00 00
00
00 00 00 - bound
81 00 00 00 f2 49 02 00


Edited by Garr, 23 October 2014 - 08:42 AM.


changed severity to: 4 - High

You can as well change it to fixed. Solution: src/char/char.c:
 int mmo_char_fromsql(int char_id, struct mmo_charstatus* p, bool load_everything) {
...
//read inventory
	//`inventory` (`id`,`char_id`, `nameid`, `amount`, `equip`, `identify`, `refine`, `attribute`, `card0`, `card1`, `card2`, `card3`, `expire_time`, `favorite`, `bound`, `unique_id`)
 	StrBuf->Init(&buf);
	StrBuf->AppendStr(&buf, "SELECT `id`, `nameid`, `amount`, `equip`, `identify`, `refine`, `attribute`, `expire_time`, `favorite`, `bound`, `unique_id`");
 	for( i = 0; i < MAX_SLOTS; ++i )
		StrBuf->Printf(&buf, ", `card%d`", i);
	StrBuf->Printf(&buf, " FROM `%s` WHERE `char_id`=? LIMIT %d", inventory_db, MAX_INVENTORY);
	
	if( SQL_ERROR == SQL->StmtPrepareStr(stmt, StrBuf->Value(&buf))
	||	SQL_ERROR == SQL->StmtBindParam(stmt, 0, SQLDT_INT, &char_id, 0)
	||	SQL_ERROR == SQL->StmtExecute(stmt)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  0, SQLDT_INT,       &tmp_item.id, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  1, SQLDT_SHORT,     &tmp_item.nameid, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  2, SQLDT_SHORT,     &tmp_item.amount, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  3, SQLDT_UINT,      &tmp_item.equip, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  4, SQLDT_CHAR,      &tmp_item.identify, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  5, SQLDT_CHAR,      &tmp_item.refine, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  6, SQLDT_CHAR,      &tmp_item.attribute, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  7, SQLDT_UINT,      &tmp_item.expire_time, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  8, SQLDT_CHAR,      &tmp_item.favorite, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  9, SQLDT_UCHAR,     &tmp_item.bound, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt, 10, SQLDT_UINT64,    &tmp_item.unique_id, 0, NULL, NULL) )
...
}
Add in memset(&tmp_item, 0, sizeof(tmp_item)); before if with sql errors, so it goes like:
//read inventory
	//`inventory` (`id`,`char_id`, `nameid`, `amount`, `equip`, `identify`, `refine`, `attribute`, `card0`, `card1`, `card2`, `card3`, `expire_time`, `favorite`, `bound`, `unique_id`)
 	StrBuf->Init(&buf);
	StrBuf->AppendStr(&buf, "SELECT `id`, `nameid`, `amount`, `equip`, `identify`, `refine`, `attribute`, `expire_time`, `favorite`, `bound`, `unique_id`");
 	for( i = 0; i < MAX_SLOTS; ++i )
		StrBuf->Printf(&buf, ", `card%d`", i);
	StrBuf->Printf(&buf, " FROM `%s` WHERE `char_id`=? LIMIT %d", inventory_db, MAX_INVENTORY);
	
	memset(&tmp_item, 0, sizeof(tmp_item));
	if( SQL_ERROR == SQL->StmtPrepareStr(stmt, StrBuf->Value(&buf))
	||	SQL_ERROR == SQL->StmtBindParam(stmt, 0, SQLDT_INT, &char_id, 0)
	||	SQL_ERROR == SQL->StmtExecute(stmt)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  0, SQLDT_INT,       &tmp_item.id, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  1, SQLDT_SHORT,     &tmp_item.nameid, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  2, SQLDT_SHORT,     &tmp_item.amount, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  3, SQLDT_UINT,      &tmp_item.equip, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  4, SQLDT_CHAR,      &tmp_item.identify, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  5, SQLDT_CHAR,      &tmp_item.refine, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  6, SQLDT_CHAR,      &tmp_item.attribute, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  7, SQLDT_UINT,      &tmp_item.expire_time, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  8, SQLDT_CHAR,      &tmp_item.favorite, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt,  9, SQLDT_UCHAR,     &tmp_item.bound, 0, NULL, NULL)
	||	SQL_ERROR == SQL->StmtBindColumn(stmt, 10, SQLDT_UINT64,    &tmp_item.unique_id, 0, NULL, NULL) )
That memset was there for cart, but wasn't there for inventory loading, so every item restored to inventory had a garbage data remaining.

changed status to: Confirmed

changed status to: Fixed