Fixes and Improvements since Twitch API change this month

[Windows200000]

@DevilXD I suggest you merge a pull request from my fork before you start changing anything, work on it, and then I can take it back to my fork once you have to focus on work again.

What's changed

Features

• Added dark theme
• Added prioritize by Ending soonest

Fixed bugs

• New watching method to make the miner work with the API change
• Fixed long tray description causing crash, issue in your repo, my fork, fixed with commit: Windows200000@1c1a950

Other changes

• Changes to GitHub workflows

• • ci.yml Building in-dev on changes (you will have to rename it to debug-channel, if you want it)
• • release.yml Building release, automatically bumps version (v15.x.0) and includes patch notes
• • Check_patch_notes.yml Fails on pull requests to in-dev without patch notes changed.

• English translation is now defined in English.json instead of translate.py, because GitLocalize needed a source json

• I would suggest you set up GitLocalize, as it makes it quite a bit easier for contributors to translate and you to review changes. The only main issue is that changing the contents of English.json flags all translations of that text as incomplete. I don't know of a better way to revert that, than removing all the languages from GitLocalize and adding them back.

Issues

This isn't everything, I included relevant Issues not present in your upstream

• Efficiency of new watching method: Efficiency after 2024/5 API Change Windows200000/TwitchDropsMiner-updated#3
• Translations missing: Translation to other languages. Windows200000/TwitchDropsMiner-updated#2
• Channel points have also changed, I haven't looked into this at all: Channel points not being tracked + AppImage Errors & CPU Usage Windows200000/TwitchDropsMiner-updated#23
• Sometimes TDM doesn't claim campaigns until a reload: Drops not being claimed in the intended way, causing some to be missed and no notifications Windows200000/TwitchDropsMiner-updated#85
• To see everything, look at open issues of my repo, 5 through 15 is copied from your upstream repo.
• You commented out handling of service error. I don't know why and don't want to reverse it without understanding why it was taken out: Application Terminated "GQL error: [{'message': 'service error', 'path': ['claimDropRewards']}]" (version fa75358) Windows200000/TwitchDropsMiner-updated#86

You should carefully look through the code, I might not have remembered everything here. You might want some changes, in which case I will make a new branch on my end to work on those.

The latest dev build (Reference commit: Windows200000@e82f4d8) should be stable, if you want to take a look at the user-facing changes.

[guihkx]

(I'm not speaking for DevilXD here, I'm just a contributor, like you, sharing my personal opinion).

First, thank you so much for working on these issues!

I believe you made a mistake when you opened this pull request, because the tracking branch is set to debug_channel, when you probably meant to choose master.

With that being said, having to review 4000+ changes is a massive undertaking for anyone, and I imagine even more so for DevilXD, since he currently doesn't have the time to do so.

Additionally, I believe that most of these changes are unrelated to the "main issue" at the moment: #462.

So, if this pull request stalls, allow me to suggest another approach:

• Make a new pull request only meant to fix No mining progress is being made #462
• Make 1 pull request for each issue you're fixing
• Make 1 pull request for each feature you're adding

[Windows200000]

@guihkx I made this, because I took this project over temporarily and DevilXD said this now:

As a small update on my availability, I'm currently waiting for a new desk to be installed, so that I can have a place to sit down and work properly again, as my previous desk has unfortunately finished its lifetime. This should happen later this week, or next week. Hopefully I can sit down for some dev work next week, before going back to heavy work cycle 2 weeks from now.

I don't think it would make sense for him to develop anything in parallel, which is why it's all in one. It's in debug, because he might just want to take what I have and tweak it before pushing to master.

edit: Also it's by far not 4000+ changes, GitLocalize just sorted translations and English.json was removed from gitignore instead of it being in translate.py generating the json.

[DevilXD]

As @guihkx said, if I I'd like to do it in a standard review manner, I'd rather have a separate PR for each and every change made, so that I can separately review and triage everything properly. Only that way, I can actually merge anything "right away" into the current master branch.

Otherwise, you can keep this PR open as is, and I'll slowly work my way through whatever needs to be added/changed later. However, these will either be my own commits with your code, or something similar to your code (talking about resolving #462), and generally I cannot use this PR in any other way - it won't be merged like this. So again, depending on how you want to contribute, it will either be "your way, your code", or "your way, my code".

[Windows200000]

Will sort the commits tomorrow.

[Windows200000]

@DevilXD I was able to bundle the main fix, dark theme and the tray fix into what should be working branches.

The rest, however, depend on each other. Even the dark theme can't include the translations, as they include other changes.

I tried my best to leave the in-dev branch in a mergable state. If you wish, it should be possible to just revert translations and do them separately your way. But other than that, I really do think it makes sense to review the entire thing in one.

If you close everything in the lang (99% just reordered by GitLocalize) and .github folder, it really isn't that many changed or new lines of code.

You could also take a look at main-fix, tray-title-fix, dark-theme and prioritize-end-date, review those while leaving them untranslated, and then take a look at the rest, that can't be neatly split up. I tried to include everything necessary there, but it is possible, that I missed something.

[Windows200000]

Again, disregarding workflows and the lang folder, there really isn't that many changes and I tried my best to document them well. While I can see how splitting the PR up would usually make the review easier, the changes in my repo often depend on each other, making it difficult to define clear lines, even less so if I try to split it up by commits, that often bundled the actual changes with documentation and patch notes changes.

Maybe the way forward would be to move this into a new branch in your fork, that you can take your time to review and change to your liking, potentially removing a bunch of the workflow stuff etc, before merging into main?

[DevilXD]

Unless I can get the new desk installed in the few upcoming days, I won't be able to do any significant work on this project for the two upcoming weeks. 3-9 of June I work locally, 10-16 I'm going back to delegate work. You have plenty of time to divide and split everything apart, including making adjustments to anything according to my reviews, where necessary.

I've created the PRs for each of the sub-branches you've created, excluding the main fix. I'll be leaving my reviews for each part there.

#488. #489, #490.

[Windows200000]

Why not the main fix?

Also, what are your thoughts on the GitLocalize thing? (Including the move of the default translation from translate.py to English.json)

I still believe it would be easier if I incorporate your feedback, from the 3 pull requests, and then you take over the entire thing as one, including documentation and README changes.

I'm not claiming the code is perfect and I do acknowledge a review in steps being easier, but a lot of the changes make sense together. The vast majority of "changes" are just GitLocalize reordering translations anyways.

I would be happy to take your feedback and guide you through the changes in more detail, expanding on my first message here.

[DevilXD]

Why not the main fix?

Because it don't have the time to review and validate that yet, yet alone think about merging it, and the other issues are easier to triage and generally have a much better chance of getting merged, provided the feedback will get implemented/taken care of. Making a PR for the sake of making a PR makes no real sense.

Also, what are your thoughts on the GitLocalize thing? (Including the move of the default translation from translate.py to English.json)

I'm not familiar with GitLocalize, so can't really say much (yet?). The default translation is required to have all translation keys implemented, and thus it's defined in code with a structure representing said translation, to determine if/when keys are missing from it. In contrast, any other translation isn't validated, and missing keys are simply replaced with the default translation instead. There already is existing code that exposes English.json in dev environments, so if really needed, one could make it save everything to a file for translation purposes - it's just that the current solution does not need a file at all, so there was no real point to save anything. Slightly less disk usage is always something.

I still believe it would be easier if I incorporate your feedback, from the 3 pull requests, and then you take over the entire thing as one, including documentation and README changes.

Probably, but then again, all of the three changes that got their respective PRs made for, can also be merged right away, provided they'd be written in a way that I'd be able to maintain going forward. Unfortunately, that's not the case so far. I can try working on each of them once I will be able to do some dev work in the future - hopefully "Soon™️"

I'm not claiming the code is perfect and I do acknowledge a review in steps being easier, but a lot of the changes make sense together. The vast majority of "changes" are just GitLocalize reordering translations anyways.

I'm kinda against reordering, as the current translations are put together in a way that mimics the order of how they appear in code, and display in the final window. This makes it easier to translate things as you're going through the file. Technically, the tree structure already gives the translating person a little bit of order to the whole thing, but still.

The code is never perfect, even my own code - I always find myself refactoring old code to make it better, more readable and expandable in the future. That's what like half of the commits in this repo is all about 😅 I wouldn't worry about it much though, if you don't feel like implementing any of the requested changes, you don't need to 🙂

I would be happy to take your feedback and guide you through the changes in more detail, expanding on my first message here.

I can see all the changes made in the diff =) But sure, if I'd have any questions and I'll be working on this, why not.

[Windows200000]

I'm kinda against reordering, as the current translations are put together in a way that mimics the order of how they appear in code, and display in the final window.

GitLocalize does that. You can look at the UI here. I added it specifically to make contribution easier, especially for people not as familiar with coding, JSON etc.

Probably, but then again, all of the three changes that got their respective PRs made for, can also be merged right away, provided they'd be written in a way that I'd be able to maintain going forward. Unfortunately, that's not the case so far.

Working on those would be the best thing to do. It might make sense to create another one for the translation thing, if you're at all interested. I want to get to a point where you're not held up by me once you have time.

[Windows200000]

@DevilXD this got closed because I made another release and recreated in-dev.

[Windows200000]

@DevilXD Campaigns with subscription requirement and 0 required minutes are now also a thing (Windows200000#101), and I haven't found a cleaner way of detecting it, than this:

    def progress(self) -> float:
        if self.required_minutes:   # Quick fix to prevent division by zero crash
            return self.current_minutes / self.required_minutes
        else:
            self._manager.print(f'!!!required_minutes for "{self.name}" is 0 This could be due to a subscription requirement, tracked in Issue #101!!!')
            self.preconditions_met = False
            return 0

[DevilXD]

It appears there's a new drop field called requiredSubs, which can be found next to requiredMinutesWatched. It seems like either one of these two values can be zero now, means it's either time or subs based. Weird to see Twitch put a subs based drop in "timedDrops", but whatevs.

I'm not sure what to do with drops like that, but your crude solution should work for the time being. I'm still yet to find enough time to sit down to all this mess, and it looks like Twitch has introduced a yet another thing one needs to take care of. sigh