feat(Core/ItemHandler): Optional item recovery #2442

IntelligentQuantum · 2019-11-15T20:07:56Z

It is for the AzerothJS

FrancescoBorzi · 2019-11-16T10:19:38Z

hey @IntelligentQuantum thanks for the PR

It is for the AzerothJS

can you elaborate more? what features does this PR implements and in which way are they required to be in the core?

Helias · 2019-11-24T14:26:38Z

Features of this PR:

it allows to have a log about deleted items
It allows to recover items via SQL

This PR adds 3 configs:

CONFIG_ITEMDELETE_METHOD
CONFIG_ITEMELETE_QUALITY
CONFIG_ITEMELETE_ITEM_LEVEL

These configs allow to choose the method, quality and item_level of the items that could be logged in the table recovery_item.

Do we need this?
Yes, because this is an extra-feature of AC.
Besides, I don't think that this bother the blizzlike game, indeed because it's a config, so you can enable/disable.
Moreover, we can implement third-party APIs, modules or tools that work with AC.

Here a video made by @IntelligentQuantum that shows how this feature works (using the AzerothJS API).

https://mega.nz/#!pyYxlYyJ!vy6-5kGcBiTmE6xOOpbBWPuSAZvhhi2jGiFo4FXXHfM

FrancescoBorzi · 2019-11-24T22:18:10Z

thanks @Helias for the description and @IntelligentQuantum let us know when the PR is ready to be reviewed

IntelligentQuantum · 2019-11-25T05:35:05Z

Hi @FrancescoBorzi it's ready

Ali-Khazaee · 2019-11-26T07:44:24Z

As i used this feature before, There is an issue when you sell items to vendors there will be no log

MarkMBarnett · 2019-11-27T05:14:47Z

Tested successfully

src/server/worldserver/worldserver.conf.dist

src/server/game/Handlers/ItemHandler.cpp

FrancescoBorzi · 2019-12-04T18:08:49Z

@IntelligentQuantum looks good 👍

Now waiting for a tester feedback.

Helias · 2019-12-06T16:54:14Z

I'm trying to test this PR but I got the following error during the import of db characters updates with latest master revision:

ERROR 1062 (23000) at line 126: Duplicate entry '1572331570357004681' for key 'PRIMARY'

Probably you need to generate again the sql pending file using data/sql/updates/pending_db_characters/create_sql.sh after updating this PR with master.

Anyway, I imported the table recovery_item manually but the feature does not work, I don't kno why.
Probably you added a condition that does not work properly.

…414.sql

FrancescoBorzi · 2019-12-06T18:16:50Z

@Helias no, we can't merge to master if it gives such an error. I've tried to put a different id (just re-generated). Can you please give another try?

Also @IntelligentQuantum just a tip for future: never push changes to your master branch, always create a new branch for each PR you want to open. But for this time it's fine.

IntelligentQuantum · 2019-12-07T07:58:40Z

@FrancescoBorzi Kk sorry
@Helias Video

Helias · 2019-12-07T10:22:05Z

I rebuild everything and checked the worldserver.conf, probably I missed something, now it works.

BarbzYHOOL · 2019-12-10T00:43:24Z

quite interesting, don't you think it will flood the DB though? I have seen the configuration options but still

and thank you @IntelligentQuantum

Nefertumm · 2019-12-11T19:33:48Z

It would be awesome if it stores the unixtime of deletion in db, so we can implement an option for delete entries after X amount of days.

mynew · 2019-12-15T09:42:14Z

#0 0x0000000000c751ab in GetUInt32Value (index=, this=, index=) at ......../src/server/game/Entities/Object/Object.h:164
#1 GetEntry (this=) at ......../src/server/game/Entities/Object/Object.h:138
#2 WorldSession::HandleBuybackItem (this=, recvData=...) at ......../src/server/game/Handlers/ItemHandler.cpp:797
#3 0x0000000000ce55db in WorldSession::Update (this=0x7fff9d62e300, diff=, updater=...) at ......../src/server/game/Server/WorldSession.cpp:328
#4 0x0000000000c8ad81 in Map::Update (this=0x7fffc7f3a300, t_diff=0, s_diff=10) at ......../src/server/game/Maps/Map.cpp:672
#5 0x0000000000e44a99 in MapUpdateRequest::call (this=0x7fff31e40330) at ......../src/server/game/Maps/MapUpdater.cpp:57
#6 0x0000000000ea1f09 in DelayExecutor::svc (this=0x7ffff197cb68) at ......../src/common/Threading/DelayExecutor.cpp:53
#7 0x00007ffff74d56d7 in ACE_Task_Base::svc_run (args=0x7ffff197cb68) at Task.cpp:260
#8 0x00007ffff74d6dc5 in ACE_Thread_Adapter::invoke_i (this=) at Thread_Adapter.cpp:161
#9 0x00007ffff74d6d0f in ACE_Thread_Adapter::invoke (this=0x7fffbbb40d00) at Thread_Adapter.cpp:96
#10 0x00007ffff63e4184 in start_thread (arg=0x7fffb3cd2700) at pthread_create.c:312
#11 0x00007ffff56ed03d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Crash ItemHandler.cpp
stmt->setUInt32(1, pItem->GetEntry());

Nefertumm · 2019-12-15T23:22:18Z

@mynew open a new issue please.

robens · 2019-12-16T18:19:25Z

#0 0x0000000000c751ab in GetUInt32Value (index=, this=, index=) at ......../src/server/game/Entities/Object/Object.h:164
#1 GetEntry (this=) at ......../src/server/game/Entities/Object/Object.h:138
#2 WorldSession::HandleBuybackItem (this=, recvData=...) at ......../src/server/game/Handlers/ItemHandler.cpp:797
#3 0x0000000000ce55db in WorldSession::Update (this=0x7fff9d62e300, diff=, updater=...) at ......../src/server/game/Server/WorldSession.cpp:328
#4 0x0000000000c8ad81 in Map::Update (this=0x7fffc7f3a300, t_diff=0, s_diff=10) at ......../src/server/game/Maps/Map.cpp:672
#5 0x0000000000e44a99 in MapUpdateRequest::call (this=0x7fff31e40330) at ......../src/server/game/Maps/MapUpdater.cpp:57
#6 0x0000000000ea1f09 in DelayExecutor::svc (this=0x7ffff197cb68) at ......../src/common/Threading/DelayExecutor.cpp:53
#7 0x00007ffff74d56d7 in ACE_Task_Base::svc_run (args=0x7ffff197cb68) at Task.cpp:260
#8 0x00007ffff74d6dc5 in ACE_Thread_Adapter::invoke_i (this=) at Thread_Adapter.cpp:161
#9 0x00007ffff74d6d0f in ACE_Thread_Adapter::invoke (this=0x7fffbbb40d00) at Thread_Adapter.cpp:96
#10 0x00007ffff63e4184 in start_thread (arg=0x7fffb3cd2700) at pthread_create.c:312
#11 0x00007ffff56ed03d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Crash ItemHandler.cpp
stmt->setUInt32(1, pItem->GetEntry());

Just in case the issue won't be created, try moving all that new code on line ~797(ItemHandler) to the beginning of the "if" so we avoid using pItem after calling StoreItem. That or save the entry before

Nefertumm · 2019-12-16T18:25:09Z

That crash made me wonder, what if the seller bought back the item? He can claim the recovery but he has the item...

robens · 2019-12-16T18:34:52Z

That crash made me wonder, what if the seller bought back the item? He can claim the recovery but he has the item...

I think that the block of code that causes the crash is used to avoid that, it's deleting from the recovery table after buying back

Nefertumm · 2019-12-16T18:41:00Z

Oh yeah.
Also, what if the item has multiple entries? Recovering just one of them would delete all of them from the table.
This came to me just now, sorry :P

IntelligentQuantum · 2019-12-17T10:37:15Z

Hey guys

@BarbzYHOOL Actually we can flood db with CharacterRecovery && LagReport, btw we have configuration and ItemRecovery work like CharacterRecovery.

@GitVerge @Nefertumm tnx for help.

@mynew Can you test this diff please Crash.diff

Sorry my english is bad! 😄

IntelligentQuantum added 9 commits October 27, 2019 19:12

Remote

2cd4a4c

Remote.sql

64f3dea

Update

15c4994

Update

1bbd3d3

RecoveryItem Part 1

ec87016

RecoveryItem Part 2

c520a78

Fixed typo

5a6b425

Coin

bd72555

azeroth_mail.sql

9016fcb

FrancescoBorzi added the Needs feedback label Nov 16, 2019

IntelligentQuantum added 4 commits November 17, 2019 16:18

cleanup

36f50f3

Added config

602dcf8

Cleanup

48a6323

...

c0a8aae

IntelligentQuantum added 2 commits November 24, 2019 19:06

..

c6e90ec

Fixed build

8083448

FrancescoBorzi added Ready for Review and removed Needs feedback labels Nov 25, 2019

Vendor log

90672d9

BuyBack Item

c7fb90e

Helias approved these changes Nov 27, 2019

View reviewed changes

FrancescoBorzi requested changes Nov 27, 2019

View reviewed changes

src/server/worldserver/worldserver.conf.dist Show resolved Hide resolved

src/server/worldserver/worldserver.conf.dist Show resolved Hide resolved

src/server/worldserver/worldserver.conf.dist Show resolved Hide resolved

Description

2f4c4a6

FrancescoBorzi reviewed Dec 2, 2019

View reviewed changes

src/server/worldserver/worldserver.conf.dist Outdated Show resolved Hide resolved

FrancescoBorzi reviewed Dec 2, 2019

View reviewed changes

src/server/worldserver/worldserver.conf.dist Outdated Show resolved Hide resolved

FrancescoBorzi reviewed Dec 2, 2019

View reviewed changes

src/server/game/Handlers/ItemHandler.cpp Outdated Show resolved Hide resolved

? xD

f4aaae7

FrancescoBorzi approved these changes Dec 4, 2019

View reviewed changes

FrancescoBorzi removed the Ready for Review label Dec 4, 2019

Update and rename rev_1572331570357004681.sql to rev_1575656087867346…

8c4d17a

…414.sql

Helias added To Be Merged and removed Waiting to be tested labels Dec 7, 2019

ghost changed the title ~~feat(core) API~~ feat(Core/ItemHandler): Optional item recovery Dec 8, 2019

ghost merged commit b6c0f58 into azerothcore:master Dec 9, 2019

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Core/ItemHandler): Optional item recovery #2442

feat(Core/ItemHandler): Optional item recovery #2442

IntelligentQuantum commented Nov 15, 2019 •

edited

Loading

FrancescoBorzi commented Nov 16, 2019

Helias commented Nov 24, 2019

FrancescoBorzi commented Nov 24, 2019

IntelligentQuantum commented Nov 25, 2019

Ali-Khazaee commented Nov 26, 2019

MarkMBarnett commented Nov 27, 2019

FrancescoBorzi commented Dec 4, 2019

Helias commented Dec 6, 2019

FrancescoBorzi commented Dec 6, 2019

IntelligentQuantum commented Dec 7, 2019

Helias commented Dec 7, 2019

BarbzYHOOL commented Dec 10, 2019 •

edited

Loading

Nefertumm commented Dec 11, 2019

mynew commented Dec 15, 2019

Nefertumm commented Dec 15, 2019

robens commented Dec 16, 2019 •

edited

Loading

Nefertumm commented Dec 16, 2019

robens commented Dec 16, 2019 •

edited

Loading

Nefertumm commented Dec 16, 2019

IntelligentQuantum commented Dec 17, 2019 •

edited

Loading

feat(Core/ItemHandler): Optional item recovery #2442

feat(Core/ItemHandler): Optional item recovery #2442

Conversation

IntelligentQuantum commented Nov 15, 2019 • edited Loading

FrancescoBorzi commented Nov 16, 2019

Helias commented Nov 24, 2019

FrancescoBorzi commented Nov 24, 2019

IntelligentQuantum commented Nov 25, 2019

Ali-Khazaee commented Nov 26, 2019

MarkMBarnett commented Nov 27, 2019

FrancescoBorzi commented Dec 4, 2019

Helias commented Dec 6, 2019

FrancescoBorzi commented Dec 6, 2019

IntelligentQuantum commented Dec 7, 2019

Helias commented Dec 7, 2019

BarbzYHOOL commented Dec 10, 2019 • edited Loading

Nefertumm commented Dec 11, 2019

mynew commented Dec 15, 2019

Nefertumm commented Dec 15, 2019

robens commented Dec 16, 2019 • edited Loading

Nefertumm commented Dec 16, 2019

robens commented Dec 16, 2019 • edited Loading

Nefertumm commented Dec 16, 2019

IntelligentQuantum commented Dec 17, 2019 • edited Loading

IntelligentQuantum commented Nov 15, 2019 •

edited

Loading

BarbzYHOOL commented Dec 10, 2019 •

edited

Loading

robens commented Dec 16, 2019 •

edited

Loading

robens commented Dec 16, 2019 •

edited

Loading

IntelligentQuantum commented Dec 17, 2019 •

edited

Loading