pvrs12 / Anesidora

Anesidora - Pandora extension for Firefox
Other
31 stars 8 forks source link

Can't dislike unrated songs #59

Closed hucario closed 3 years ago

hucario commented 3 years ago

I've been noticing this issue for a while now. When the "dislike" button is pressed, the song is only skipped. The erroneous code: anesidora.js:247

if (rating == song.songRating) {
    return;
}

Because JS likes being an "interesting" and "fun" language, unlike Python which is "uninteresting" and "reliable", 0 is evaluated as falsy in JS., while 1 evaluates as truthy. From the Pandora API doc:

property type description
items.songRating int 1 if song was given a thumbs up, 0 if song was not rated yet

rating is false if disliking, true if liking. so... if the song is already liked, it doesn't like it again (true == 1 (truthy)) but if you're trying to dislike it, false == 0 (unrated), returns false == 1 (liked), continues

A workaround in the current version is just to like the song before disliking, although I don't know if that screws up the algorithm or something.

""improved"" code:

    if (rating == (song.songRating===0?true:false)) {
        return;
    }

In JS, === checks type as well as value, while == only checks value. ¯\_(ツ)_/¯ (seems like you're a python guy so I don't really expect for you to know that)

It would also be good to move the check to after this block

    if (rating === true || rating === false) {

    } else if (rating < 0) {
        rating = false;
    } else if (rating == 0) {
        return;
    } else if (rating > 0) {
        rating = true;
    }

in order to make sure that rating is a boolean.

I'll open a PR for this Soon:tm:

hucario commented 3 years ago

realized a better way to fix it would be

    if (rating && rating == song.songRating) {
        return;
    }

that way it only checks if it's already liked if it's liking the song. :ok_hand:

hucario commented 3 years ago

Unrelated but while I'm here: The CSS ::before property can be used instead of manually putting <span></span>s everywhere for icons, so I may work on that soon