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
add-ons can and probably should be checked every refresh #49
Comments
I have a partial solution that I hacked together this afternoon. Will test and then send you a pull-request. |
@eengstrom Cool. I'm anxious to see what you come up with! In the current implementation, you can remove the I'm assuming you have a plan where these are combined by keeping track of things in a dict or something. I'm anxious to see your solution! Thanks for contributing! |
Running live tests now. I think I've got it right, though you may not like the volume of changes I made. If you want, I can generate a pull request from my |
I'll leave it up to your discretion. I'd definitely appreciate that if you're going to submit a pull request, you thoroughly test it. At the moment I don't have unit tests or anything to help with this so all of my testing is done in the real world so I'm quite cautious about changes. Whenever you feel it is stable and solves the problem you set out to solve, feel free to submit a PR and I'll review. |
@eengstrom PS: I'm about to merge my PR which has a few changes throughout the file, this will likely cause a merge conflict for you. Hopefully it's not too bad! |
@tomreece -- I was able to merge your changes in without much difficulty, as I was already using your refactor branch. I then left it run overnight and got some examples of both new and add-on trades. So, I think this is ready for review. I'll generate a pull-request in just a moment. The output from both new and add-on trades is unified and looks like this:
There are new
I removed the previous two (obsolete) options having to do with checking for add_ons and the refresh period for that activity. Currently, add-ons are always looked for. We can change if you want to make it an option. Personally, since there are a lot of people who add cards to their wants list serially, plus times when traders get new points often, I see this as an obvious "always on" or at least "on by default" feature. The one change that I would like to make better, but I will save for a future day, is to somehow speed up loading of the trades table more. Currently, if you have unshipped traders, it will always load the entire trades list. If we didn't, we'd miss potential add-ons. I don't like it currently, but it works. Finally, I also did my best to mimic your new Let me know if there are changes you want to see before merging. |
Awesome! Thanks for your contribution. I won't be able to look it over thoroughly until tomorrow or this weekend, but expect some feedback soon. Loading the full trade list sucks for users with large collections. That's what motivated me to introduce a solution that only checks for addons every so often in the first place, because normal bundle checking can get by by only scrolling down a few times. I'm anxious to look closer at what you came up with though. |
The current method of checking for add-ons does so infrequently (depending upon config). It should be easy to track existing unshipped traders and ship additional cards to them every time we refresh the trades page and search for new bundles.
The text was updated successfully, but these errors were encountered: