Merge pull request 'v3.6.0 (music commands trimmed)' (#36) from dev into master

Reviewed-on: #36

docs/commands.md

@@ -60,14 +60,10 @@ Anyone can use these. `/rps`, `/rpsstats` and `/rpsleaderboard` work in servers
 | `/queue` | | **DJ** role | Shows the queue |
 | `/volume` | `value`* (0 to 150) | **DJ** role | Sets the volume |
 | `/music toggle` | | Manage Server | Turns music on or off in this server |
-| `/music skip` | | **DJ** role | Skips the current song |
-| `/music pause`, `/music resume` | | **DJ** role | Pauses or resumes |
-| `/music stop` | | **DJ** role | Stops and clears the queue |
-| `/music repeat` | `mode`* (off, song, queue) | **DJ** role | Repeats the song, the queue, or nothing |
 | `/music remove` | `position`* (its number in `/queue`) | **DJ** role | Removes a queued song |
 | `/music move` | `from`*, `to`* | **DJ** role | Moves a queued song |
 
-Requires Lavalink. See [Getting started](getting-started.md#optional-music-with-lavalink).
+Pause, skip, repeat and stop are buttons on the now-playing message. Requires Lavalink. See [Getting started](getting-started.md#optional-music-with-lavalink).
 
 ## Meeting rooms
 

docs/features.md

@@ -68,7 +68,7 @@ Lavalink-powered playback. Code: `internal/music`, commands in `internal/command
 
 - `/play` takes a name or a URL, `/queue` shows the queue, `/volume` sets 0 to 150 percent. These need a role called **DJ**.
 - The now-playing message has buttons for playback and repeat. Buttons also check for the DJ role.
-- `/music toggle` (Manage Server) turns music on or off in a server. `/music skip`, `pause`, `resume`, `stop`, `repeat`, `remove` and `move` control playback and the queue (DJ role).
+- `/music toggle` (Manage Server) turns music on or off in a server. `/music remove` and `/music move` edit the queue (DJ role).
 - The queue and now-playing state are kept in the database, and playback can also be controlled by writing to the `music_commands` table. The hosted Velox dashboard uses this (see [Database](database.md)); you never need it, the commands above do the same.
 - Music needs a Lavalink server. Without `LAVALINK_HOST` the rest of the bot works and music does not.
 

internal/commands/help/help.go

@@ -1,6 +1,7 @@
 package help
 
 import (
+	"fmt"
 	"log"
 	"unicode/utf8"
 
@@ -20,141 +21,53 @@ const (
 	maxFieldChars = 1024
 )
 
+// docsURL is where each command is explained in full, with its options and
+// who may use it.
+const docsURL = "https://velox.fernandovideira.com/docs/commands"
+
+// col is one short column of the help: a list of commands, no descriptions.
+// Fields are inline, so Discord lays three of them side by side and the help
+// is wide and short instead of narrow and long.
+func col(name string, lines ...string) *discordgo.MessageEmbedField {
+	v := ""
+	for i, l := range lines {
+		if i > 0 {
+			v += "\n"
+		}
+		v += l
+	}
+	return &discordgo.MessageEmbedField{Name: name, Value: v, Inline: true}
+}
+
 func fields() []*discordgo.MessageEmbedField {
 	return []*discordgo.MessageEmbedField{
+		col("Fun",
+			"`/ping` `/joke` `/coinflip`", "`/dice` `/rps` `/rpsstats`", "`/rpsleaderboard`"),
+		col("Music (DJ role)",
+			"`/play` `/queue` `/volume`", "`/music remove` `move`", "`/music toggle` (Manage Server)"),
+		col("Sessions and time",
+			"`/schedule create`", "`/schedule list`", "`/schedule reschedule`", "`/timezone set` `show`"),
+		col("Tickets",
+			"Open one from the panel.", "`/ticket add` `close`", "`/ticket claim` `remove` (staff)", "`/tickets setup` `settings`", "`/tickets category` `panel` `list`"),
+		col("Security",
+			"`/security status` `settings` `history`", "`/security toggle`", "`/security joingate` `words` `raid`", "`/security antinuke` `heat` `alert-dms`", "`/security trust` `release` `end-raid`", "`/security alerts` `quarantine-role`", "`/security snapshot` `restore`", "`/lockdown` `/unlock`"),
+		col("Moderation",
+			"`/moderation purge`", "`/moderation timeout` `untimeout`", "`/moderation kick` `ban` `unban`", "`/moderation slowmode`", "`/moderation bans` `timeouts`"),
+		col("Meeting rooms",
+			"`/meeting add-lobby`", "`/meeting remove-lobby` `lobbies`", "`/meeting lock` `lock-private`", "`/meeting unlock`", "`/meeting invite` `uninvite`"),
+		col("Leveling",
+			"`/rank` `/leaderboard` `/rewards`", "`/slvl toggle` `status`", "`/slvl set` `set-channel`", "`/slvl set-message`", "`/slvl set-reward` `remove-reward`"),
+		col("Projects",
+			"`/projects list`", "`/projects create`", "`/projects add-helper`", "`/projects close`"),
+		col("Welcome and roles",
+			"`/config setwelcomechannel`", "`/config setwelcomemessage`", "`/config setwelcomedm`", "`/config setwelcomegif`", "`/config setdefaultrole`"),
+		col("Logging and Twitch",
+			"`/config setlogchannel`", "`/config enablelogging` `disablelogging`", "`/config addstreamer` `removestreamer`", "`/config streamers`", "`/config toggletwitch`", "`/config settwitchnotificationchannel`"),
+		col("Settings and beta",
+			"`/config view` `clear`", "`/config beta-features` (server owner)", "Bot owner, in a DM with me:", "`/beta set` `off` `status`"),
 		{
-			Name: "Fun",
-			Value: "" +
-				"`/ping` — Pong!\n" +
-				"`/joke [type]` — Random joke (optional category)\n" +
-				"`/coinflip` — Flip a coin\n" +
-				"`/dice <expr>` — Roll dice (e.g. `d20`, `2d20+1`, `2#d20+1`)\n" +
-				"`/rps <hand>` — Rock Paper Scissors (server only)\n" +
-				"`/rpsstats` — Your RPS score (server only)\n" +
-				"`/rpsleaderboard` — Top RPS scores (server only)",
-		},
-		{
-			Name: "Music (requires **DJ** role)",
-			Value: "" +
-				"`/play <query>` — Play a song (search or URL)\n" +
-				"`/queue` — Show the current queue\n" +
-				"`/volume <0-150>` — Set playback volume\n" +
-				"`/music skip|pause|resume|stop` — Control playback\n" +
-				"`/music repeat <off|song|queue>`, `/music remove <n>`, `/music move <from> <to>` — Repeat and reorder\n" +
-				"`/music toggle` — Turn music on or off (**Manage Server**)",
-		},
-		{
-			Name: "Scheduling (1:1 sessions)",
-			Value: "" +
-				"`/schedule create user:@someone datetime:\"YYYY-MM-DD HH:MM\" [description]` — Create a session\n" +
-				"`/schedule list` — List your upcoming sessions\n" +
-				"`/schedule reschedule id:<id> datetime:\"YYYY-MM-DD HH:MM\"` — Request a new time\n\n" +
-				"Invites are sent via DM; react with ✅ / ❌ / 🔁 to respond.",
-		},
-		{
-			Name: "Timezone",
-			Value: "" +
-				"`/timezone set <IANA>` — Set your timezone (e.g. `Europe/Lisbon`)\n" +
-				"`/timezone show` — Show your timezone",
-		},
-		{
-			Name: "Projects",
-			Value: "" +
-				"`/projects list` — List active projects\n" +
-				"`/projects create name:\"...\" [description]` — Create a project (**Manage Server**)\n" +
-				"`/projects add-helper project-id:<id> user:@someone` — Add helper (**Manage Server**)",
-		},
-		{
-			Name: "Tickets",
-			Value: "" +
-				"Open a ticket from the panel: you get a private thread with the staff.\n" +
-				"`/ticket add user:@someone` — Add someone to your ticket\n" +
-				"`/ticket close [reason]` — Close the ticket (opener or staff)\n" +
-				"`/ticket claim`, `/ticket remove user:@someone` — Staff only\n" +
-				"`/tickets setup`, `/tickets settings`, `/tickets category add|edit|remove|list`, `/tickets panel`, `/tickets list` — Set up (**Manage Server**)",
-		},
-		{
-			Name: "Security (anti-raid & anti-nuke)",
-			Value: "" +
-				"`/security status`, `/security settings`, `/security history` — What's on, every setting, what happened\n" +
-				"`/security toggle` — Join Gate, raid mode, anti-nuke, heat on/off (off: owner or trusted)\n" +
-				"`/security joingate`, `words`, `raid`, `antinuke`, `heat`, `alert-dms` — Tune thresholds and filters (owner or trusted)\n" +
-				"`/security alerts`, `/security quarantine-role`, `/security release`, `/security end-raid` — Owner or trusted\n" +
-				"`/security trust add|remove` — Owner only\n" +
-				"`/security snapshot` — Save roles and channels; `/security restore` — Rebuild what a nuke deleted (owner or trusted)\n" +
-				"`/lockdown channel|server|joins`, `/unlock …` (unlock: owner or trusted)",
-		},
-		{
-			Name: "Meetings (**Manage Server**)",
-			Value: "" +
-				"`/meeting add-lobby channel:#voice` — Add a voice lobby that creates meetings (up to 10)\n" +
-				"`/meeting remove-lobby channel:#voice` — Stop using a voice lobby\n" +
-				"`/meeting lobbies` — List the voice lobbies\n" +
-				"`/meeting lock` — Lock your meeting room\n" +
-				"`/meeting lock-private` — Lock + hide your meeting room\n" +
-				"`/meeting unlock` — Unlock your meeting room\n" +
-				"`/meeting invite user:@someone` — Allow a user to join\n" +
-				"`/meeting uninvite user:@someone` — Remove a user's permission",
-		},
-		{
-			Name: "Moderation (focused tools)",
-			Value: "" +
-				"`/moderation purge amount:<1-100>` — Delete recent messages (**Manage Messages**)\n" +
-				"`/moderation timeout user:@someone duration:<10m|1h|...> [reason]` — Timeout member (**Moderate Members**)\n" +
-				"`/moderation kick user:@someone [reason]` — Kick member (**Kick Members**)\n" +
-				"`/moderation ban user:@someone [delete_days] [reason]` — Ban member (**Ban Members**)\n" +
-				"`/moderation unban user_id:<id> [reason]` — Unban by user ID (**Ban Members**)\n" +
-				"`/moderation untimeout user:@someone [reason]` — Remove timeout (**Moderate Members**)\n" +
-				"`/moderation slowmode channel:#channel seconds:<0-21600>` — Set slowmode (**Manage Channels**)\n" +
-				"`/moderation bans`, `/moderation timeouts` — Who is banned, who is timed out",
-		},
-		{
-			Name: "Leveling",
-			Value: "" +
-				"`/slvl toggle` — Enable/disable leveling (**Manage Server**)\n" +
-				"`/slvl set user:@someone level:<n>` — Set user level (**Manage Server**)\n" +
-				"`/slvl set-channel channel:#channel` — Level-up message channel (**Manage Server**)\n" +
-				"`/slvl set-message message:\"...\"` — Custom level-up message (**Manage Server**)\n" +
-				"`/slvl set-reward level:<n> role:@role`, `/slvl remove-reward level:<n>` — Role rewards (**Manage Server**)\n" +
-				"`/slvl status` — Show the leveling settings (**Manage Server**)\n" +
-				"`/rank [user]` — Your rank card (server only)\n" +
-				"`/leaderboard` — Top levels (server only)\n" +
-				"`/rewards` — List configured rewards (server only)",
-		},
-		{
-			Name: "Twitch live notifications (**Manage Server**)",
-			Value: "" +
-				"`/config addstreamer username:<twitch-name>` — Add a streamer\n" +
-				"`/config removestreamer username:<twitch-name>` — Remove a streamer\n" +
-				"`/config settwitchnotificationchannel channel:#channel` — Set notification channel\n" +
-				"`/config toggletwitch enabled:<bool>`, `/config streamers` — Turn alerts on or off, list streamers",
-		},
-		{
-			Name: "Welcome / onboarding (**Manage Server**)",
-			Value: "" +
-				"`/config setwelcomechannel channel:#channel` — Channel for welcome messages\n" +
-				"`/config setwelcomemessage message:\"...\"` — Welcome text (supports `{user}`, `{server}`)\n" +
-				"`/config setwelcomedm message:\"...\"` — DM template (supports `{user}`, `{server}`)\n" +
-				"`/config setwelcomegif url:<image-url>` — GIF/image used in welcome embed\n" +
-				"`/config setdefaultrole role:@role` — Role granted to new members\n" +
-				"`/config view`, `/config clear setting:<...>` — See or clear these settings",
-		},
-		{
-			Name: "Logging (**Manage Server**)",
-			Value: "" +
-				"`/config setlogchannel channel:#channel` — Set log output channel\n" +
-				"`/config enablelogging` — Enable event/moderation logging\n" +
-				"`/config disablelogging` — Disable event/moderation logging",
-		},
-		{
-			Name: "Beta features",
-			Value: "" +
-				"`/config beta-features [enabled]` — Let testers use beta features here (server owner only)\n" +
-				"Bot owner: `/beta` in a DM with the bot — set the beta server and role, list or remove servers",
-		},
-		{
-			Name:  "Other",
-			Value: "`/help` — Show this help in DMs",
+			Name:  "Everything in detail",
+			Value: "Options, permissions and what each command does: " + docsURL + "\nMost setup commands need **Manage Server**. `/help` shows this list again.",
 		},
 	}
 }
@@ -168,7 +81,7 @@ func embeds(author *discordgo.MessageEmbedAuthor) []*discordgo.MessageEmbed {
 	var out []*discordgo.MessageEmbed
 	cur := &discordgo.MessageEmbed{
 		Title:       "Velox Bot Help",
-		Description: "Command reference. Some commands require server permissions or roles.",
+		Description: "Every command, by area. Some need server permissions or a role.",
 		Color:       0xFFA500,
 		Author:      author,
 	}
@@ -227,8 +140,8 @@ func HelpHandler(s *discordgo.Session, i *discordgo.InteractionCreate) {
 		// when "no mutual guilds" appears for someone who is in a server with
 		// the bot: a command run through a user install, or in a server the bot
 		// isn't actually a member of, has no shared server for Discord to see.
-		log.Printf("help: DM not possible, showing it here (guild %q, context %d, installs %v): %v",
-			i.GuildID, i.Context, i.AuthorizingIntegrationOwners, err)
+		log.Printf("help: DM not possible, showing it here (guild %q, context %d, installs %v, %s): %v",
+			i.GuildID, i.Context, i.AuthorizingIntegrationOwners, botMembership(s, i.GuildID), err)
 		note := "I couldn't DM you (your DMs may be off), so here it is."
 		_, _ = s.InteractionResponseEdit(i.Interaction, &discordgo.WebhookEdit{Content: &note, Embeds: &list})
 		return
@@ -249,3 +162,21 @@ func sendByDM(s *discordgo.Session, userID string, list []*discordgo.MessageEmbe
 	}
 	return nil
 }
+
+// botMembership says whether the bot is really a member of the server, which
+// is what Discord means by "mutual guilds". A server where the app was added
+// with only the applications.commands scope runs slash commands fine but has
+// no bot user in it.
+func botMembership(s *discordgo.Session, guildID string) string {
+	if s == nil || s.State == nil || s.State.User == nil {
+		return "bot user unknown"
+	}
+	inCache := false
+	if g, err := s.State.Guild(guildID); err == nil && g != nil {
+		inCache = true
+	}
+	if _, err := s.GuildMember(guildID, s.State.User.ID); err != nil {
+		return fmt.Sprintf("bot is NOT a member of that server (in gateway cache: %v): %v", inCache, err)
+	}
+	return fmt.Sprintf("bot is a member of that server (in gateway cache: %v)", inCache)
+}

internal/commands/help/help_test.go

@@ -113,3 +113,20 @@ func TestHelpInADMIsShownDirectly(t *testing.T) {
 		t.Errorf("one direct answer, no DM: %+v", f)
 	}
 }
+
+// The help is meant to be wide and short: inline fields, so Discord puts
+// three side by side, and it should fit in one message.
+func TestHelpIsCompactAndInline(t *testing.T) {
+	if n := len(embeds(&discordgo.MessageEmbedAuthor{Name: "Velox Bot"})); n != 1 {
+		t.Errorf("the help should fit in one message, it takes %d", n)
+	}
+	inline := 0
+	for _, f := range fields() {
+		if f.Inline {
+			inline++
+		}
+	}
+	if inline < len(fields())-2 {
+		t.Errorf("only %d of %d fields are inline", inline, len(fields()))
+	}
+}

internal/commands/music/public/player.go

@@ -10,10 +10,10 @@ import (
 	"github.com/bwmarrin/discordgo"
 )
 
-// The player controls. Everything a dashboard can do to the queue (skip, pause,
-// resume, stop, repeat, remove and move a track) is also a command, so a
-// server never needs one. They need the DJ role, like /play, /queue and
-// /volume. Turning music on or off needs Manage Server.
+// Queue editing. Pausing, skipping, repeating and stopping are buttons on the
+// now-playing message, so the only queue controls that need commands are
+// removing a song and moving one. They need the DJ role, like /play, /queue
+// and /volume. Turning music on or off needs Manage Server.
 
 func positionOption(name, desc string) *discordgo.ApplicationCommandOption {
 	return &discordgo.ApplicationCommandOption{
@@ -27,15 +27,6 @@ func playerOptions() []*discordgo.ApplicationCommandOption {
 		return &discordgo.ApplicationCommandOption{Type: discordgo.ApplicationCommandOptionSubCommand, Name: name, Description: desc, Options: o}
 	}
 	return []*discordgo.ApplicationCommandOption{
-		sub("skip", "Skip the current song (DJ)"),
-		sub("pause", "Pause playback (DJ)"),
-		sub("resume", "Resume playback (DJ)"),
-		sub("stop", "Stop playback and clear the queue (DJ)"),
-		sub("repeat", "Repeat the current song, the whole queue, or nothing (DJ)",
-			&discordgo.ApplicationCommandOption{
-				Type: discordgo.ApplicationCommandOptionString, Name: "mode", Description: "What repeats", Required: true,
-				Choices: []*discordgo.ApplicationCommandOptionChoice{{Name: "off", Value: "off"}, {Name: "song", Value: "song"}, {Name: "queue", Value: "queue"}},
-			}),
 		sub("remove", "Remove a song from the queue (DJ)", positionOption("position", "Its number in /queue")),
 		sub("move", "Move a song to another place in the queue (DJ)",
 			positionOption("from", "Its number in /queue"), positionOption("to", "The number it should have")),
@@ -57,44 +48,19 @@ func title(e *music.TrackEntry) string {
 	return strings.TrimSpace(e.Track.Info.Title)
 }
 
-// handlePlayer runs a player subcommand. The caller has checked that music is
-// on and that the member has the DJ role.
+// handlePlayer runs a queue subcommand. The caller has checked that music is
+// on and that the member has the DJ role. Pause, skip, repeat and stop are
+// buttons on the now-playing message, so only what has no button is a command.
 func handlePlayer(s *discordgo.Session, i *discordgo.InteractionCreate, sub *discordgo.ApplicationCommandInteractionDataOption) {
 	g := i.GuildID
 	args := map[string]*discordgo.ApplicationCommandInteractionDataOption{}
 	for _, o := range sub.Options {
 		args[o.Name] = o
 	}
-	current, _ := music.GetQueue(g)
-	needsPlayer := sub.Name != "repeat" && sub.Name != "remove" && sub.Name != "move"
-	if needsPlayer && current == nil {
-		shared.RespondEphemeral(s, i, "Nothing is playing.")
-		return
-	}
 
 	var err error
 	var msg string
 	switch sub.Name {
-	case "skip":
-		err, msg = music.Skip(g), "Skipped."
-	case "pause":
-		err, msg = music.Pause(g, true), "Paused. `/music resume` continues."
-	case "resume":
-		err, msg = music.Pause(g, false), "Resumed."
-	case "stop":
-		err, msg = music.Stop(g), "Stopped, and the queue is cleared."
-	case "repeat":
-		switch args["mode"].StringValue() {
-		case "song":
-			music.SetRepeat(g, true, false)
-			msg = "The current song repeats."
-		case "queue":
-			music.SetRepeat(g, false, true)
-			msg = "The whole queue repeats."
-		default:
-			music.SetRepeat(g, false, false)
-			msg = "Nothing repeats."
-		}
 	case "remove":
 		n := int(args["position"].IntValue())
 		e, size := queueEntry(g, n)

internal/music/manager.go

@@ -686,23 +686,6 @@ func ToggleRepeatQueue(guildID string) (bool, bool) {
 	return repeatSong, repeatQueue
 }
 
-// RepeatFlags reports whether the current song or the whole queue repeats.
-func RepeatFlags(guildID string) (song, queue bool) { return repeatFlags(guildID) }
-
-// SetRepeat sets the repeat mode directly. At most one of the two applies;
-// asking for both repeats the song.
-func SetRepeat(guildID string, song, queue bool) {
-	if manager == nil {
-		return
-	}
-	gp := getOrCreateGuildPlayer(guildID)
-	manager.mu.Lock()
-	gp.RepeatSong = song
-	gp.RepeatQueue = queue && !song
-	manager.mu.Unlock()
-	syncState(guildID)
-}
-
 func repeatFlags(guildID string) (bool, bool) {
 	if manager == nil {
 		return false, false