Random improvements

[amogus07]

next thing I want to do is make the plugins use custom flexible attributes like vocadb_[...]id instead of mb_[...]id

[prTopi]

next thing I want to do is make the plugins use custom flexible attributes like vocadb_[...]id instead of mb_[...]id

Things may have changed now, but when I first started writing the plugin, I remember there was some reason I didn't do that. For the life of me I can't remember the reason though, so it may be fixed now...

[prTopi]

As for va_name setting, should we just use the beets global va_name setting instead?
I forgot the setting exists, but it even says that other sources may use it as well in the docs.

[amogus07]

As for va_name setting, should we just use the beets global va_name setting instead? I forgot the setting exists, but it even says that other sources may use it as well in the docs.

I thought about that too, but for now I decided to keep it seperate because VocaDB has a different default value. I'll probably just use the global setting though, since that's likely what you'd expect to be used

[prTopi]

what do you think about the different handling of artists without an entry (and therefore an id)?

Sorry, but do you mean that you've made some changes to it, or do I think there should be some changes made to them?

I tried looking at the code but couldn't see anything that would affect them so I'm a little lost (or blind.)

[prTopi]

Also for artist string, I was thinking should the artists be sorted in alphabetical order?

All of the artist fields in VocaDB website are already sorted alphabetically, with the exception of the artistString, (shown at the top of the page with the album/song title) which is not used by us.

The artists given by VocaDB api seem to be in addition order however, and so pretty much just a random order, depending on who was added first.

When I got some time I'll try to check out how VocaDB creates the artistString. I thought it was written manually when editing an entry but I'm not sure about that anymore.

[amogus07]

Sorry, but do you mean that you've made some changes to it, or do I think there should be some changes made to them?

I tried looking at the code but couldn't see anything that would affect them so I'm a little lost (or blind.)

Now that artists and artists_ids are merged in a dict, each artist in the resulting artists list has a corresponsing element in the artists_ids list, even if the id has been set to an empty string. Beets seems to be inserting \0 characters as separators between each element of the artists_ids list, which causes the resulting medadate field to contain additional \0 characters with nothing in between. Should I leave it like this or should empty strings be filtered from the artists_ids list?

[prTopi]

Sorry, but do you mean that you've made some changes to it, or do I think there should be some changes made to them?
I tried looking at the code but couldn't see anything that would affect them so I'm a little lost (or blind.)

Now that artists and artists_ids are merged in a dict, each artist in the resulting artists list has a corresponsing element in the artists_ids list, even if the id has been set to an empty string. Beets seems to be inserting \0 characters as separators between each element of the artists_ids list, which causes the resulting medadate field to contain additional \0 characters with nothing in between. Should I leave it like this or should empty strings be filtered from the artists_ids list?

I say leave it as is, because if we start removing empty ids, the ids may not be at the same position as the artists name is.

Since i don't know how to properly say what I mean, here's an example of what I mean:

name	id
Artist1	1
Artist2	no entry
Artist3	2

Currently artists and artists_ids (should) get sorted correctly like this:

artists	artists_ids
Artist1	1
Artist2
Artist3	2

But if we remove empty ones, ids may point to incorrect artists:

artists	artists_ids
Artist1	1
Artist2	2
Artist3

[amogus07]

yeah, I was thinking the same thing, I just wasn't sure about it. Good to know I accidentally fixed a bug

[amogus07]

Also for artist string, I was thinking should the artists be sorted in alphabetical order?

All of the artist fields in VocaDB website are already sorted alphabetically, with the exception of the artistString, (shown at the top of the page with the album/song title) which is not used by us.

The artists given by VocaDB api seem to be in addition order however, and so pretty much just a random order, depending on who was added first.

Not sure about this. I like the way artists are currently sorted. Here's an example: https://vocadb.net/S/304504
Right now, the plugin appears to sort artists the same way as in the generated artistString shown at the top. The categories below do sort artists alphabetically, to make them easier to find, I suppose, but I prefer the artistString sorting since it matches the official credits (which it seems to do in most cases)

When I got some time I'll try to check out how VocaDB creates the artistString. I thought it was written manually when editing an entry but I'm not sure about that anymore.

It's definitely generated server-side

[prTopi]

The categories below do sort artists alphabetically, to make them easier to find, I suppose, but I prefer the artistString sorting since it matches the official credits (which it seems to do in most cases)

That's true. I've just had some songs in my library where artists seem to be in a completely random order so it came to mind, but now that you say it I agree.
One other thing I was thinking about (mainly concerning VocaDB and not Touhou/Utaite) was sorting featured artists based on their artist type. For example, all "Vocaloid" artist types before all "Other" artist types, so that they won't get mixed with eachother. But that would just be my preference in how I would like them to show up so, any thoughts regarding this?

Anyways, I dug a little at VocaDB source, and we've got some differences in how we create the artist string. Current main differences to how this plugin does it are:

1. We do not consider a circle to be a producer. We do add them to artist string, but not artists list
2. VocaDB puts producers in a different order, going from composers -> other producers -> circles/bands -> anything else. We could sort ours the same way, but it's not a priority.
3. In VocaDB, artists of role type "Chorus" are not added to vocalists. I don't recall ever seeing them though, so I'm not sure how often that role is used
4. In VocaDB, if there are 4 or more producers, "Various artists" will be used (in this plugin, we check for more than 5)
5. In VocaDB, if there are more than 2 vocalists, OR there are 5 or more producers + vocalists, "[producers] feat. various" will be used (in this plugin, we check for more than 5 producers + vocalists, and don't add " feat. various")

Also for transparency, points 4 and 5 are the main reasons I didn't/don't want to use the artistString given by VocaDB. I like seeing more of the artists than what they normally show, but that's just my personal preference.
I also thought about making the cutoff line for producers and vocalists a config option, but maybe that's overkill. Though more options never hurt I guess

For reference:

• How artistString gets made
• What categories are never added to artistString
• How artists get sorted
• What counts as producer

[amogus07]

I really don't like how request params are handled. Can you think of a better solution?

[prTopi]

Sorry, but as in what context? If it's about the *Params classes then at a quick look, I'm not sure how I would change/improve it.

[amogus07]

jcrist/msgspec#797 (comment)
this might be a good solution

[prTopi]

Should we move all plugin files to be under vocadb folder, or leave additional sources to be under beetsplug?

[amogus07]

I think I'll leave it like it is.. for now.
I'm not sure if that's possible, but maybe an event hook could be used to dynamically add sources based on a configuration, thereby merging the currently separate plugins into one that could support arbitrary sources. But that's really not a priority for me right now.

Also, my goal is to decouple requests_handler from the plugin as much as possible, and turning it into a general-purpose API client at some point (still with the primary goal of making the plugin work).

[prTopi]

After looking at the pr more closely, I'm not sure about the new dependencies.

• httpx I'm pretty fine with. In my opinion though urllib would do just fine for our use case. I assume that you changed to use it because of http2, with urllib doesn't support?

• msgspec I'm honestly very unsure about. Mostly because it isn't packaged in a lot of linux distros, and e.g. nixpkgs unstable (rolling) still has an old version even though it has been 3 weeks since a new version has come out. I know we don't package this plugin anywhere, so package versions shouldn't matter as it'll come from PyPi. But what I'm mostly getting at is that the package doesn't seem to be widely supported.

Mostly, for both of these dependencies I'm just wondering, (how) will it improve this plugin? I just personally do not see that we gain much by using either of them.

[amogus07]

After looking at the pr more closely, I'm not sure about the new dependencies.

• httpx I'm pretty fine with. In my opinion though urllib would do just fine for our use case. I assume that you changed to use it because of http2, with urllib doesn't support?

beetcamp uses it as well, it seems to be more reliable and future-proof (snejus/beetcamp@46c51eb)

• msgspec I'm honestly very unsure about. Mostly because it isn't packaged in a lot of linux distros, and e.g. nixpkgs unstable (rolling) still has an old version even though it has been 3 weeks since a new version has come out. I know we don't package this plugin anywhere, so package versions shouldn't matter as it'll come from PyPi. But what I'm mostly getting at is that the package doesn't seem to be widely supported.

https://jcristharif.com/msgspec/why.html

[amogus07]

I'm probably not gonna be able to get much done in the next few weeks since I have some important school work to to... I'll just push what I have right now.