Skip to content

Audio changes - #5593

Merged
aikaterna merged 8 commits into
Cog-Creators:V3/developfrom
Drapersniper:audio_llset
Mar 28, 2022
Merged

Audio changes#5593
aikaterna merged 8 commits into
Cog-Creators:V3/developfrom
Drapersniper:audio_llset

Conversation

@Drapersniper

@Drapersniper Drapersniper commented Feb 26, 2022

Copy link
Copy Markdown
Contributor

1 - Allow configuringJava's -Xmx flag (supersedes #5542)
2 - Replace the application.yml in favor of config storage.
3 - Allow toggling sources on and off
4 - Allow changing the managed node binding IP, password, and port.
5 - Allow customizing the framebuffer and NAS buffer
6 - Framework to modify the application YAML is baked in, all it takes to add more stuff (plugins for example) if following the structure currently present.
7 - Added captcha for bot owners for specific llset commands
8 - Reduce the number of config calls in cog_before_invoke by 4
9 - Unify all defaults into a single location to make changing them easier in the future
10 - Add llset reset which resets all changes made by llset
11 - Update docstring for all llset subcommands
12 - llset host|password|port are now only usable if you first enable an external Lavalink server.
13 - llset info update
14 - llset yaml added
15 - audioset info updated
16 - -Xms64M always set.
17 - llset commands will not be available depending on Node mode (i.e llset config will not be available if you run an external mode, llset password will not be available if you run a managed node)
18 - Too many fixes to mention all of them around the shutdown and reconnection logic
19 - llset secured added

Depends on:

Cog-Creators/Red-Lavalink#118
Cog-Creators/Red-Lavalink#117
Cog-Creators/Red-Lavalink#116
Cog-Creators/Red-Lavalink#115
Cog-Creators/Red-Lavalink#114

To Test use: Cog-Creators/Red-Lavalink#113

Before merging there are TODO's and dependencies changes required in the PR which are in place now just to facilitate testing

@github-actions github-actions Bot added the Category: Cogs - Audio This is related to the Audio cog. label Feb 26, 2022
@aikaterna

Copy link
Copy Markdown
Member

Leaving a reminder to set some memory bounds and that 64mb is alright for a minimum.

@Drapersniper
Drapersniper marked this pull request as ready for review February 26, 2022 18:14
@Flame442 Flame442 linked an issue Feb 26, 2022 that may be closed by this pull request
@aikaterna aikaterna self-assigned this Feb 28, 2022
@aikaterna aikaterna added this to the 3.4.17 milestone Feb 28, 2022
@aikaterna

Copy link
Copy Markdown
Member

Keys with dashes in them (yaml__lavalink__server__gc-warnings, yaml__logging__file__max-history, and yaml__logging__file__max-size) can't be registered in Config as - can't be used.

[10:09:36] ERROR [red] Package loading failed
Traceback (most recent call last)
/.../nyaenv/lib/python3.9/site-packages/redbot/core/core_commands.py:174 in _load 
❱ 174 await bot.load_extension(spec) 
/.../nyaenv/lib/python3.9/site-packages/redbot/core/bot.py:1533 in load_extension 
❱ 1533 lib.setup(self) 
/.../nyaenv/lib/python3.9/site-packages/redbot/cogs/audio/__init__.py:7 in setup 
❱ 7 cog = Audio(bot) 
/.../nyaenv/lib/python3.9/site-packages/redbot/cogs/audio/core/__init__.py:171 in __init__ 
❱ 171 self.config.register_global(**default_global) 
/.../nyaenv/lib/python3.9/site-packages/redbot/core/config.py:859 in register_global 
❱ 859 self._register_default(self.GLOBAL, **kwargs) 
/.../nyaenv/lib/python3.9/site-packages/redbot/core/config.py:815 in _register_default 
❱ 815 to_add = self._get_defaults_dict(k, v) 
/.../nyaenv/lib/python3.9/site-packages/redbot/core/config.py:777 in _get_defaults_dict 
❱ 777 raise RuntimeError("'{}' is an invalid config key.".format(k)) 
RuntimeError: 'gc-warnings' is an invalid config key.

@aikaterna

Copy link
Copy Markdown
Member

Also if you're interested, something that could fit in this PR is the non existent llset settings needed for Cog-Creators/Red-Lavalink#107 (but there's also RLL work to do there, too, to support them).

@aikaterna

Copy link
Copy Markdown
Member

I had a bot with an external lavalink server set, then loaded this PR/cog after editing the config values for the application.yml creation, just to test...
"llset external" did nothing
I deleted the settings and restarted the bot so it would be set on internal
The jar was not downloaded
"llset external" doesn't do anything here either
Used "llset yaml" - the bot DMed me the yaml in an embed and then also said "playlist is too large, here is the compressed version" and uploaded the file... besides it sending it 2x, it's not a playlist 😉
Using "llset config server bind" has no user feedback on command use, with or without a host ip/name present, actually seems like most "llset" commands aren't functioning, may be related to the external/internal toggle and what you were saying about denying some command use on external nodes...
There are some phrasing changes I'll suggest at some point as well, but just wanted to get the ball rolling on what I found so far...

Comment thread redbot/cogs/audio/core/cog_utils.py Outdated
Comment thread redbot/cogs/audio/core/cog_utils.py Outdated
Comment thread redbot/cogs/audio/core/cog_utils.py Outdated
Comment thread redbot/cogs/audio/core/cog_utils.py Outdated
Comment thread redbot/cogs/audio/core/cog_utils.py Outdated
Comment thread redbot/cogs/audio/core/cog_utils.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
Drapersniper added a commit to Drapersniper/Red-DiscordBot that referenced this pull request Mar 18, 2022
Drapersniper added a commit to Drapersniper/Red-DiscordBot that referenced this pull request Mar 18, 2022
Drapersniper added a commit to Drapersniper/Red-DiscordBot that referenced this pull request Mar 18, 2022
@Drapersniper Drapersniper reopened this Mar 20, 2022
Drapersniper added a commit to Drapersniper/Red-DiscordBot that referenced this pull request Mar 22, 2022
Drapersniper added a commit to Drapersniper/Red-DiscordBot that referenced this pull request Mar 22, 2022
@Drapersniper
Drapersniper force-pushed the audio_llset branch 4 times, most recently from 0cd556d to 2b1eef6 Compare March 22, 2022 12:26
Comment thread redbot/cogs/audio/utils.py Outdated
Comment thread redbot/cogs/audio/utils.py Outdated
Comment thread redbot/cogs/audio/core/events/dpy.py Outdated
Comment thread redbot/cogs/audio/core/commands/audioset.py Outdated
Comment thread redbot/cogs/audio/core/commands/llset.py Outdated
…iendly names

- Change Xms to "Initial Heapsize"
- Change Xmx to "Max Heapsize"

Signed-off-by: Draper <27962761+Drapersniper@users.noreply.github.com>
@aikaterna
aikaterna merged commit 9ec85d4 into Cog-Creators:V3/develop Mar 28, 2022
@red-githubbot red-githubbot Bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Mar 28, 2022
@Drapersniper
Drapersniper deleted the audio_llset branch March 28, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Category: Cogs - Audio This is related to the Audio cog. Category: Meta This is related to the repository maintenance. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Audio] Discourage users from using [p]llset to fix unrelated issues

4 participants