Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Red music bot support (and fix a small null reference bug) #268

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

w-biggs
Copy link

@w-biggs w-biggs commented Dec 8, 2023

This PR adds support for Red as a music bot.

The way that I've done this integration (since Red is a self-hosted bot) is to make the name of the bot configurable via .configuration. If this would be better done some other way I'd be happy to make changes to that effect.

Additionally, a issue I've run into is that artist and song title will sometimes be reversed (especially if the upload channel is the name of the artist, or a "topic channel"); this PR adds a check for which order has the most listeners on Last.fm and uses the one that's higher. If that's too big of a change, it might make sense to check for "- Topic" specifically and flip the artist/title around only if it's a topic channel (which will miss more wrong-way-around scrobbles, but be a smaller change.)

I also fixed #267 in the process (which was preventing me from running the bot in the first place.)

(I'd also be happy to DM anyone an invite link for my personal Red bot instance for testing, if needed)

Copy link
Member

@th0mk th0mk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for the PR. It would work, but I'm still going to request some changes.

I try to keep the amount of user/guildsettings as low as possible for simplicity and complexity sake. So I would rather not add a guild setting for the name. Instead I would propose to check the bot for either a name or a role to determine if it is the Red music bot.

So for example we can determine if a music bot is Red by:

  • The name starts with 'red'
  • Or it has a role called 'red music bot'

Doing a role check is fairly straightforward, for example:

if (context.User is IGuildUser guildUser)
{
    var role = context.Guild.Roles.FirstOrDefault(x => string.Equals(x.Name, "red music bot", StringComparison.OrdinalIgnoreCase));
    if (role != null && guildUser.RoleIds.Contains(role.Id))
    {
        
    }
}

A role is easy to add for any server staff and it would still allow custom names for the bot itself.

Another point is the checking with the Last.fm API if the artist - track is reversed. I don't think we can do that, it would use too many API calls. Instead I'd suggest to use the GetTrackFromDatabase method to check if both versions exist. If one does and the other does not you know which one is right.

Let me know if you have any other questions, happy to DM as well (Frikandel on Discord)

@w-biggs
Copy link
Author

w-biggs commented Feb 9, 2024

Sorry for the delay, I'll try to work on those changes within the next week!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullReferenceException if UseShardEnvConfig is false
2 participants