pucauto
pucauto copied to clipboard
tomreece
Second cut at always checking for add_on trades.
So, this is a replacement for the previous pull request (#50) that adheres to all the same commentary I made in issue #49 and should also fix issue #56. The "only" exception is that this time I don't do a full reload of the trades table EXCEPT when the find_add_ons config option is true AND minutes_between_add_ons_check has elapsed. Note that add-on trades found with traders who have points higher than the min_trade value are still found every time, but we didn't have to scroll the trades table to find them, so "free". I think this is a improvement overall, especially since it unifies the searching and sending code -- the routine find_and_send_add_ons() is completely gone.
I tried to make small commits so that you can see the clear progression of the code base, but in the end, this kind of change really was a little pervasive. There are some output and other minor enhancements I would like to make later, but would rather get the base functionality merged first.
I should have also said that I've been running this code all along and have accumulated many new and add-on bundles in the process, so I'm quite confident that it works as I intended. I've been running with the minutes_between_add_ons_check set to default (5 minutes) and then I turned it down to 0.01 (effectively zero), which effectively means do a full search for add ons EVERY time, but the logic in the code is smart enough not to bother if you have no unshipped trades outstanding.
I have had two sets of family in town over the past two weeks. This weekend will slow down for me and I'll actually be able to bunker down and take a look at this pull request. Thanks for your contribution!
Sorry I didn't get around to reviewing this during the weekend! I haven't forgotten about you. I'll take a look this week, promise.
Added a couple more minor changes based upon what I've been seeing / wanting to see.
@eengstrom I am so sorry it's taken me so long to look at this and try it out. I just launched www.ppbounty.com but the dust is settling around that. I promise to look at this by the end of this weekend!
Merged in the recent mainline changes to make your subsequent merge of this pull-request easier. I've been using it continuously since December, so I'm quite confident in the stability. Let me know what I can do to make the merge easier if needed.
It's good to hear that you've been using it for a while. Thanks for that info. I'll be honest, I haven't gotten around to merging it because it's a lot to look at and think about. I do have some initial comments I'll be adding soon. I'm very picky and hope you are patient with me. :) I try to balance simplicity with feature sets. I want to avoid too complicated of a product, because I work 9 to 5 daily on complicated bloated software. I definitely appreciate the contribution though and am glad that at the very least you have been able to benefit from your hard work since December. I'm so glad I made Pucauto open source.
Just merged in all changes up to current master (v.0.4.5). I know you are busy, so what can I do to make the merge of this request easier?
@eengstrom Have you still been using this reliably for some time? Has it met your goal? Can you summarize the benefits this adds to Pucauto in a few sentences for me? Sorry if that seems redundant because you've described very well above. I just need a refresher, then I'm going to look through the code and make any comments and run this branch myself for a few days to a week.
@tomreece -- been using it non-stop.
Fundamentally, because my version is always looking for add on trades, I'm more likely to catch additional cards when (a), someone is serially adding wants or (b) when someone is gaining points (due to their outbound trades being completed), or (c) when multiple people are attempting to trade with the same recipient, and some of those trades are subsequently canceled. Probably other circumstances, but for someone like me that does a lot of "trading up" (C/U for R/M), this results in more valuable packages.
As a side effect, I think it cleans up the code base a bit.
Is it perfect? Absolutely not. The biggest failing is that it nearly always has to scroll the trades table to the bottom, which means I'm somewhat more likely to get "beaten to the draw". I have some notions in my head to fix that, but I keep waiting for the Future Sight change on PucaTrade, figuring that we'lll have some non-trivial re-work to keep something like this working. So many other things to work on that the current state for me is "good enough."
Small bump for a few suboptimal things I noticed:
- Currently when loading unshipped traders you only load the first page and don't have a loop to click 'next' until you run through it all.
- In general, when scrolling through a whole lot of potential trades it's a good idea to have your loop scroll and then sort through potential trades while it's waiting for Puca to give you the next part of the trade table. The master doesn't have this either, but they should both probably have it.
