1. Hercules bug tracker archive
  2. Hercules Bug Archives
  3. Core
  4. @autotrade/@at issue

@autotrade/@at issue

Issue information

Issue ID
#8410
Status
Fixed
Severity
High
Started
Garr
Oct 19, 2014 23:55
Last Post
Haru
Oct 24, 2014 22:48
Confirmation
Yes (2)
No (0)

Garr - Oct 19, 2014 23:55

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

[b]Issue itself:[/b]
Some items are missing from the vend after using @at.

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

[b]Result:[/b]
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.

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

[b]Screens:[/b]
[img]http://i59.tinypic.com/219ry1g.jpg[/img]
[img]http://i62.tinypic.com/2yyt9xt.jpg[/img]
[img]http://i61.tinypic.com/2il0o43.jpg[/img]

[b]Some in-depth view:[/b]
Problem lies in autotrade_populate.[code=auto:0] 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; } } [/code]
Specifically, in this line:[code=auto:0] !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)) [/code]
If you don't relog it fails comparison for new items added to cart.

Please check.

This post has been edited by Garr on Oct 19, 2014 23:57

Zer - Oct 20, 2014 7:38

[size=3][font=tahoma, geneva, sans-serif]I experience this for long time and keep ignoring it, also sometimes when you @autotrade/@at you just get logged out*[/font][/size]

Garr - Oct 20, 2014 8:04

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.

Zer - Oct 20, 2014 8:06

[size=3][font=tahoma, geneva, sans-serif]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.[/font][/size]

johnyboy - Oct 20, 2014 13:48

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

Garr - Oct 20, 2014 21:07

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.

This post has been edited by Garr on Oct 20, 2014 21:14

Garr - Oct 23, 2014 8:38

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:[code=auto:0] 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 [/code]
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?[code=auto:0] 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 [/code]
So, these extra bytes are what's causing memcmp to fail, as seen below:[code=auto:0] 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 [/code]

This post has been edited by Garr on Oct 23, 2014 8:42

Garr - Oct 23, 2014 19:34

You can as well change it to fixed. Solution: src/char/char.c:[code=auto:0] 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) ) ... } [/code]
Add in memset(&tmp_item, 0, sizeof(tmp_item)); before if with sql errors, so it goes like:[code=auto:0] //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) ) [/code]
That memset was there for cart, but wasn't there for inventory loading, so every item restored to inventory had a garbage data remaining.

Haru - Oct 24, 2014 22:47

Thank you very much, fixed in [url="https://github.com/HerculesWS/Hercules/commit/3e1fe0d3842aab1c85f4dfd8e3533ca6631fc4e5"]https://github.com/HerculesWS/Hercules/commit/3e1fe0d3842aab1c85f4dfd8e3533ca6631fc4e5[/url]