Issue Information
-
#008410
-
4 - High
-
Fixed
Issue Confirmations
-
Yes (2)No (0)
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:
Some in-depth view:
Problem lies in autotrade_populate.
Please check.
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:
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:
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 00New 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 = 356489272So, 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