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

Feat: add option to prioritize drops ending soonest #433

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

Conversation

jaredkotoff
Copy link

@jaredkotoff jaredkotoff commented Feb 9, 2024

Personally, been using this for months and thought others might appreciate it.

This maximizes the amount of potential drops by focusing on drops ending soonest.
It still takes into account user's priority list and mines those drops first before moving onto any drops that aren't prioritized.

Screenshot 2024-02-10 at 6 49 14 PM

Note: the priority list's order doesnt matter when this setting is turned on

Screenshot 2024-02-10 at 6 49 23 PM

See that Halo's campaign is mining first because these campaigns are often very short so this is top priority to mine before it ends. You can see that Rainbow Six Siege will be mined next as its campaigns ends after Halo's. We will mine this campaign so we don't miss it by mining something else.

I hardly miss any drops from games in my Priority list this way because we focus on mining games that will end soon rather than mining a game that we might have a whole month to get too.

@DevilXD
Copy link
Owner

DevilXD commented Feb 10, 2024

Hello o/

Interesting. I'm lost on it's function though. First of all, there's no need to sort campaigns with their ends_at, as that's already done during each and every inventory fetch:

TwitchDropsMiner/twitch.py

Lines 1652 to 1654 in d7c2c8d

campaigns.sort(key=lambda c: c.active, reverse=True)
campaigns.sort(key=lambda c: c.upcoming and c.starts_at or c.ends_at)
campaigns.sort(key=lambda c: c.linked, reverse=True)

Upcoming campaigns are sorted by their starts_at, but since they can't be watched yet, the loop will just skip over them. linked is used as the last sort, to avoid looping over unlinked campaigns, and since Python's sort function is stable, the previous sorting orders remain in place. This also avoids decreasing the max_wanted_priority, whatever that represents.

The same campaigns order is then used to fetch the relevant channels for each campaign to progress, but since campaigns and channels aren't tied with each other, the order gets lost. If the order would be somehow preserved, the miner would always pick the very first channel to watch, as it'd have a game of the highest priority, and if no priority would be present (priority of 0), then the sorting by ends_at would ensure the first channel would be for a campaign that's going to end the soonest. But then again, the order between campaigns and channels isn't kept by anything other than the priority value.

Also, I can see this code actually modifies the priorities. How exactly is the priority list order kept in place and it prioritizes campaigns that's going to end soon?

@jaredkotoff
Copy link
Author

jaredkotoff commented Feb 10, 2024

Also, I can see this code actually modifies the priorities. How exactly is the priority list order kept in place and it prioritizes campaigns that's going to end soon?

Priority list is considered second to time ending soonest. This is intentional.
I want these drops from selected games, but I don't actually care about what order they're done in. By focusing on the order that they end in, you're less likely to miss a drop, by focusing purely on the name list.

Once the priority list is finished, then it does the same for non-priority games.

Of course, this is only if the user wants this since it's a toggleable setting.

@DevilXD
Copy link
Owner

DevilXD commented Feb 11, 2024

So essentially, when toggled on, it ignores the order on the priority list, but works only off the campaign's end date?

I mean, this was never the purpose of the priority list in the first place, and "priority" in the priority list stops making sense at that point. But I kinda like the idea a bit.

I'll need to think about this for a while. This isn't something that I can merge right away, but I'm not saying no to it. Could be something for #220 as well.

@jaredkotoff
Copy link
Author

jaredkotoff commented Feb 11, 2024

It doesn't fully ignore the priority list though. It still works on those games before any games not in the priority list.
A more major change is to add a third "top-priority" list that will work like the current logic either way, but I fell like that's unnecessary as this has been working amazingly well for me

This isn't something that I can merge right away

Just to be clear, it doesn't affect how the system works right now unless it's enabled by the users.
Although, I'm sure you can implement it better if you consider this feature "higher up" in the code. I kind of had to hack it in to not have to edit a lot of how the campaign lists are created atm

@WrayOfSunshine
Copy link

I actually thought that this was how the priority list worked - I didn't realize that the order listed in the priority list was the order the program would prioritize them in. I always assumed the system would prioritize in something like "If a priority campaign exists (regardless of location on the list), prioritize it over campaigns not on the list at all. Then, pick the one ending soonest that we can still get drops from."

@herhor67
Copy link

Even better would be an algorithm maximizing the count of possible drops - like, make sure that few shorter are completed instead of one longer.
But this all would require writing some kind of actual scheduler and not just a simple sorting...

@Windows200000
Copy link

@jaredkotoff If you are willing to adapt this code to my new fixed fork, I would be willing to include it in that. Please only take the part, that prioritises end time, because as DevilXD said, the rest is redundant.

@jaredkotoff
Copy link
Author

@Windows200000 i already have this code merged into a different fork, I was just trying to contribute back upstream

@Windows200000
Copy link

@jaredkotoff but it's broken now and DevilXD doesn't have the time to work on it, so I'm maintaining it in a fork for now.

I'm still not familiar with that part of the codebase, which is why it would make sense if you took the end-date priority and made a pull request in my fork, that works right now. Only if you want to, or if you were gonna adapt it for yourself anyways, of course.

@Windows200000
Copy link

@Windows200000 the fork I use has been working fine. So I'm not really interested in redoing the work. But like I said, this code is a bit hacky anyway. If you're going to maintain a new fork and want this feature, I would suggest having a different shape for the data that lets you sort these based on end time easier than how I did it here.

@jaredkotoff Twitch changed the API recently and there is now no progress with code from the main repo. DevilXD expressed, that he would like to rewrite a big part sometime in the future anyways, but he doesn't have time now.

And I personally don't like working with python. If it was me I would make it in rust, work a lot more with defined variables, rather than hardcoded integers, compartmentalize a lot more, and have an SQLite db that allows simple access and use of all necessary data about streams and drops. But since my fork works, I have no motivation to write it entitely from scratch.

@jaredkotoff
Copy link
Author

jaredkotoff commented May 21, 2024

@Windows200000 I understand what you are saying. The other fork I use already pulled in your changes, as well as several features I've already contributed.
You're free to pull in all this code into your fork though if you want, there's no modification needed as it works as I expect it too (i have been using it non-stop since i opened this PR)

@Windows200000
Copy link

@jaredkotoff would've moved it elsewhere, but you don't have issues on your fork

The comparison is interesting. The "starting" message is kinda pointless, because there is no terminal it could get printed to and I can't implement headless, because despite me having to leave my whole PC running, DevilXD is (rightfully) worried about people using that to make money/spam twitch.

I will look into the ending priority tho, I hope nothing collides with the dark mode I just added.

@Windows200000
Copy link

@jaredkotoff Oh you have 2 accounts, that confused me xD, is there a reason you don't have this priority in your main fork?

@jaredkotoff
Copy link
Author

jaredkotoff commented May 21, 2024

@Windows200000 I don't have two accounts. I have an GitHub org I sometimes work under.
I have a fork from this repo there for the changes that don't conflict with this repo's philosophies.
The fork under my account is to contribute to another fork (that I have not mentioned or linked to in respect for the philosophies of this repo).

If you're looking for this sorting code, this PR has the extracted and relevant code.
The branch on the fork for my personal code has some altered functionality. I forgot, that I never actually got this code merged into a different fork because I was hoping this would get accepted here and wanted to avoid conflicts later on.

For some additional clarification, check the network graph: https://github.com/DevilXD/TwitchDropsMiner/network

twitch.py Show resolved Hide resolved
twitch.py Show resolved Hide resolved
twitch.py Show resolved Hide resolved
twitch.py Show resolved Hide resolved
@jaredkotoff
Copy link
Author

Also, It might be a worthwhile venture to implement your own solution.
This code is kind of hacky. I added some comments to better explain things though.

@Windows200000
Copy link

@jaredkotoff I don't see a good reason to add the unlinked stuff, so I think I will stick to this. I think that would just confuse people, it's not like you get anything without linking anyways.

@jaredkotoff
Copy link
Author

jaredkotoff commented May 22, 2024

@Windows200000 there is no unlinked stuff in this PR or fork.

it's not like you get anything without linking anyways.

You do. You get the drops.

@Windows200000
Copy link

@Windows200000 there is no unlinked stuff in this PR or fork.

it's not like you get anything without linking anyways.

You do. You get the drops.

@jaredkotoff But they can't get added to your game account without linking, or am I missing something? I mean you can link later, but there is no reason not to link right away if you want them.

@jaredkotoff
Copy link
Author

jaredkotoff commented May 22, 2024

@Windows200000 You are missing how time works...
Drops are a limited time event and games cost money. Not everyone owns every game the second they are released.

You can buy furniture for a house that you will move into someday, you don't have to own the house first and miss out on a good sale.

Either way, this is irrelevant to this PR. No one bought up the unlinked stuff before, nor was I suggesting adding that to this or your repo.

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

5 participants