Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor AuctionHouseBot.cpp + small bugfix #117

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

PixelWeaver
Copy link

@PixelWeaver PixelWeaver commented Nov 2, 2024

Changes Proposed:

  • Renaming of the BuyPrice config members into something more meaningful to help with readability
  • Refactor in AuctionHouseBot.cpp for readability
  • Refactor and bugfix in AuctionHouseBot::Buy (make variable names more meaningful, simplify some if/else, move some checks to a little earlier when relevant, most importantly, properly remove auction from auctionPool: erase takes an iterator argument when you want to delete an element at a specific position, giving it a uint32 will make it look for that value in the set).

Issues Addressed:

  • Fixes an issue where the bot sometimes bids several times the above ceiling value.

Tests Performed:

  • Builds without error
  • Doesn't purchase above the ceiling value anymore

How to Test the Changes:

  1. Sell 20 green items with a sell price.
  2. For each of them, set the bidding price at exactly the ceiling value minus 1 copper (sell_price_in_copper * 5 - 1), do not put a buyout price.
  3. Do this with and without the changes in this branch: with the changes, you'll never have more than the ceiling value, without the changes you'll experience the issue where the bot bids sometimes above twice the ceiling value.

@PixelWeaver PixelWeaver changed the title Refactor auction house bot cpp Refactor AuctionHouseBot.cpp + small bugfix Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant