🎵Artwork Rendering #6
Conversation
Picking image URL from itunes lookup whenever player notification is up
🎵 Artwork always rendering properly
There was a problem hiding this comment.
Hey, thanks for the PR! I've found a few lines that needs changing before this can be merged.
By the way, since we still have to fetch the artwork URL from iTunes to set it for Discord, I don't think getting the artwork from MusicTrack to display in the UI is very useful since it might not match the one we got from iTunes, confusing users. Don't get me wrong, it's great that you got it working, I just feel like there's little utility to it.
| @@ -121,11 +121,13 @@ class DiscordRPCObservable: ObservableObject { | |||
| self.artwork.album = album | |||
| // if no artworks embbeded to the file, it returns nil so RPCStatusView can render no artwork | |||
| self.artwork.url = artworks?.first?.data ?? nil | |||
|
|
|||
| self.rpc.setPresence(presence) | |||
There was a problem hiding this comment.
I don't think this does anything, since the presence setting happens inside the dataTask closure after we've fetched the artwork from iTunes.
| self.artwork.url = nil | ||
| } | ||
| self.rpc.setPresence(presence) | ||
| self.artwork.url = nil |
There was a problem hiding this comment.
This should be updated on the main thread. Also, presence.assets.largeImage should be set to applemusic_large so there's no empty box on Discord.
| @@ -49,7 +55,8 @@ class DiscordRPCObservable: ObservableObject { | |||
|
|
|||
| private let logger: Logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "DiscordRPCObservable") | |||
| private let jsond: JSONDecoder = JSONDecoder() | |||
|
|
|||
| private var albumId: Int = 0 | |||
There was a problem hiding this comment.
IMO this should be moved to the DiscordRPCData struct.
| } | ||
|
|
||
| @Published var musicArt: Data?; |
There was a problem hiding this comment.
This is unused, should be removed.
Hey, thanks for reviewing it! It makes sense your train of thoughts and I indeed did think in just update the URL request and avoid everything else over the code. It was intentional on my part to capture the artwork for a few main reasons:
My original plan was to do the point 3, but the request seems to be too large, due to the base64 size. If you prefer, I can keep this PR out for a while and update the fetch just for improved search on start + lookup on every other request |
Added Artwork on RPC Status Bar coming from SBApplication
MusicTrackhas a method that returns one album cover whenever it is availableAdded new search endpoint
GET /lookupis a more reliable source of collecting data, once if you have the album ID, it should be one and only. Once playerInfo moves, if it's a song pulled from Apple Music that is available to buy on iTunes, anitmss://url appears that we can pluck album IDImproved search URL
searching by
entity=albumreturns albums, but it was all over the place. using theterm=\(artist)andattribute=artistTermwe can guarantee that all the data returned is from the specific artists, leaving us to find the right album through the appCaveats
while working on this, I removed the condition preventing request/replace a cover from the same album. I'll try to get this sorted out before you get to review this, but as of now every new song is a request to iTunes and the icon flashes on Discord.I kept the query, but I fixed the flickering. iTunes allows 20 requests/minute, it shouldn't be a problem for most users.you'll need to put your developer id back to build (but you might know that). Opening through Xcode to run/debug forces you to change into your own dev id and I pushed it to you as it was "mine".