Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(plugin): BetterScreenshare, BetterMicrophone, PhilsPluginLibrary #730

Closed
wants to merge 67 commits into from

Conversation

philhk
Copy link
Contributor

@philhk philhk commented Mar 29, 2023

BetterScreenshare

Features

  • Profileable
  • Custom Framerate
  • Custom Resolution
  • Custom Keyframe Interval
  • Custom Video Bitrate
  • Custom Video Codec
  • Custom Audio Settings (identical to those in BetterMicrophone)
  • HDR
  • Hide default screenshare settings
  • Custom settings panel button
  • Simple mode which only offers (Custom Framerate, Custom Video Bitrate (with a list of options) and Custom Resolution (with a list of options))

image
image
image
image
image
image
image

BetterMicrophone

Features

  • Profileable
  • Custom Channels (Stereo)
  • Custom Audio Bitrate
  • Custom Sample Frequency
  • Custom Sample Rate
  • Custom Pac Size
  • Simple mode which only offers (Stereo toggle and Custom Audio Bitrate (with a list of options))

image
image
image

Both plugins are independent of each other so that means both are seperate plugins. If you'd enabled both the settings panel would look like this

image

PhilsPluginLibrary

This plugin makes it easy to add a button to the settings panel and also contains some utility functions and frequently used components.

@resucutie
Copy link
Contributor

may you say which features does it add on the description of this PR? it is good to do that so that you do not need to look to the source code to see which changes were made

@philhk
Copy link
Contributor Author

philhk commented Mar 30, 2023

Of course! I've added a README explaining how each feature works, but here's a list.

  • Custom Screenshare Audio Source no matter what you are streaming
  • Custom Screenshare Resolution
  • Custom Screenshare Framerate
  • Custom Screenshare Keyframe Interval
  • Custom Screenshare Video Bitrate
  • Custom Screenshare Audio Bitrate
  • Custom Screenshare Video Codec
  • Screenshare HDR Toggle
  • Screenshare Profiles (they store the above mentioned settings)
  • A toggle to hide the default screen sharing settings in the plugin options
  • Everything is toggleable so no default features are removed from discord

@resucutie
Copy link
Contributor

resucutie commented Mar 30, 2023

i meant to list them in the PR's description, not in a comment. it is easier to people to check it on the PR's description than in the readme or in a comment, again to avoid having to see the source code or the comments

@philhk
Copy link
Contributor Author

philhk commented Mar 30, 2023

Oh Okay I updated the description and added some pictures.

@resucutie
Copy link
Contributor

Oh Okay I updated the description and added some pictures.

thanks. i appreciate your effort

@144reasons
Copy link
Contributor

Honestly, since plugins are built in, there shouldn't be any need to add author-specific libraries. Each function should be made available inside the vencord API, if that makes sense.

@ArjixWasTaken
Copy link
Contributor

ArjixWasTaken commented Apr 17, 2023

Honestly, since plugins are built in, there shouldn't be any need to add author-specific libraries. Each function should be made available inside the vencord API, if that makes sense.

Unless said lib is huge enough to impact the bundle size.

(I haven't checked said lib, I'm just stating the possibility)

@Vap0r1ze
Copy link
Contributor

I have a feeling that this pr is actually waiting on better 3rd party plugin support. but in terms of built-in plugins you shouldn't add author specific libraries. If you're plugin is Doing the same thing as another plugin then it will probably be moved to a client api/util.

@TheKodeToad
Copy link
Contributor

Honestly, since plugins are built in, there shouldn't be any need to add author-specific libraries. Each function should be made available inside the vencord API, if that makes sense.

I feel the same way. Maybe the api should be descriptive of what it is instead of being called Phil (I like the name though)!

@ArjixWasTaken
Copy link
Contributor

Naming smth after the author is such a devilbro thing

@TheKodeToad
Copy link
Contributor

This isn't really on-topic any more I don't think but it's quite a common thing with modding. I've seen it in Minecraft too lol!

@philhk philhk closed this Apr 29, 2023
@exitss
Copy link
Contributor

exitss commented May 4, 2023

closed :(

@iwa
Copy link
Contributor

iwa commented May 11, 2023

couldn't this get you banned anyways? they might have some warnings about bandwidth usage, kind of the same as twitch does, so even if it's a good idea, i don't think it'd be worth the risk...

@philhk
Copy link
Contributor Author

philhk commented May 11, 2023

I've been using this plugin daily since creating this PR at the max bitrate of 10000kbps and nothing has happened. You decide whether to take the risk.

@zxsilenx
Copy link

Can't build your vencord fork:

Build failed with 3 errors: src/plugins/betterMicrophone.desktop/logger/index.ts:19:7: ERROR: No matching export in "src/utils/Logger.ts" for import "default" src/plugins/betterScreenshare.desktop/logger/index.ts:19:7: ERROR: No matching export in "src/utils/Logger.ts" for import "default" src/plugins/philsPluginLibrary/discordModules/components.tsx:19:9: ERROR: No matching export in "src/utils/misc.tsx" for import "LazyComponent" 2 warnings and 3 errors  ELIFECYCLE  Command failed with exit code 1.

@ArjixWasTaken
Copy link
Contributor

Can't build your vencord fork:

Build failed with 3 errors: src/plugins/betterMicrophone.desktop/logger/index.ts:19:7: ERROR: No matching export in "src/utils/Logger.ts" for import "default" src/plugins/betterScreenshare.desktop/logger/index.ts:19:7: ERROR: No matching export in "src/utils/Logger.ts" for import "default" src/plugins/philsPluginLibrary/discordModules/components.tsx:19:9: ERROR: No matching export in "src/utils/misc.tsx" for import "LazyComponent" 2 warnings and 3 errors  ELIFECYCLE  Command failed with exit code 1.

I can build it just fine, so you are doing smth wrong.

@ArjixWasTaken
Copy link
Contributor

ArjixWasTaken commented May 13, 2023

Dunno if this is already a known issue, but on web (and vencord desktop) this plugin makes it impossible to connect to a VC.
It is stuck at "RTC Connecting".

(Just noticed that the plugins are .desktop, so it makes sense why they don't work on web.)

@philhk
Copy link
Contributor Author

philhk commented May 13, 2023

That's weird, it probably has something to do with something else because I'm not having any issues at all. Have you tried restarting your PC?

@ArjixWasTaken
Copy link
Contributor

ArjixWasTaken commented May 13, 2023

did you try on normal discord?
or did you mean vencord desktop? vencord desktop is discord web on an electron wrapper

https://github.com/Vencord/Desktop

@zxsilenx
Copy link

zxsilenx commented May 15, 2023

it's fine, we solved this with dev (@philhk)

@Loukious
Copy link

Loukious commented Jun 9, 2023

Why was this closed btw?

@3941
Copy link

3941 commented Jun 14, 2023

No idea, but I'd REALLY like to see this plugin be a thing.
If anyone has some pointers towards how to use this manually, I'd be very happy.
Just modifying the video bitrate would be enough for me, actually.

@EyeDeck
Copy link

EyeDeck commented Jun 15, 2023

@3941
You can use philhk's fork directly, follow this: https://github.com/Vendicated/Vencord/blob/main/docs/1_INSTALLING.md
Installation is exactly the same as that guide, except you replace
git clone https://github.com/Vendicated/Vencord
with philhk's fork:
git clone https://github.com/philhk/Vencord

@3941
Copy link

3941 commented Jun 16, 2023

Thank you so much @EyeDeck !
I did run into an ExecutionPolicy issue (using powershell), but after setting it to unrestricted, temporarily, it all worked just fine!

@berksmbl
Copy link

I think the development was stopped before it could be released :(
Sadly, it was a promising project

@3941
Copy link

3941 commented Jun 23, 2023

It works fine as it is. I think the problem is rather the review process. Since Vencord has every plugin built in, and no way to add them manually, everything needs to be reviewed thoroughly before it gets added to the project. And this is a HUGE plugin(s) to review.

Not sure why it was closed. Maybe they didn't like something. Maybe they didn't want to bother reviewing it.
But it works. I'm using philhk's fork now. Ofc that means I won't get any other updates, but right now it does everything I need it to do and it runs very stable.

@berksmbl
Copy link

@3941 I don't want to break away from the official version.

@lewisakura
Copy link
Sponsor Collaborator

Not sure why it was closed. Maybe they didn't like something. Maybe they didn't want to bother reviewing it.

The author themselves closed the PR. I am unsure if this is due to discussions in Discord or whatnot.

From a contributor standpoint, this is a HUGE change and as aforementioned would be better suited for when the third-party plugin system comes out fully. The only reasonable way this would get reviewed would be if this was split into two separate PRs (one for the BetterScreenshare plugin, one for the BetterMicrophone plugin).

On top of this, however, there are a few issues with this PR:

  • BetterMicrophone violates Vencord's "no stereo mic" rule, though I suppose this is more of a thing to stop people from asking for it since we don't particularly want to support it.
  • Plugin libraries aren't really a thing in Vencord and the abilities this library has should be turned into a general API plugin or into part of Vencord core.
  • The way the plugins are written is highly unusual (using a class?) and will potentially break in the future since it is not using definePlugin. This is a relatively easy fix but I am unsure why they are written this way.
  • The plugins have direct references to the library plugin's files in its imports (see point two).
  • These plugins are absolutely massive compared to anything else in Vencord, which increases review time. There are roughly 100 different source code files we have to look through here compared to the 1-3 on average for new plugins (though from quick inspection a lot of these could be merged together).
  • There are concerns that it is too risky. Vencord is meant to have guarantees that plugins are safe to use and enabling all of them shouldn't lead to issues. However, both of these plugins give control to parameters that allow to produce results very far outside the bounds of the regular client, which may lead to your account being at risk in the future.
  • I am unsure if this will continue to be maintained.

Again, you can run the fork directly, but we will not give support for doing so. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet