aquelemiguel / parrot Goto Github PK

Description

The !seek command is blocking the bot. The larger the seek, the more time Parrot spends processing the request.

Seeking to further points in the track always yields larger and larger block times and is not relative to the current track position. This means that even if, for instance, the user successfully seeks to 4:00, seeking to 4:05 takes a little bit longer than the initial one.

Expected vs. Observed

Repro Steps

1. Play any moderately lengthy track.
2. Try to seek to a timestamp of about 10 seconds. The bot should stop playing for a while but resume activity after a few seconds.
3. Now seek to a larger timestamp of a couple of minutes. Now the bot should be irresponsive for a substantially larger amount of time.

Environment

Screens

N/A

Explore the possibility of bundling a dependencies like FFmpeg into the executable.

In larger servers, there needs to be a voting system in place for skipping songs so the queue cannot be griefed.
This means performing the following changes:

• !skip / !s should no longer immediately skip a track but instead display a new reaction-based voting system where x votes (maybe other metric?) skip it.
• !forceskip / !fs should be restricted to members with a DJ permission and immediately skips the track. This is identical to the current behavior of !skip.

More on this DJ permission on a later issue because it should also be used by other destructive commands like !clear and !remove or even !seek.

There are possibly other scenarios where leaving a voice channel breaks the bot in unexpected ways however fixing these two main ones identified in the section below would be a nice start.

Known issues

• Disconnecting from a voice channel either manually or via the !leave command while a song is being played makes it so subsequent commands are irresponsive (such as !play and !np). If you try !summon, Parrot replies it's rejoining the channel but nothing happens.
• Summoning, leaving (either manually or through !leave) and then queueing a song won't make the bot rejoin. You'll have to !summon and only then the song will autoplay.

Allows for removing tracks in the middle of the queue.
Example usage: !remove 2 pops the second enqueued track.

Add a new section to the README listing the minimum permissions that should be granted to Parrot.
As of now, these are:

• View Channels
• Send Messages
• Manage Messages
• Embed Links
• Read Message History (untested!)
• Add Reactions
• Connect
• Speak
• Deafen Members
• Use Voice Activity

Discord seems to be going in the direction of dropping bot prefixes for slash commands, expecting all bots to migrate to it by April 2022. See The Future of discord.py.

Implement a new command: !autopause

The listening party mode works as a toggle: starts as OFF -> !autopause -> becomes ON and vice-versa

The idea is that, if the listening party mode is ON, after each track, the queue stops. This allows people to discuss the track they heard without having to pause the next song, and seeking it to 00:00 when they are ready. For example, if track 1 is playing, and tracks 2 and 3 are enqueued, after track 1 finishes, nothing more plays. Only after typing !resume does it proceed to the next track.

Rationale

Documentation helps newcomers grasp how Parrot does what it does (best). It also helps current maintainers because we can't rely on having it all committed in memory.

A nice static website generated by cargo doc which looks like the default Rust codebase doc site would be nice to host on Github pages of this repository for instance.

Description

1. Document the main modules (for example commands) using the standard Rust doc style.

2. Create a better README splitting it into multiple markdown files stored under /docs and in the README link to them. Main sections: how to run either by installing dependencies (leave installation if possible linked to external resources so that we don't have to repeat what others did and avoid having to describe it for every OS) or using docker, how to run tests, contributing guide

3. Automatize the generation of the static site. Should amount to at the end of the main workflow (or in a new one) to do cargo doc or something similar and push it to Github pages.

4. Document discord and spotify token generation.

Reference

Documentation

• https://doc.rust-lang.org/cargo/commands/cargo-doc.html
• https://github.com/serenity-rs/serenity/blob/current/src/cache/mod.rs#L1
• https://github.com/serenity-rs/serenity/blob/current/src/cache/mod.rs#L106

Github Pages

• https://github.com/s0/git-publish-subdir-action

• Across the application, there are a handful of commands that lock the call mutex for more time than necessary (lock is done at the start of the command and released at the end).

• A critical overview of every command is expected to ensure locks are only done when strictly necessary as this prevents other commands from executing until the mutex is released.

• The famous example is #1.

Rationale

Chaining commands allows for faster and easier use of the bot.

Description

Commands should be able to be chained using some kind of and operator, sort of like Unix's &&. One could copy-paste a chain of commands and use that.

For example, suppose you want to pause, go to the start of the track and resume it. You would have to type and enter each of the following:

!pause + !seek 00:00 + !resume

The idea of this feature is to have the following syntax:

!pause && !seek 00:00 && !resume

Note that here I used the && Unix and operator, but we could use a more friendly operator like +.

Parrot is capable of directly playing local music files, downloading from single video and playlist URLs. This appears to be an issue with YTDL itself but further analysis is required.

As described in Rust's documentation, the cargo.lock should be added to the repository in binaries.

This ticket will also fix possible version mismatches that can happen, as it could be seen in #103.

After the recent changes in c53cb87 (specifically, by the reqwest dependency), one gets this error when trying to build the Docker image:

error: failed to run custom build command for `openssl-sys v0.9.67`

Caused by:
  process didn't exit successfully: `/parrot/target/release/build/openssl-sys-d8b0cdcd87927011/build-script-main` (exit status: 101)
  --- stdout
  cargo:rustc-cfg=const_fn
  cargo:rerun-if-env-changed=AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR
  AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_LIB_DIR
  OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR
  AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_INCLUDE_DIR
  OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_DIR
  AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_DIR
  OPENSSL_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_NO_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG
  cargo:rerun-if-env-changed=OPENSSL_STATIC
  cargo:rerun-if-env-changed=OPENSSL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR
  run pkg_config fail: "`\"pkg-config\" \"--libs\" \"--cflags\" \"openssl\"` did not exit successfully: exit status: 1\n--- stderr\nPackage openssl was not found in the pkg-config search path.\nPerhaps you should add the directory containing `openssl.pc'\nto the PKG_CONFIG_PATH environment variable\nNo package 'openssl' found\n"

  --- stderr
  thread 'main' panicked at '

  Could not find directory of OpenSSL installation, and this `-sys` crate cannot
  proceed without this knowledge. If OpenSSL is installed and this crate had
  trouble finding it,  you can set the `OPENSSL_DIR` environment variable for the
  compilation process.

  Make sure you also have the development packages of openssl installed.
  For example, `libssl-dev` on Ubuntu or `openssl-devel` on Fedora.

  If you're in a situation where you think the directory *should* be found
  automatically, please open a bug at https://github.com/sfackler/rust-openssl
  and include information about your system as well as this message.

  $HOST = aarch64-unknown-linux-gnu
  $TARGET = aarch64-unknown-linux-gnu
  openssl-sys = 0.9.67

  It looks like you're compiling on Linux and also targeting Linux. Currently this
  requires the `pkg-config` utility to find OpenSSL but unfortunately `pkg-config`
  could not be found. If you have OpenSSL installed you can likely fix this by
  installing `pkg-config`.

  ', /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/openssl-sys-0.9.67/build/find_normal.rs:174:5
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
error: build failed
The command '/bin/sh -c cargo build --release' returned a non-zero code: 101

Commands that perform modifications to the queue or make the bot connect/disconnect should be locked to a role or be put behind a voting system (relates to #44). As of writing, this is every command except for informative ones, which should stay unmodified:

• now_playing
• queue
• genius/explain
• genius/lyrics

If a user does not have said role and tries to use the command, a message informing their lack of permissions should be sent to the channel. A good discussion point would be how to tell the bot which role is the DJ role (relates to #45's discussion about where to store the prefix).

Currently, the Docker image generated by the Docker file is too big, 2.4Gb big.

We should aim to reduce it the best we can.

Explore integration with Genius API to explain lyrics/songs.

Duration until play is badly calculated. Below is an example where there were no other musics in the queue.

Rationale

Right now, there are a few commands that require a pagination system.
!queue already has it and the code for it is relatively large. It'd be great to generalize this system for usage in other commands.

Description

The response from Genius commands !explain and !lyrics can exceed the maximum size of the embed (especially the lyrics). If it does, the response should be split into multiple pages. A user can switch to a different page via reactions.

Reference

See #19 implementation.

Rationale

strings.rs is a really good starting point for future translations, so we should move as many strings to there.

Currently there is no limit to the number of tracks in the queue. It'd be nice to have multiple pages to prevent a wall of text if there are dozens of songs.

A command that shuffles the current queue.

Usage

• !shuffle

Running the play command with a YouTube playlist link freezes Parrot and makes it unable to handle new commands before all songs are enqueued. Possibly just a case of the call mutex not being released.

A toggle that makes Parrot clean after itself. For each message Parrot sends, after the specified delay it cleans both response and request messages. It should only be callable by guild members with the Manage Messages permission.

Usage

• !autoclean defaults the delay to something like 5 seconds
• !autoclean <delay in seconds>

Rationale

If the bot is the only thing in a voice channel it should automatically stop playback and disconnect to save resources. If the bot is kicked from the voice channel or leaves on it's own a play command should automatically summon it, assuming it's not already in another channel.

Description

Automatically disconnect the bot if it's the only thing in a voice channel (either immediately or after a set time). Add a summon hook to the play command.

This version supports yt-dlp, so we no longer need to be dependent on the current branch.

Docker CI image build not working since .env is required.

Self-explanatory.

Playing songs while on a different channel than the bot should not be possible. This is a blocker for big servers with a lot of voice rooms.

Expected vs. Observed

Repro Steps

1. Sending a play command to the bot when in a different channel.
2. Works 😢

Rationale

Most people use Spotify to listen to tracks. They also usually have playlists on Spotify. It would be nice to have support for on-the-fly conversion of Spotify links.

Description

When a Spotify link is given, fetch the artist and title of the track (or tracks, in the case of a playlist), and then feed them into ytdl_search as usual.

Forcing users to use the ! prefix can cause conflicts with other bots on the server.

Should this be an environment variable (easier but requires redeploying) or does it justify a whole new !prefix command usable only by admins (harder but allows for changing on the fly)? Requesting your inputs here, @afonsojramos and @joao-conde.

Regardless of the chosen approach:

• The bot's activity should also reflect the prefix change.
• Existing messages that reference the exclamation mark prefix should now use the current prefix.

We're only using the youtube_dl crate for fetching the songs within a playlist.
This is overkill since we're using yt-dlp as well. Investigate executing a yt-dlp command (possibly with the --flat-playlist flag).

Description

When playing long songs, such as radios or playlists (which usually run about an hour long), the bot suddenly stops playing audio and stops responding to commands.

Expected vs. Observed

Repro Steps

1. !play a long video (for example, this one)
2. Listen until the video ends
3. Attempt to add commands later in the video's runtime

Environment

Rationale

/remove 0 currently replies with an error message. Should this behaviour change?

Description

I propose that a /remove 0 skips the current song, as the tooltip even suggests that 0 is the currently playing song.

Reference

Rationale

A command to display the Parrot version used.

Description

A !version command that sends a discord message with the Parrot version running.

The estimated time until play is being calculated incorrectly - it should be the sum of the all the non-elapsed song durations before.
For instance, if song A is currently being played and is at 4:00 / 30:00 and song B is next and is 2:00 minutes long, then song C's estimated time until play should be 30:00 - 4:00 + 2:00 = 28:00.

Example

Allows for pushing songs to the top of the queue to be played up next.

Usage

• !playtop <song> pushes a new track to the top of the queue.

Notes

• If the bot isn't playing anything, the command should push to the first position.
• However, if it is currently playing, it should place the new track in the queue's second position as the topmost element will be the track that's now playing.

Rationale

Currently, when selecting a track to be removed from the queue, it isn't immediately obvious how it is going to work.

Description

The UX of selecting the track to be removed could be improved with a tooltip that better clarifies how the queue indexing works. Additionally, the index parameter could be converted to an INTEGER option.

Reference

Application Command Option Type

Add a Dockerfile for people to be able to deploy the bot in a containerized way.

Always good to bump Serenity. Also unlocks #97.

Implement a !lyrics command that finds the currently playing song via the Genius API and returns its lyrics.

Description

Adding a livestream (for example, a music radio) to the queue makes some commands stop working. The /play command stops showing which video was added to the queue (even if the video has a fixed length), while /queue and /np show error messages. The /skip command functions as expected, and the commands start working again after livestream playback ends.

Expected vs. Observed

Repro Steps

1. /play two regular videos
2. Verify that the bot states which videos were added to the queue
3. Check that /np and /queue work properly
4. /play an ongoing livestream (for example, this one)
5. Check that the bot only says "Searching..."
6. Verify that /queue throws an error message while /np functions as expected
7. /play a regular video
8. Verify that the bot does not say which video was added
9. /skip videos until the livestream starts
10. Verify that both /np and /queue throw error messages
11. /skip the livestream
12. Verify that /play, /np and /queue function normally (the first one with regular videos)

Environment

Right now, there are some instances where code is needlessly duplicated:

• Before playing, the !play command summons the bot into a voice chat if it wasn't in one before.
  The !summon command does the same.

• The !play and !playtop commands do exactly the same except for the position where the new track is inserted.
  So, for instance, changing the behavior of adding playlists requires one to change both files. (This is very bad!)

• If a new track was enqueued via !play or !playtop and the queue was previously empty, a Now Playing message is sent.
  However, the user can also get this message via !nowplaying and code is duplicated there.

• Maybe others I'm not seeing right now.

Solving this comes down to identifying the duplicate code and exporting it to its own functions to allow sharing between commands.

Changes to the queue (either new tracks being added or a track that finished playing) should automatically update the queue response interaction instead of the user having to click the arrow buttons to refresh it.

Visually, the queue button components could use a little change:

• Disable buttons when it's not possible to navigate to a specific page
• Use alt-codes instead of emojis
• Use primary blurple color instead of grey

Current

Expected

Since the introduction of slash commands on #89, new commands have been harder to test because new global application commands are only available in guilds after 1 hour. It doesn't seem right that we have to wait this much time while developing or reviewing PRs.

I also dislike using create_guild_application_commands momentarily and having to remember to replace it in the end. Also I'm too lazy to edit someone's PR locally to use guild commands.

Perhaps we could have some sort of debug flag that, when used, registered commands to the provided guild. Otherwise, it'd register to the global Discord command framework.

Rationale

When playing a playlist, you sometimes want to shuffle the playlist before playing it. Sometimes you might even want to play it right after the current track, or even interrupt the current track and skip right away. These can be all achieved via flags and it was a much appreciated feature from Groovy.

Description

When using the play command, adding a flag modifies the behavior of the play command.

These were the available flags in Groovy (with the exception of -choose, which seems more appropriate as a search command). They were also available as shorthand flags with the first letter only:

• -all: This instructs Groovy to queue all returned songs (...) when you queue a YouTube video in a playlist.
• -shuffle: (...) randomize the order of songs in the playlist before it's added to the queue (...)
• -next: (...) the song you queued is placed directly after the playing track in the queue. (...)
• -jump: Instantly jumps to the track you just queued. (...)
• -reverse: Queue the playlist in reverse order.

Explore the possibility of having unit tests and CI.

Parrot should server deafen itself to reduce both its and Discord's bandwidth usage.
This also protects the users' privacy by ensuring them we're not silently recording anything.

Notes

• Assuming you haven't given your test bot the administrator role, implementing this will require you to grant it the "Deafen Members" permission.

Reference

• Why does Groovy (RIP) deafen itself?

Rationale

Discord has input validation built into the app, however, Serenity has not released/implemented all of them, like in create_application_command.rs#L163 which would help in #93/#95.

Description

When Serenity releases the new version, implement input validation with the built-in tools, instead of manual validation, where possible. Example: remove doesn't need to check if the input is <=0.

SponsorBlock is an open-source crowdsourced browser extension and open API for skipping sponsor segments in YouTube videos. It also keeps track of non-music segments for music videos.

The idea would be enqueueing a song, using the API to search for non-music segments (e.g., cinematic parts in music videos, credits or intermissions) and automatically skip them when the playtime enters that segment.

Possible usage

• !sponsorblock as an on/off toggle for this feature

Implementation

Triggering the seek action at specific timestamps might raise some issues.

• Should the bot periodically (every second or less) fire an event checking whether the song entered a non-music segment?
• We could also schedule events for those specific timestamps but we'd have to be extra careful adjusting the scheduling because of time altering commands like !pause and !seek.
• ...