NiyaShy / XB1ControllerBatteryIndicator

A tray application that shows a battery indicator for an Xbox-ish controller and gives a notification when the battery level drops to (almost) empty.
GNU General Public License v2.0
710 stars 53 forks source link

Add options to select which controller states/types to show #38

Closed rolodato closed 4 years ago

rolodato commented 4 years ago

Haven't done localisation besides English, but figured this was a good place to start a PR. This is what it looks like:

image

The tooltip has been changed to always show the status of all connected controllers:

image

NiyaShy commented 4 years ago

Do I understand this correctly that you removed the icon cycling and made it show a separate tray icon for every connected controller with the option to hide certain types?

I'm a bit torn about this change... While it's nice to be able to hide certain types (like wired controllers since they don't report battery info), I opted for a cycling single icon in order to avoid clutter in the systray. And I'm apparently not alone with this since I've been asked several times(!) if it's possible to hide the icon completely when no controller is detected (which I had to decline).

rolodato commented 4 years ago

Do I understand this correctly that you removed the icon cycling and made it show a separate tray icon for every connected controller with the option to hide certain types?

No, sorry for the confusion - the second tray icon shown in my screenshots is the stable release, I was just running it beside my build to compare the behaviour as I was developing this PR.

With my change, the icons rotate as normal, but each controller is only displayed if its state is enabled in the settings. If controllers are connected but none can be displayed, the default "no controller detected" icon is shown. The tooltip always shows the state of all controllers or "no controllers detected" if none are connected. You can see this in the connectedControllers and shownControllers collections in the main loop that updates the state.

rolodato commented 4 years ago

Here's an example of what I mean:

image

The top icon is the stable release, showing a single wired controller. The bottom icon is from my PR, with displaying of wired controllers disabled and no other controllers connected.

NiyaShy commented 4 years ago

Ah, thanks for the clarification. As you guessed, the second icon in your screenshot confused me a bit, and a glance at the code didn't help much either since githubs way of displaying it made it a bit... hard xD

With that out of the way, I'd be happy to accept it. But, as you said yourself, translations for the new menu entries are missing, and I want to keep such stuff out of the main branch until the language files have been updated. Haven't found a way to redirect the pull request to the testing branch, so I fear I'll have to decline/close it (unless you can change it).

rolodato commented 4 years ago

I messed up this PR and opened #39 instead. Thanks @NiyaShy !