Add seek command #344

whew-inc · 2020-01-22T03:53:09Z

This pull request...

Fixes a bug
Introduces a new feature
Improves an existing feature
Boosts code quality or performance

Description

Allows DJs and user that added the track to perform a seek on a playing track.

Purpose

Example: Going to a specific timestamp in a song compilation.

Relevant Issue(s)

No relevant issue AFAIK, only a planned feature:
https://github.com/jagrosh/MusicBot/projects/1#card-1542746

jagrosh

This is a feature I have kept on the backburner because just adding this command is not quite enough. The following are all related to seeking and should all be added at the same time:

seeking to a specific point in a song (what this command currently does)
seeking forward/backward a certain amount of time in a song
starting a song at a specific time (both via the play command as well as in playlists; youtube has a format for this and jmusicbot should probably adapt that same format)

Additionally, I want to keep time-related notation consistent throughout the bot.

src/main/java/com/jagrosh/jmusicbot/JMusicBot.java

src/main/java/com/jagrosh/jmusicbot/commands/music/SeekCmd.java

whew-inc · 2020-01-22T18:49:10Z

How do you think we should deal with invalid input in parseTime? Three options that came to mind are throw exception, optional and null.

whew-inc · 2020-01-22T19:03:23Z

By the way sorry if this PR is messy, this is my first one. If you got any suggestions on how I can do it better, I'm open to them :)

jagrosh · 2020-01-22T20:15:07Z

Since the time parsing will likely need to be able to handle both specific and relative times (+/-), a new class for the return type will likely be needed. Probably something like

public static class SeekTime
{
    public final int seconds;
    public final boolean relative;
    /* constructor here */
}

How do you think we should deal with invalid input in parseTime? Three options that came to mind are throw exception, optional and null.

Exception or null, I tend to use nulls but exceptions are useful if there are multiple fail-cases.

By the way sorry if this PR is messy, this is my first one. If you got any suggestions on how I can do it better, I'm open to them :)

Generally it's better to keep similar changes in a single commit, but I can squash before merging so it's not a huge deal.

jagrosh

For the parsing function, I think the format for relative times should just be the same but with a preceding + or -. For example, +04:20 will seek forward 4 minutes and 20 seconds, -20 will seek backwards 20 seconds, and 01:23 will seek directly to 1 minute 23 seconds into the song. The parsing function should be in the new TimeUtil you added (checking that the resulting time is valid for the current song should still be in the command).

src/main/java/com/jagrosh/jmusicbot/commands/music/SeekCmd.java

whew-inc · 2020-01-24T16:48:42Z

I like the idea with the + and - before the timestamp. I'll do the forward/backward seeking in the coming days.

src/main/java/com/jagrosh/jmusicbot/commands/music/SeekCmd.java

src/main/java/com/jagrosh/jmusicbot/utils/TimeUtil.java

src/main/java/com/jagrosh/jmusicbot/commands/music/SeekCmd.java

whew-inc · 2020-03-02T04:42:08Z

Seek with decimals, no timestamp separator : limit, lavaplayer seek error catching and some basic tests added.

And examples of course 👀

jagrosh

Nearly there!

jagrosh · 2020-03-02T15:49:33Z

src/main/java/com/jagrosh/jmusicbot/commands/music/SeekCmd.java

+        this.help = "seeks the current song" +
+                "\nExamples: `1:02:23` `+1:10` `-90`";


I wouldn't include this second line in the help, as every other command has a single-line brief help. Save the examples for if someone enters the command wrong (after the 'Expected format' error message later in this file)

src/main/java/com/jagrosh/jmusicbot/commands/music/SeekCmd.java

jagrosh · 2020-03-02T15:51:29Z

src/main/java/com/jagrosh/jmusicbot/commands/music/SeekCmd.java

+        TimeUtil.SeekTime seekTime = TimeUtil.parseTime(args);
+        if (seekTime == null)
+        {
+            event.replyError("Invalid seek! Expected format: " + arguments);


Put the second line of the help here instead of in the help itself.

jagrosh · 2020-03-02T15:52:04Z

src/test/java/com/jagrosh/jmusicbot/TimeUtilTest.java

@@ -0,0 +1,49 @@
+package com.jagrosh.jmusicbot;


license header

jagrosh · 2020-03-02T15:54:36Z

src/test/java/com/jagrosh/jmusicbot/TimeUtilTest.java

+        assertTrue(seek.relative);
+        assertEquals(-5000, seek.milliseconds);
+    }
+}


Good tests so far, although I also want to see assertFalse(seek.relative) for the seeks that are not relative, as well as some things that intentionally return null from parseTime

Merge with upstream

Timardo · 2020-06-17T22:23:56Z

So, is this finally able to become a real feature? I am really looking forward to this functionality.

realmar · 2020-06-26T16:17:06Z

Really looking forward to this feature! I tested it and get the following exception when trying to play a song, eg. !!play hello:

I tested it using openjdk 8 and 14 running on Ubuntu 20.04

[18:14:39] [ERROR] [LocalAudioTrackExecutor]: Error in playback of YQHsXMglC9A
com.sedmelluq.discord.lavaplayer.tools.FriendlyException: Something broke when playing the track.
 at com.sedmelluq.discord.lavaplayer.tools.ExceptionTools.wrapUnfriendlyExceptions(ExceptionTools.java:43)
 at com.sedmelluq.discord.lavaplayer.track.playback.LocalAudioTrackExecutor.execute(LocalAudioTrackExecutor.java:117)discord_music_bot_1  |  at com.sedmelluq.discord.lavaplayer.player.DefaultAudioPlayerManager.lambda$executeTrack$1(DefaultAudioPlayerManager.java:349)
 at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
 at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
 at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.lang.NullPointerException: null
 at java.base/java.net.URI$Parser.parse(URI.java:3104)
 at java.base/java.net.URI.<init>(URI.java:600)
 at com.sedmelluq.discord.lavaplayer.source.youtube.YoutubeTrackFormat.getUrl(YoutubeTrackFormat.java:66)
 at com.sedmelluq.discord.lavaplayer.source.youtube.YoutubeSignatureCipherManager.resolveFormatUrl(YoutubeSignatureCipherManager.java:97)
 at com.sedmelluq.discord.lavaplayer.source.youtube.YoutubeAudioTrack.loadBestFormatWithUrl(YoutubeAudioTrack.java:87)
 at com.sedmelluq.discord.lavaplayer.source.youtube.YoutubeAudioTrack.process(YoutubeAudioTrack.java:42)
 at com.sedmelluq.discord.lavaplayer.track.playback.LocalAudioTrackExecutor.execute(LocalAudioTrackExecutor.java:105)discord_music_bot_1  |  ... 4 common frames omitted

whew-inc · 2020-06-28T12:16:43Z

@realmar My fork wasn't up to date with the latest changes. It should work now.

As for the PR, I think it can be considered abandoned, unless @jagrosh approves it or someone else picks it up (feel free to do so!). I have no interest in the project anymore since I've quit Discord.

realmar · 2020-06-29T14:21:46Z

@whew-inc Fantastic, it works! Thanks a lot, I'm very happy with your work :D

Khaled-20 · 2020-12-04T15:45:55Z

Is this still open or has it been implemented yet? I would love to see this feature and would be happy to help with it as needed.

jagrosh · 2020-12-04T18:02:01Z

Is this still open or has it been implemented yet?

The PR is still open because it isn't finished yet, as per this comment

This is a feature I have kept on the backburner because just adding this command is not quite enough. The following are all related to seeking and should all be added at the same time:
* [x]  seeking to a specific point in a song (what this command currently does)

* [x]  seeking forward/backward a certain amount of time in a song

* [ ]  starting a song at a specific time (both via the `play` command as well as in playlists; youtube has a format for this and jmusicbot should probably adapt that same format)
Additionally, I want to keep time-related notation consistent throughout the bot.

MichailiK · 2021-04-12T07:36:18Z

EDIT: Okay I figured something out, for anyone else who stumbles onto this...

Seems too hacky for me.

As for the PR, I think it can be considered abandoned, unless jagrosh approves it or someone else picks it up (feel free to do so!). I have no interest in the project anymore since I've quit Discord.

Alright, I'm picking it back up in #674.

Sanduhr32 · 2021-04-12T08:06:27Z

!eval // ```js
var jump = {
  relative: true,
  hours: 0,
  minutes: 0,
  seconds: 0,
  milliseconds: 0
};
// Seek code
var track=guild.getAudioManager().getSendingHandler().getPlayer().getPlayingTrack();function jumpToMs(e){return 36e5*e.hours+6e4*e.minutes+1e3*e.seconds+e.milliseconds}function jumpToStr(e,t){var r=jumpToMs(t),a=t.relative?r<0?"-":"+":"→";return msToString(e)+" "+a+" "+msToString(r)}function pad(e,t,r){return r=r||"0",(e+="").length>=t?e:new Array(t-e.length+1).join(r)+e}function msToString(e){e=Math.abs(e);var t=Math.floor(e/36e5);e-=36e5*t;var r=Math.floor(e/6e4);e-=6e4*r;var a=Math.floor(e/1e3);e-=1e3*a;var n="";return t>0&&(n+=pad(t,2)+":"),r>0&&(n+=pad(r,2)+":"),n+pad(a,2)+"."+pad(e,3)}var status="Can't seek";if(track.isSeekable()){pos=track.getPosition();var newPos=36e5*jump.hours+6e4*jump.minutes+1e3*jump.seconds+jump.milliseconds;jump.relative&&(newPos+=pos),track.setPosition(Math.max(0,newPos)),status=jumpToStr(pos,jump)}
//```

Maybe don't inline everything? Also a native implementation is better than using eval for this.

Also Java isn't JS. It would have been better if you picked the PR up instead of designing a roundabout for a straight path.

MichailiK · 2021-04-12T08:28:24Z

but what if the scripting engine used for eval is js? :o

ribbanya · 2021-04-12T11:18:37Z

Maybe don't inline everything? Also a native implementation is better than using eval for this.

Also Java isn't JS. It would have been better if you picked the PR up instead of designing a roundabout for a straight path.

I'm not a contributor, just a user. I listen to long-form audio so I needed a way to seek without restarting the whole track. I only posted the script on the off chance someone would have a similar use case while the pull request is ongoing.

antaxiom · 2021-09-16T13:34:44Z

Hello, I noticed the last there hasn't been any comments on this issue since April, any updates?

MichailiK · 2021-09-16T13:36:24Z

I will work on this feature in #674 in the future.

MichailiK · 2022-07-13T08:25:09Z

Since my PR #674 is (mostly) finished, I'll close this in favor of it.

* Add seek command * Combine nested if statements * Put the seek command in correct place to keep alphabetical order * Add license header * Brackets on next line * Restructure if-statements * Check for permissions with DJCommand#checkDJPermission * Make regex slightly smaller * Optimize imports * Output length of current track if requested seek time is invalid * Restate seeked point when seeked successfully * Add empty newline at end of file to keep consistency * Create TimeUtil class for parsing and formatting time * Move FormatUtil#formatTime to TimeUtil, and refactor * Apply requested changes (Pass 2) * Seek based on current position in track * Apply requested changes (Pass 3) * Add javadoc param * Apply requested changes (Pass 4) * Fix merge * Avoid reassigning parameter (Codacy) * Rework timestamp parsing * Refactor timestamp parsing * Apply requested changes (Pass 5) * Add examples in help * Apply requested changes (Pass 6) * Fix missing import * Keep track of start timestamp & add "unit" times * Fix my abdominal merge with QueuedTrack * Use RequestMetadata to store start timestamp * Store request info in request metadata * Add regex to try getting a timestamp from the url * Require RequestMetadata for QueuedTracks * Add some unit tests for unit seeking * Add docs & examples --------- Co-authored-by: Whew., Inc <22574706+Whew-Inc@users.noreply.github.com>

commit 8733899 Author: John Grosh <john.a.grosh@gmail.com> Date: Wed Jul 10 17:35:50 2024 -0400 Update bug-report.yml commit e5fd05e Author: Moritz Bender <35152647+Morilli@users.noreply.github.com> Date: Wed Jul 10 13:19:13 2024 +0200 Update dependencies to fix youtube issues (jagrosh#1609) commit 6a5a9c7 Author: John Grosh <john.a.grosh@gmail.com> Date: Fri May 10 17:43:49 2024 -0400 Change playlist page count (jagrosh#1542) * change playlist page count * make the value configurable * small refactor commit 8557f7a Author: Michaili K <git@michaili.dev> Date: Fri May 10 21:36:43 2024 +0200 Log track exceptions in the audio handler (jagrosh#1558) commit 48e62f1 Author: Michaili K <git@michaili.dev> Date: Fri May 10 21:32:30 2024 +0200 Revert "Always self-deafen (jagrosh#1491)" (jagrosh#1551) This reverts commit a7807b9. commit 6f12c33 Author: John Grosh <john.a.grosh@gmail.com> Date: Fri May 10 15:23:30 2024 -0400 custom eval engine (jagrosh#1530) commit 0afb3db Author: Michaili K <git@michaili.dev> Date: Fri May 10 20:19:11 2024 +0200 Switch to lavalink's new YouTube audio source manager (jagrosh#1552) * Switch to new lavalink's new YouTube audio source manager * Add back setPlaylistPageCount call for the youtube audio source manager * Manually add all the individual audio sources EXCEPT for old youtube * Remove unused import * Upgrade lavaplayer-youtube-source to 1.0.3 * Upgrade lavaplayer-youtube-source to 1.0.4 commit 81322ef Author: Michaili K <git@michaili.dev> Date: Fri May 10 20:14:28 2024 +0200 Add seek command (supersedes jagrosh#344) (jagrosh#674) * Add seek command * Combine nested if statements * Put the seek command in correct place to keep alphabetical order * Add license header * Brackets on next line * Restructure if-statements * Check for permissions with DJCommand#checkDJPermission * Make regex slightly smaller * Optimize imports * Output length of current track if requested seek time is invalid * Restate seeked point when seeked successfully * Add empty newline at end of file to keep consistency * Create TimeUtil class for parsing and formatting time * Move FormatUtil#formatTime to TimeUtil, and refactor * Apply requested changes (Pass 2) * Seek based on current position in track * Apply requested changes (Pass 3) * Add javadoc param * Apply requested changes (Pass 4) * Fix merge * Avoid reassigning parameter (Codacy) * Rework timestamp parsing * Refactor timestamp parsing * Apply requested changes (Pass 5) * Add examples in help * Apply requested changes (Pass 6) * Fix missing import * Keep track of start timestamp & add "unit" times * Fix my abdominal merge with QueuedTrack * Use RequestMetadata to store start timestamp * Store request info in request metadata * Add regex to try getting a timestamp from the url * Require RequestMetadata for QueuedTracks * Add some unit tests for unit seeking * Add docs & examples --------- Co-authored-by: Whew., Inc <22574706+Whew-Inc@users.noreply.github.com>

* Add seek command * Combine nested if statements * Put the seek command in correct place to keep alphabetical order * Add license header * Brackets on next line * Restructure if-statements * Check for permissions with DJCommand#checkDJPermission * Make regex slightly smaller * Optimize imports * Output length of current track if requested seek time is invalid * Restate seeked point when seeked successfully * Add empty newline at end of file to keep consistency * Create TimeUtil class for parsing and formatting time * Move FormatUtil#formatTime to TimeUtil, and refactor * Apply requested changes (Pass 2) * Seek based on current position in track * Apply requested changes (Pass 3) * Add javadoc param * Apply requested changes (Pass 4) * Fix merge * Avoid reassigning parameter (Codacy) * Rework timestamp parsing * Refactor timestamp parsing * Apply requested changes (Pass 5) * Add examples in help * Apply requested changes (Pass 6) * Fix missing import * Keep track of start timestamp & add "unit" times * Fix my abdominal merge with QueuedTrack * Use RequestMetadata to store start timestamp * Store request info in request metadata * Add regex to try getting a timestamp from the url * Require RequestMetadata for QueuedTracks * Add some unit tests for unit seeking * Add docs & examples --------- Co-authored-by: Whew., Inc <22574706+Whew-Inc@users.noreply.github.com>

whew-inc added 2 commits January 22, 2020 04:27

Add seek command

cc336c7

Combine nested if statements

abd2c22

jagrosh requested changes Jan 22, 2020

View reviewed changes

whew-inc added 12 commits January 22, 2020 18:09

Put the seek command in correct place to keep alphabetical order

6fe00cd

Add license header

2bd70b8

Brackets on next line

f423b24

Restructure if-statements

7aecc0c

Check for permissions with DJCommand#checkDJPermission

270f5bc

Make regex slightly smaller

7fc52f2

Optimize imports

0c83119

Output length of current track if requested seek time is invalid

1e5df75

Restate seeked point when seeked successfully

06dc985

Add empty newline at end of file to keep consistency

3ce601f

Create TimeUtil class for parsing and formatting time

65792cc

Move FormatUtil#formatTime to TimeUtil, and refactor

65f2310

whew-inc requested a review from jagrosh January 22, 2020 18:54

jagrosh requested changes Jan 22, 2020

View reviewed changes

Apply requested changes (Pass 2)

10bc4f8

Seek based on current position in track

0ef1940

jagrosh requested changes Jan 27, 2020

View reviewed changes

jagrosh mentioned this pull request Feb 4, 2020

[Feature Request] Ability to seek in a track #348

Closed

whew-inc added 2 commits February 5, 2020 23:54

Apply requested changes (Pass 3)

20f2894

Add javadoc param

c6801ef

jagrosh requested changes Feb 6, 2020

View reviewed changes

whew-inc added 2 commits February 7, 2020 19:34

Apply requested changes (Pass 4)

f02416d

Merge branch 'master' into master

e922f8d

Add examples in help

b5673c5

jagrosh requested changes Mar 2, 2020

View reviewed changes

whew-inc added 2 commits April 23, 2020 21:22

Merge branch 'master' of https://github.com/jagrosh/MusicBot

0536e58

Merge with upstream

Apply requested changes (Pass 6)

605ae04

Merge branch 'master' of https://github.com/jagrosh/MusicBot

9bdaf36

jagrosh force-pushed the master branch from e9a7cde to 403a435 Compare October 27, 2020 20:45

This comment has been minimized.

Sign in to view

MichailiK mentioned this pull request Apr 12, 2021

Add seek command (supersedes #344) #674

Merged

6 tasks

MichailiK linked an issue Sep 16, 2021 that may be closed by this pull request

[Feature Request] Add the Ability to Skip to a Certain Time in a Song #469

Closed

MichailiK mentioned this pull request Sep 17, 2021

[Feature Request] Add the Ability to Skip to a Certain Time in a Song #469

Closed

MichailiK closed this Jul 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add seek command #344

Add seek command #344

whew-inc commented Jan 22, 2020 •

edited

Loading

jagrosh left a comment •

edited

Loading

whew-inc commented Jan 22, 2020 •

edited

Loading

whew-inc commented Jan 22, 2020

jagrosh commented Jan 22, 2020

jagrosh left a comment

whew-inc commented Jan 24, 2020

whew-inc commented Mar 2, 2020 •

edited

Loading

jagrosh left a comment

jagrosh Mar 2, 2020

jagrosh Mar 2, 2020

jagrosh Mar 2, 2020

jagrosh Mar 2, 2020

Timardo commented Jun 17, 2020

realmar commented Jun 26, 2020 •

edited

Loading

whew-inc commented Jun 28, 2020

realmar commented Jun 29, 2020

Khaled-20 commented Dec 4, 2020

jagrosh commented Dec 4, 2020

This comment has been minimized.

MichailiK commented Apr 12, 2021 •

edited

Loading

Sanduhr32 commented Apr 12, 2021

MichailiK commented Apr 12, 2021 •

edited

Loading

ribbanya commented Apr 12, 2021 •

edited

Loading

antaxiom commented Sep 16, 2021

MichailiK commented Sep 16, 2021

MichailiK commented Jul 13, 2022

		this.help = "seeks the current song" +
		"\nExamples: `1:02:23` `+1:10` `-90`";

Add seek command #344

Add seek command #344

Conversation

whew-inc commented Jan 22, 2020 • edited Loading

This pull request...

Description

Purpose

Relevant Issue(s)

jagrosh left a comment • edited Loading

Choose a reason for hiding this comment

whew-inc commented Jan 22, 2020 • edited Loading

whew-inc commented Jan 22, 2020

jagrosh commented Jan 22, 2020

jagrosh left a comment

Choose a reason for hiding this comment

whew-inc commented Jan 24, 2020

whew-inc commented Mar 2, 2020 • edited Loading

jagrosh left a comment

Choose a reason for hiding this comment

jagrosh Mar 2, 2020

Choose a reason for hiding this comment

jagrosh Mar 2, 2020

Choose a reason for hiding this comment

jagrosh Mar 2, 2020

Choose a reason for hiding this comment

jagrosh Mar 2, 2020

Choose a reason for hiding this comment

Timardo commented Jun 17, 2020

realmar commented Jun 26, 2020 • edited Loading

whew-inc commented Jun 28, 2020

realmar commented Jun 29, 2020

Khaled-20 commented Dec 4, 2020

jagrosh commented Dec 4, 2020

This comment has been minimized.

MichailiK commented Apr 12, 2021 • edited Loading

Sanduhr32 commented Apr 12, 2021

MichailiK commented Apr 12, 2021 • edited Loading

ribbanya commented Apr 12, 2021 • edited Loading

antaxiom commented Sep 16, 2021

MichailiK commented Sep 16, 2021

MichailiK commented Jul 13, 2022

whew-inc commented Jan 22, 2020 •

edited

Loading

jagrosh left a comment •

edited

Loading

whew-inc commented Jan 22, 2020 •

edited

Loading

whew-inc commented Mar 2, 2020 •

edited

Loading

realmar commented Jun 26, 2020 •

edited

Loading

MichailiK commented Apr 12, 2021 •

edited

Loading

MichailiK commented Apr 12, 2021 •

edited

Loading

ribbanya commented Apr 12, 2021 •

edited

Loading