-
Notifications
You must be signed in to change notification settings - Fork 104
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
feat: profession items #111
Conversation
Holds all items which are used for professions or created by them. Exported from Spell.dbc
Hi there. Some of those items have buy or sell prices set to zero, especially enchanting reagents. How have you addressed this issue? Some of such items does not have any price related the sell or buy variable, meaning that they will be ignored by the current bot. Right now I have requested a pull to resolve this issue here. For what I see you have put the items ID in a table and side loading them into the bot items bins, but this does not solve the problem of selling items set at zero. Do you see the dust reagents obtained by disenchanting the items, for example? If I'm not wrong lot of these items are already put into the bin if you use the |
Hi, Before I made these changes, I just took some items, that i was missing on the ah and searched for them in all linked tables... and found nothing. (Maybe a bad coincidence, when you say, that many of them are also in VendorTradeGoods, LootTradeGoods and OtherTradeGoods). So I decided to create a new table with all relevant ids. And for many things it's working perfectly. Except for the ones without a sell/ buy price. I'm only hosting a little server for some friends, and for now they're happy with these changes. But it's not perfect. Can you maybe test my table with your changes? In theory that should bring all relevant items to the ah. |
You did a good job isolating the items necessary for professions. Probably what you will need here is a way to make the current setup backward compatible; there can be setup that does not want the bots to trade in profession items because they want players to engage in their exchange. If I may provide a comment about the current implementation, is that probably it need a configuration set to zero by default to provide profession items. You can setup an additional option, And organize the configuration loading in order to act when the flag is set to true. You have to repeat the lines above for the normal query and add the element only if not already present into the results (putting it more than once will result in unbalance the random pick of the algorithm)
Something like:
This will make the PR cohexists with any existing installation of the AH bots, so other server installation will not be disrupted by your contribution. What do you think about this? |
That's a good point. Will do it. |
Already had some time to implement it. Works as expected 👍 |
@BieJay93 |
There is an issue in the pipeline, please solve it so we can merge this PR
|
Just synced my fork, now it should work. |
merged, thanks! |
Just curious, but is there a reason the new database table this commit creates doesn't follow the module's standard for its tables? This one creates an |
No, there isn't a reason, just hadn't thought about it. |
Changes Proposed:
To get these item ids, i selected all spell ids from "SkillLineAbility.dbc" which are connected to a profession.
More information about these spells are stored in "Spell.dbc", so i selected all spells with the ids that i got from the first step and exported the item ids of the fields "Reagent_1" - "Reagent_8" and "EffectItemType_1" - "EffectItemType_3".
Removed the duplicates and stored them in a new table called "auctionhousebot_professionItems"
If someone is interested, i can also upload my php script to get the ids.
Issues Addressed:
Tests Performed:
Applied the changes, recompiled and looked for profession related items in the auctionhouse.
For example "silver rod" was missing before theses changes. Now I can find it in the ah
How to Test the Changes:
just compile, run and check the ah