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

Store percentages as floats #86

Open
wants to merge 1 commit into
base: rework-1
Choose a base branch
from

Conversation

dedmen
Copy link

@dedmen dedmen commented May 10, 2024

I have one Artifact quality level item I want to be sold.
If I set percentage to the minimum possible value (1%) that means the AH bot wants to sell 90 of these items, and sells tons of this supposed to be rare item.

With percentages as float it can be set to 0.1%. I also made it round up so a 0.1% doesn't end up with zero..

I don't know if the naming of the DB update file is correct. it does work like that, but would you prefer some date prefix? which date?

if (diff < 0)
{
if (_itemsPercentages[AHB_ITEM_QUALITY_NORMAL] - diff > 0)
_itemsPercentages[AHB_ITEM_QUALITY_NORMAL] -= diff;
else if (_itemsPercentages[AHB_ITEM_QUALITY_UNCOMMON] - diff > 0)
_itemsPercentages[AHB_ITEM_QUALITY_UNCOMMON] -= diff;
}
else if (diff < 0)
else if (diff > 0)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bug was already present on rework branch, didn't notice this before.

@dedmen
Copy link
Author

dedmen commented May 12, 2024

https://github.com/azerothcore/mod-ah-bot/actions/runs/9036393402/job/24833180952#step:10:564
I don't understand that, I would expect that the base SQL script is already applied before the update scripts run

@Helias
Copy link
Member

Helias commented May 12, 2024

@pangolp any idea?

@pangolp
Copy link

pangolp commented May 13, 2024

@pangolp any idea?

Tomorrow as soon as I turn on the PC it fixed the error. I hadn't seen the notification.

@pangolp
Copy link

pangolp commented May 13, 2024

https://github.com/azerothcore/mod-ah-bot/actions/runs/9036393402/job/24833180952#step:10:564
I don't understand that, I would expect that the base SQL script is already applied before the update scripts run

It should work as you say, but it seems to me that it doesn't work that way unfortunately. It has happened to me with other modules and I have had the same results. When the SQL is edited, however, the hash changes and the script runs again. So you can edit the original script or, failing that, not use the updates folder. There must be an error, in the order in which the SQLs are executed. It should be base first, then updates. But I always fail that. And it is something that the truth is, I never investigated why it happened.

@pangolp
Copy link

pangolp commented May 18, 2024

For this to pass the tests, and to be able to merge, it is best to create the SQL within the base, so that it is executed after the first one. For some reason that I don't know, the modules have this problem, that they first execute the updates. The same thing has happened to me in several modules. But it must be something about the emulator (regarding the modules). First you should run all the bases, and then the updates. But here, that does not happen, and that is why this error is caused.

@dedmen
Copy link
Author

dedmen commented May 26, 2024

I tried that at first.
The server sees that the hash has changed, and runs the whole base SQL. Which means your config table with the percentages gets overwritten with the defaults.
I don't want to add a feature that deletes all your configuration..

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.

None yet

3 participants