Create a forgotten favorites feature

[1hitsong]

Creates a list of tracks weighted by play count and time since last play.

As part of this, I created a new lastPlay property. I know there is also a lastPlayed property, but it works differently , has a different default value, and is used all over the code, so I didn't want to change it or rework it as part of a new feature PR.

Screenshot of feature

Fixes #18

[Chaphasilor]

Nice, that looks promising. I think it would be neat to emphasize the "favorites" aspect of this a bit more, only including tracks that we're played a lot, or are actual favorites.
From a quick glance it seems like currently any track that was played at least once can be included, and it's ordered by when it was last played. So if I played a track from a new album at the beginning of the year, decided I didn't like it, and never played it again, it would show up right at the top of the "forgotten favorites", which seems a bit counter-intuitive.

What do you think?

[1hitsong]

So if I played a track from a new album at the beginning of the year, decided I didn't like it, and never played it again, it would show up right at the top of the "forgotten favorites", which seems a bit counter-intuitive.

Only if you didn't play much else during the year. I have a lot of tracks I only played once at the start of the year, and you can see it only picked up the ones I played at least twice.

The sort weights the play count. We can play with the weight value to fine tune the formula.

[Chaphasilor]

Ahh yeah I didn't see that.

Another problem I could see is that if there really isn't a "forgotten love", it would still create and show the list. That could contain something like "last played December 20th", in theory. Maybe we should add a threshold there?
The skip features have a similar issue.

[1hitsong]

I've added a minimum last play age - currently set to 180 days, roughly half a year.

I also added code to auto skip the forgotten favorites slide if no tracks are found. I coded it so we can easily use the auto skip logic for other feature slides.

[Chaphasilor]

Thanks for the minimum age and auto skip. I think we can set the age lower, 3-4 months are probably enough.
Also, I'd like to stay consistent with the other features and only feature the first 5 tracks, with the remaining tracks in a plain list, to avoid scrolling and improve screenshotability on mobile. Could you please change that?
Also, you should add a padding to the description/subtitle on the feature (or limit the width in another way), so it can't stretch to the edges of the screen, as that looks a bit weird. Here's a picture:

[Chaphasilor]

Sorry about the force push. Commit ended up in the wrong branch :(

[Chaphasilor]

Since all other features (top albums, top tracks, etc.) also list the top 20, and feature the first 5, I've adjusted that quickly.
Theoretically this can be merged, but do you think it would be possible to somehow integrate this with the getTopItems() in aggregate.js? This is how all other top-something features are generated, including most/least skipped. So it would be nice if the "forgotten favorite" could stay consistent with this.

If it's more trouble than benefit, we can also leave it like this :)

[1hitsong]

I can move it if you tell me to, but personally, I think we should be going the other way and moving things out of getTopItems() and into their own dedicated functions.

I'm a big believer of clean code standards and single responsibility. I like functions to do 1 thing - that way all the code in the function is pointing to 1 goal and 1 goal only. Sticking to this helps the code be cleaner, easier to maintain, and easier to write tests for.

[Chaphasilor]

@1hitsong that sounds good and I'm also a fan of it, as long as things are consistent. So things like local variables setting the number of returned items are a bit icky. Maybe we could have some sort of global configuration/constant object that each function can then use to determine these things? What do you think?

[1hitsong]

Maybe we could have some sort of global configuration/constant object that each function can then use to determine these things? What do you think?

🤘 Heck yeah, I'm in for that! I'll open a ticket.

[1hitsong]

Code Cleanup ticket created: #42

[1hitsong]

Once we get this feature buttoned up and merged, I'll go ahead and work on ticket #42 next before working on any new stuff. As part of #42, I'll update not only this feature, but also the others to use the new global config.

[1hitsong]

@Chaphasilor Anything else you need on this PR to get it merged?