[3.3.5] Core/AuctionHouse: Auction Bidder Table #18328

iridinite · 2016-11-27T11:50:15Z

Changes proposed:

Save a list of all characters who have placed a bid on an auction, to the characters database. When client requests list of auctions (s)he is involved in, use this table, instead of only returning auctions where that character is the current highest bidder.
This prevents auctions that you placed a bid on earlier from disappearing from the Bids tab if you are outbid and you relog or the server is restarted.

Target branch(es):

3.3.5
(it might also apply to master, I have not tested this)

Todo:

Could someone please help me understand under what circumstances the client requests certain auction IDs in WorldSession::HandleAuctionListBidderItems and if it matters if we just ignore that entirely? I mean, using this bid table system, it's completely irrelevant if the client happens to remember a handful of auction IDs.

Tests performed:
Built and tested in game.

Shauren · 2016-11-27T11:52:45Z

is this how it works on retail?

Shauren · 2016-11-27T11:54:32Z

src/server/game/AuctionHouse/AuctionHouseMgr.cpp

+    PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_SEL_AUCTION_BIDDERS);
+    stmt->setUInt32(0, Id);
+
+    if (PreparedQueryResult result = CharacterDatabase.Query(stmt))


this is going to be terribly slow if you launch a separate query for every single auction during startup

Would it be a better idea to load all of the table at once and keep it around in memory until all auctions are loaded?

All auctions are loaded on startup with one query in AuctionHouseMgr::LoadAuctions, just run another query (for the entire table) there and parse it before the loop calling AuctionEntry::LoadFromDB

iridinite · 2016-11-27T11:54:45Z

To be fair, I can't say with 100% certainty this is retail. But it seems, to me at least, like an oversight.

Shauren · 2016-11-27T13:00:36Z

src/server/game/AuctionHouse/AuctionHouseMgr.h

@@ -85,6 +85,7 @@ struct TC_GAME_API AuctionEntry
    ObjectGuid::LowType bidder;
    uint32 deposit;                                         //deposit can be calculated only when creating auction
    uint32 etime;
+    std::unordered_set<uint32> bidderlist;


if this is meant to be a guid, use ObjectGuid::LowType, never the underlying type (master branch portability issue, its a different type there)

Fixed, thanks

iridinite · 2016-11-27T20:58:26Z

It seems like the Travis build breaks on the SQL patch. I can't quite make sense of the error, what's going on there?

Aokromes · 2016-11-27T23:51:56Z

It's not your issue, the guy who merges the patch must rename the file and add it to updates.

Shauren · 2016-11-29T11:05:56Z

src/server/game/AuctionHouse/AuctionHouseMgr.cpp

@@ -373,13 +396,17 @@ void AuctionHouseMgr::LoadAuctions()
            continue;
        }

+        auto it = bidderlists.find(aItem->Id);
+        if (it != bidderlists.end())
+            aItem->bidderlist = std::unordered_set<ObjectGuid::LowType>(it->second);


for extra performance&fun, std::move it->second

Shauren · 2016-11-29T11:07:15Z

src/server/game/AuctionHouse/AuctionHouseMgr.cpp


+            auto it = bidderlists.find(auctionId);


not needed, operator[] creates entries in map if not present.
just bidderlists[fields[0].GetUInt32()].insert(fields[1].GetUInt32());

iridinite · 2016-11-29T20:36:16Z

@Shauren Done, thanks for your comments!

iridinite · 2016-12-26T22:32:47Z

Any comments?

Faq · 2017-11-12T14:06:24Z

@joschiwald could you please check?

joschiwald · 2017-11-12T14:39:54Z

src/server/game/AuctionHouse/AuctionHouseMgr.cpp

@@ -845,6 +865,10 @@ void AuctionEntry::DeleteFromDB(SQLTransaction& trans) const
    PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_AUCTION);
    stmt->setUInt32(0, Id);
    trans->Append(stmt);
+
+    PreparedStatement* stmt2 = CharacterDatabase.GetPreparedStatement(CHAR_DEL_AUCTION_BIDDERS);


you can reuse stmt

ghost · 2017-11-12T14:41:18Z

The branch iridinite:3.3.5-bidders needs to be updated/rebased or at least .travis.yml needs to be updated to latest version (bf187a1) in order for travis-ci to work on this PR.

joschiwald · 2017-11-12T14:41:29Z

src/server/game/Handlers/AuctionHouseHandler.cpp

+        PreparedStatement* stmt2 = CharacterDatabase.GetPreparedStatement(CHAR_INS_AUCTION_BIDDERS);
+        stmt2->setUInt32(0, auction->Id);
+        stmt2->setUInt32(1, auction->bidder);
+        trans->Append(stmt2);


you only need to fire this query if bidder was not present in bidderlist before

joschiwald · 2017-11-12T14:43:30Z

sql/base/characters_database.sql

+DROP TABLE IF EXISTS `auctionbidders`;
+CREATE TABLE `auctionbidders` (
+	`id` INT(10) UNSIGNED NOT NULL DEFAULT '0',
+	`bidder` INT(10) UNSIGNED NOT NULL DEFAULT '0',


we don't like tabs and better name will be BidderGuid

iridinite · 2017-11-12T15:12:44Z

Well I can't say I was expecting this PR to be dug up again. Thanks for the comments, I'll reinstall the tools and see what I can do.

Is there an easy way to rebase this PR on HEAD of 3.3.5 without screwing up the version history? Am a bit of a git noob

ghost · 2017-11-12T15:18:21Z

To update your branch 3.3.5-bidders with the 3.3.5 TC branch, make sure you choose 'rebase' to base your changes on top of the merge.

I only use Git Extensions, so I use the GUI approach. There is of course a git command for doing this, but I would prefer if someone doing it regularly could advise you on what it looks like in your case, if you use git on the command line.

ghost · 2017-11-12T15:52:07Z

BTW, you may want to include an update in the sql/base/characters_database.sql file if you want to see a full green result from travis-ci.

Example, insert the following at the end of line 2571 of your sql/base/characters_database.sql file:

,('9999_99_99_18328_characters.sql','B978AC2CF95DDFC71E640362578295FA2D63AF76','RELEASED','2017-11-12 16:56:23',0)

before the ending semicolon (;).

That content will have to be changed again when merging due to different file name and date/time info.

r00ty-tc · 2017-11-12T15:58:02Z

The way I do it is, something like this:

Do this once for the local repository. If you clone a new one you must do it again.

git remote add TrinityCore https://github.com/TrinityCore/TrinityCore

Then every time you want to update

git fetch TrinityCore
git rebase TrinityCore/3.3.5

Of course, if it cannot auto merge your changes on top of the current latest version it will list conflicts, which you need to fix. Once you're sure all are fixed (after fixing each file you need to add with git add) you can then enter (only if there were conflicts)

git rebase --continue

If you did a good job of fixing conflicts it will complete and you'll have a repo in the state that your commits appear after the latest commits from the TrinityCore 3.3.5 branch.

Now you should ensure that it compiles and works as before. If so, then you must force push to your own github repo with git push -f. The reason to force it is because you're rewriting the history of your repo by rebasing.

Now, the way to do it without rewriting history is git merge. But, for the purposes of updating a PR I think it's better to rebase.

This will cause a spam of text in our IRC channel for each commit.. :)

iridinite · 2017-11-12T16:00:21Z

@r00ty-tc "This will cause a spam of text in our IRC channel for each commit.. :)"
I assume you don't mind that, then, because I was just about to start rebasing :P

r00ty-tc · 2017-11-12T16:01:19Z

Well I personally think the bot should mute/summarize these multiple commits that happen in a short period. But, that's not your problem :)

because that seems less expensive than dereferencing and parsing the value three times

r00ty-tc · 2017-11-12T16:09:03Z

Actually looks like I was wrong. Just a single line sent. I think it's because I was working on a branch on TrinityCore itself and that logs each commit. So, all good.

iridinite · 2017-11-12T16:10:36Z

Neat-o. I'll look into the requested changes!

iridinite · 2017-11-12T16:20:37Z

@tkrokli What hash algorithm is used for the characters.updates table? Doesn't seem like throwing SHA1 at the file gets me the same value as in your comment.

Aokromes · 2017-11-12T16:41:24Z

md5

r00ty-tc · 2017-11-12T16:42:13Z

It is sha1sum, but if you're on windows it will give the wrong result. Try using dos2unix (if you have git bash it will be there I think) or similar to convert to unix text file and then do sha1sum and the value should then be correct.

ghost · 2017-11-12T16:42:36Z

Sorry, I generated a SHA1 hash because that was what came up when I checked the DB table info for the updates table. If you are able to generate a hash locally from the auto import of the file, use yours.

ghost · 2017-11-12T16:48:35Z

To look up your local hash, run this query in mysql or your SQL DB editor:

SELECT * FROM `characters`.`updates` WHERE `name`= '9999_99_99_18328_characters.sql';

If you use a different name for your characters table, remove/replace that part in my suggested query.

Tested in-game

iridinite · 2017-11-27T19:13:12Z

Come on, gents, don't leave me hanging again. If you think this is a shitty patch, just tell me, then at least I know which way is up.

….sql - hoping to trigger a successful build in AppVeyor this time

Shauren · 2017-11-27T20:51:13Z

No, this is actually pretty good

ghost · 2017-11-27T21:38:27Z

Looks good this time around, travis-ci has accepted the updated sql/base/characters_database.sql file (and all the other SQL parts).

Shauren reviewed Nov 27, 2016

View reviewed changes

Aokromes added Branch-3.3.5a Comp-Core Feedback-PatchFix labels Nov 27, 2016

Shauren requested changes Nov 29, 2016

View reviewed changes

Shauren reviewed Nov 29, 2016

View reviewed changes

Aokromes added Feedback-FixOutdatedMissingWIP Feedback-PatchFix and removed Feedback-PatchFix Feedback-FixOutdatedMissingWIP labels Oct 22, 2017

joschiwald reviewed Nov 12, 2017

View reviewed changes

iridinite added 2 commits November 12, 2017 17:06

Add db table for bidder lists

fed30b3

Add SQL queries for manipulating auctionbidders

8b195e4

iridinite added 9 commits November 12, 2017 17:06

Implement bidder lists

22fecdf

Remove unimplemented GetBidsMap declaration

c9b8bd7

Remove collate from sql update

8505bf1

Register existing auction bidders in sql update

cbadd90

Rename sql update with PR number

f8aa800

Change list type into ObjectGuid::LowType

a9a0c24

Load all bidder lists with one query

747522e

Access auction id in query result only once

e6e6966

because that seems less expensive than dereferencing and parsing the value three times

Cleanup / optimizations in LoadAuctions

6601805

iridinite force-pushed the 3.3.5-bidders branch from ed61003 to 6601805 Compare November 12, 2017 16:07

Make requested changes

e9675d8

Tested in-game

ghost changed the title ~~[3.3.5] Auction Bidder Table~~ [3.3.5] Core/AuctionHouse: Auction Bidder Table Nov 12, 2017

ghost added the Sub-AH label Nov 12, 2017

Rename 9999_99_99_18328_characters.sql to 2017_99_99_18328_characters…

4f1ef16

….sql - hoping to trigger a successful build in AppVeyor this time

Shauren approved these changes Nov 27, 2017

View reviewed changes

Prepare merge

b1d0138

Shauren merged commit 9d454eb into TrinityCore:3.3.5 Nov 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3.3.5] Core/AuctionHouse: Auction Bidder Table #18328

[3.3.5] Core/AuctionHouse: Auction Bidder Table #18328

iridinite commented Nov 27, 2016 •

edited

Shauren commented Nov 27, 2016

Shauren Nov 27, 2016

iridinite Nov 27, 2016

Shauren Nov 27, 2016

iridinite commented Nov 27, 2016 •

edited

Shauren Nov 27, 2016

iridinite Nov 27, 2016

iridinite commented Nov 27, 2016

Aokromes commented Nov 27, 2016

Shauren Nov 29, 2016

Shauren Nov 29, 2016 •

edited

iridinite commented Nov 29, 2016

iridinite commented Dec 26, 2016

Faq commented Nov 12, 2017

joschiwald Nov 12, 2017

ghost commented Nov 12, 2017

joschiwald Nov 12, 2017

joschiwald Nov 12, 2017

iridinite commented Nov 12, 2017

ghost commented Nov 12, 2017 •

edited by ghost

ghost commented Nov 12, 2017 •

edited by ghost

r00ty-tc commented Nov 12, 2017

iridinite commented Nov 12, 2017

r00ty-tc commented Nov 12, 2017

r00ty-tc commented Nov 12, 2017

iridinite commented Nov 12, 2017

iridinite commented Nov 12, 2017

Aokromes commented Nov 12, 2017

r00ty-tc commented Nov 12, 2017

ghost commented Nov 12, 2017

ghost commented Nov 12, 2017

iridinite commented Nov 27, 2017

Shauren commented Nov 27, 2017

ghost commented Nov 27, 2017

[3.3.5] Core/AuctionHouse: Auction Bidder Table #18328

[3.3.5] Core/AuctionHouse: Auction Bidder Table #18328

Conversation

iridinite commented Nov 27, 2016 • edited

Shauren commented Nov 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iridinite commented Nov 27, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iridinite commented Nov 27, 2016

Aokromes commented Nov 27, 2016

Choose a reason for hiding this comment

Shauren Nov 29, 2016 • edited

Choose a reason for hiding this comment

iridinite commented Nov 29, 2016

iridinite commented Dec 26, 2016

Faq commented Nov 12, 2017

Choose a reason for hiding this comment

ghost commented Nov 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iridinite commented Nov 12, 2017

ghost commented Nov 12, 2017 • edited by ghost

ghost commented Nov 12, 2017 • edited by ghost

r00ty-tc commented Nov 12, 2017

iridinite commented Nov 12, 2017

r00ty-tc commented Nov 12, 2017

r00ty-tc commented Nov 12, 2017

iridinite commented Nov 12, 2017

iridinite commented Nov 12, 2017

Aokromes commented Nov 12, 2017

r00ty-tc commented Nov 12, 2017

ghost commented Nov 12, 2017

ghost commented Nov 12, 2017

iridinite commented Nov 27, 2017

Shauren commented Nov 27, 2017

ghost commented Nov 27, 2017

iridinite commented Nov 27, 2016 •

edited

iridinite commented Nov 27, 2016 •

edited

Shauren Nov 29, 2016 •

edited

ghost commented Nov 12, 2017 •

edited by ghost

ghost commented Nov 12, 2017 •

edited by ghost