Support MPRIS loop and volume change

[foonathan]

This PR supports MPRIS loop status on linux, enabling playerctl loop to fetch and playerctl loop None/Track/Playlist to set.

[foonathan]

This approach is a bit of a hack (it presses the button the appropriate number of times to go to the desired state), but I can't see a better way.

[Araxeus]

I've tried making it work using the youtube api, but its weird and doesn't visually change the button state

So your solution is probably the best bet

[Araxeus]

What is the point of the first modulo? (newIdx - curIdx) can only be in the range (-2, 2) then adding 3 is the exact result we need

Can be simplified to just

const delta = (newIdx - curIdx + 3) % 3;

See calc

0 = (0 - 0) % 3 + 3) % 3
0 = (0 - 0 + 3) % 3

2 = (0 - 1) % 3 + 3) % 3
2 = (0 - 1 + 3) % 3

1 = (0 - 2) % 3 + 3) % 3
1 = (0 - 2 + 3) % 3

1 = (1 - 0) % 3 + 3) % 3
1 = (1 - 0 + 3) % 3

0 = (1 - 1) % 3 + 3) % 3
0 = (1 - 1 + 3) % 3

2 = (1 - 2) % 3 + 3) % 3
2 = (1 - 2 + 3) % 3

2 = (2 - 0) % 3 + 3) % 3
2 = (2 - 0 + 3) % 3

1 = (2 - 1) % 3 + 3) % 3
1 = (2 - 1 + 3) % 3

0 = (2 - 2) % 3 + 3) % 3
0 = (2 - 2 + 3) % 3

for (let a = 0; a <= 2; a++) {
    for (let b = 0; b <= 2; b++) {
        console.log(`${((a - b) % 3 + 3) % 3} = (${a} - ${b}) % 3 + 3) % 3`);
        console.log(`${(a - b + 3) % 3} = (${a} - ${b} + 3) % 3`);
    }
}

---

also, I personally think that short variable names aren't helpful, its better if they are clear and informative
could maybe be named currentIndex and targetIndex instead?

[foonathan]

What is the point of the first modulo?

I originally had just (newIdx - curIdx) % 3, but that has the wrong sign. I then simply added the general code. Good catch.

[Araxeus]

++i is useless here since the value isn't used
i++ is more appropriate in this scenario (tho ultimately it's the same, the syntax is more clear)

also instead of i != n you should use i < n to avoid edge cases (this is standard)

[foonathan]

Funny. I come from a C++ background, and if someone wrote for (auto i = 0; i < n; i++) I would have requested it changed to the way I wrote it, as that's the standard there :D

[Araxeus]

LGTM (but can't personally test it since I don't have access to Linux currently)

[foonathan]

Anything I could do to help it get merged?

[Araxeus]

@th-ch

[foonathan]

I've just pushed another fix for a bug when using playerctl play-pause.

[Araxeus]

In javascript we always use === instead of == 😋

(and !== instead of !=)

could you change those?

[th-ch]

Looks ok overall, could be worth applying Araxeus' suggestion on using === - feel free to apply the change, otherwise I can merge and do the fix afterwards

[foonathan]

I've changed it. Do you want me to clean up the commits and force push?

[Araxeus]

you should also change it in plugins/shortcuts/mpris.js lines 43-45-47

[foonathan]

Volume change support is now implemented as well.

[foonathan]

Can this be merged?

[th-ch]

Looks good, thanks for the contribution! ✅