Add Vibe Mode #42

aadibajpai · 2020-07-26T02:19:01Z

Implements an automatic loop so you do not need to request lyrics again and again if the song changes, until you stop listening to music or use the $kill command.

This is similar to the way it works in the cli swaglyrics application.

pep8speaks · 2020-07-26T02:19:04Z

Hello @aadibajpai! Thanks for updating this PR.

In the file SwagLyricsBot/general_commands.py:

Line 95:66: W291 trailing whitespace

Comment last updated at 2020-08-20 01:32:12 UTC

codecov · 2020-07-26T02:20:09Z

Codecov Report

Merging #42 into master will decrease coverage by 1.59%.
The diff coverage is 14.51%.

@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
- Coverage   28.70%   27.11%   -1.60%     
==========================================
  Files           9        9              
  Lines         310      354      +44     
==========================================
+ Hits           89       96       +7     
- Misses        221      258      +37

Impacted Files	Coverage Δ
SwagLyricsBot/lyrics.py	`21.87% <0.00%> (ø)`
SwagLyricsBot/general_commands.py	`35.32% <14.75%> (-6.95%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9232d96...b39c037. Read the comment docs.

flabbet

Well, I know you said it's not going to work with more users, but this approach is a no-go. Using self statement makes it almost impossible to make it work for more users. Also, I don't like the idea of calling it vibe mode, Especially in code, what if someone will work on this except us? We would need to explain to him what it is, code should be self-explanatory, this is an unnecessary complication. This is all right to add this as command alias, but not as default one.

SwagLyricsBot/general_commands.py

aadibajpai · 2020-07-29T22:24:07Z

Well, I know you said it's not going to work with more users, but this approach is a no-go. Using self statement makes it almost impossible to make it work for more users.

fixed

aadibajpai · 2020-07-29T22:26:27Z

@thealphadollar, this adds a looping feature to the bot, where you once call the commands and it will keep sending lyrics as activity changes until you stop listening or kill the loop with $kill. The logic is that the $vibe command adds your info to a shared variable and starts a loop if the loop isn't already running. And then the loop every 5s takes care of all the users in the shared variable and $kill removes your info from the shared variable.

There's a bunch of debug print statements there but I'll get rid of them.

aadibajpai · 2020-07-29T22:26:58Z

@dolphingarlic @fadedcoder review again, thanks!

looks like there's a problem with the logging part, it doesn't integrate well with this. Once commented it works as intended.

addressed

SwagLyricsBot/general_commands.py

dolphingarlic · 2020-07-30T15:08:03Z

SwagLyricsBot/general_commands.py

+
+    @commands.command()
+    async def vibe(self, ctx):
+        prev = self.current.copy()


Instead of copying the entire dict, you could just check whether the new dict has length equal to 1

Also, check whether the user is already in self.current if you do this

Also, check whether the user is already in self.current if you do this

what do you mean by this

look at the updated commit and see if that's ok?

SwagLyricsBot/general_commands.py

dolphingarlic · 2020-07-30T15:09:44Z

SwagLyricsBot/general_commands.py

+                await self.send_lyrics(channel, song, artists)
+            except LyricsError as e:
+                print('No activity detected, stopping loop.')
+                await channel.send("No activity detected, killing the vibe.")


Suggested change

await channel.send("No activity detected, killing the vibe.")

await channel.send("No activity detected. Killing the vibe.")

it's not that bad is it?

dolphingarlic · 2020-07-30T15:10:30Z

SwagLyricsBot/general_commands.py

+            self.vibe_mode.start()
+
+    @commands.command()
+    async def kill(self, ctx):


Maybe rename this to kill_vibe or something to prevent ambiguity

ambiguity with what?

SwagLyricsBot/lyrics.py

flabbet · 2020-08-06T17:53:23Z

Make logs work (you can't call add_sub_log without first calling add_log) + clean up console output, and make sure it works independently for every user, also I think this option should be only called on direct messages

flabbet

Make changes based on the comment above

we don't need to store channel now since DMs only

aadibajpai · 2020-08-09T08:35:05Z

Make logs work (you can't call add_sub_log without first calling add_log) + clean up console output, and make sure it works independently for every user, also I think this option should be only called on direct messages

check now, also how much console output would you like to keep so it's okay?

flabbet

Well, it's too much same information for one loop, get rid of it, it should print maybe ony a user name + song name.
And get rid of the slang. Use casual language, but not slang. It's not consistent for the rest of the bot and I will not merge it until you change it. I let you leave $vibe as a command string, but provide aliases like $loop, and describe it in $help

Oh and fix the conflict

aadibajpai · 2020-08-20T01:32:54Z

Well, it's too much same information for one loop, get rid of it, it should print maybe ony a user name + song name.
And get rid of the slang. Use casual language, but not slang. It's not consistent for the rest of the bot and I will not merge it until you change it. I let you leave $vibe as a command string, but provide aliases like $loop, and describe it in $help

Oh and fix the conflict

the Spotify thing happens through the logging set up in the preexisting function, so I'm not modifying that. Rest everything has been addressed.

flabbet · 2020-08-23T14:01:49Z

Fine, It can stay then

flabbet

Add the user nickname in this line https://github.com/aadibajpai/SwagLyrics-discord-bot/blob/vibe/SwagLyricsBot/general_commands.py#L49. You should add the log to webhook that someone started vibe and ended it, so we can investigate it easier. Also include the number of people currently in vibe.

implement $vibe tasks loop and commands

ffe1a28

aadibajpai requested a review from flabbet July 26, 2020 02:19

aadibajpai marked this pull request as draft July 26, 2020 02:19

PEP8 fixes

72e7b3b

flabbet previously requested changes Jul 28, 2020

View reviewed changes

SwagLyricsBot/general_commands.py Outdated Show resolved Hide resolved

Aadi Bajpai added 2 commits July 30, 2020 03:45

rework logic acc to discord.py feedback

31c83a1

fix conflict

1d36eba

aadibajpai marked this pull request as ready for review July 29, 2020 22:24

aadibajpai requested a review from flabbet July 30, 2020 08:52

works perfectly

3fa7bee

looks like there's a problem with the logging part, it doesn't integrate well with this. Once commented it works as intended.

dolphingarlic suggested changes Jul 30, 2020

View reviewed changes

flabbet requested changes Aug 6, 2020

View reviewed changes

fix logging + address reviews

0536263

we don't need to store channel now since DMs only

flabbet requested changes Aug 11, 2020

View reviewed changes

Aadi Bajpai added 2 commits August 20, 2020 06:50

Merge branch 'master' into vibe

32827c3

simplify logging, finalize PR

b39c037

flabbet requested changes Aug 23, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Vibe Mode #42

Add Vibe Mode #42

aadibajpai commented Jul 26, 2020

pep8speaks commented Jul 26, 2020 •

edited

Loading

codecov bot commented Jul 26, 2020 •

edited

Loading

flabbet left a comment

aadibajpai commented Jul 29, 2020

aadibajpai commented Jul 29, 2020

aadibajpai commented Jul 29, 2020

dolphingarlic Jul 30, 2020

dolphingarlic Jul 30, 2020

aadibajpai Aug 9, 2020

aadibajpai Aug 9, 2020

aadibajpai Aug 9, 2020

dolphingarlic Jul 30, 2020

aadibajpai Aug 9, 2020

dolphingarlic Jul 30, 2020

aadibajpai Aug 9, 2020

flabbet commented Aug 6, 2020

flabbet left a comment

aadibajpai commented Aug 9, 2020 •

edited

Loading

flabbet left a comment

aadibajpai commented Aug 20, 2020

flabbet commented Aug 23, 2020

flabbet left a comment

	await channel.send("No activity detected, killing the vibe.")
	await channel.send("No activity detected. Killing the vibe.")

Add Vibe Mode #42

Are you sure you want to change the base?

Add Vibe Mode #42

Conversation

aadibajpai commented Jul 26, 2020

pep8speaks commented Jul 26, 2020 • edited Loading

Comment last updated at 2020-08-20 01:32:12 UTC

codecov bot commented Jul 26, 2020 • edited Loading

Codecov Report

flabbet left a comment

Choose a reason for hiding this comment

aadibajpai commented Jul 29, 2020

aadibajpai commented Jul 29, 2020

aadibajpai commented Jul 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flabbet commented Aug 6, 2020

flabbet left a comment

Choose a reason for hiding this comment

aadibajpai commented Aug 9, 2020 • edited Loading

flabbet left a comment

Choose a reason for hiding this comment

aadibajpai commented Aug 20, 2020

flabbet commented Aug 23, 2020

flabbet left a comment

Choose a reason for hiding this comment

pep8speaks commented Jul 26, 2020 •

edited

Loading

codecov bot commented Jul 26, 2020 •

edited

Loading

aadibajpai commented Aug 9, 2020 •

edited

Loading