vrcx-team / VRCX

Friendship management tool for VRChat
MIT License
966 stars 183 forks source link

CEFsharp used only for aesthetics? #436

Closed JayNasty closed 1 year ago

JayNasty commented 1 year ago

Theres zero reason you should use an embedded browser for this project. I know Maui is new and scary but join the discord(https://discord.gg/UWaVCedHcf) and please don't couple carelessly like this. I understand we're all learning but giving actual advice and getting kicked from the discord is gatekeeping.

Atleast the prefabs discord, knah and slaynash have talent and can listen to others opinions. I already started my fork bc having a slick gui and api calls is..... nevermind. Consider modifying your few files using CEFsharp and converting to Maui instead of gatekeeping

Natsumi-sama commented 1 year ago

You were kicked because you are incapable of acting like an adult, I'm not planning on doing an entire rewrite to satisfy your needs, if you're not happy with the project fix it yourself no one is stopping you.

JayNasty commented 1 year ago

So if I decoupled CEFsharp and substituted Maui, let's see who the adult is if you push it. Cheers~

BenjaminZehowlt commented 1 year ago

Please be aware changing a core technology an application is based on is a massive undertaking, and the author has every right to deny said PR. This is especially true for software such as VRCX with many moving parts, as it would require retesting everything to make sure everything works as-is. If you wish to maintain your own fork of it using a different framework than CEF you're welcome to.

JayNasty commented 1 year ago

@BenjaminZehowlt

Please don't contribute nothing to this. 1. an embedded browser like CEF isnt core. 2. your profile, "i like causing access violations"? no hate but keep learning and check out vrcxs code, its not alot.

its basically an api with a cute lil gui and refresh buttons everywhere. With all due respect, please give actual informaion if you're confident.

BenjaminZehowlt commented 1 year ago

@JayNasty

Please be aware, I’ve privately contributed to VRCX’s development not publicly (as shown by commit history). Please double check your facts before calling out things like that thank you.

But as I’ve said, you’re welcome to maintain a fork of it with CEFSharp removed, if the work shows merit, it may be merged. It doesn’t guarantee it as I’ve stated before.

JayNasty commented 1 year ago

@BenjaminZehowlt Sure i'll ask instead of fact checking. Why would changing CEF have so many moving parts?? Is it because the people working on VRCX did it as a first project and tightly coupled the logic with the ui?

Not trying to be rude, ive donated and looked through the code myself. Natsumis pretty awesome and sucks I had my derp discord moments.

Myrkie commented 1 year ago

@BenjaminZehowlt Sure i'll ask instead of fact checking. Why would changing CEF have zo many moving parts?? Is it because the people working on VRCX did it as a first project and tightly coupled the logic with the ui?

Not trying to be rude, ive donated looked through the code myself. Natsumis pretty awesome and sucks I had my derp discord moments.

I take it that you haven't seen the 22k lines hell that is app.js good luck converting that easily without losing functionality or breaking features.

BenjaminZehowlt commented 1 year ago

Please note that app.js is currently over 22,000 lines of code.

Using the framework which you suggested would require rewriting that entirely from scratch. It’s no small task to recreate that much code. It’s also not a small task to retest every feature due to that.

Let’s not forget on top of this, we have VRChat’s API which is ban trigger happy. So there is also the chance of it banning anyone who’s working on rewriting that code due to sending something very wrong while testing.

JayNasty commented 1 year ago

use fiddler and VRCX make range of api requests ????????? profit?

also imagine getting banned on a free to play game xD POLICIAAA

Myrkie commented 1 year ago

Once again no one is stopping you, Natsumi is not entitled to fulfill your requests, you have a fork so do it yourself and maybe it will get merged.

BenjaminZehowlt commented 1 year ago

To add onto what Myrk said, what you suggested was writing bug free code the first time, that’s sadly not how the world of software development works, there will always be issues big or small.

Here’s a viable example of what can happen here. Let’s say we have valid request structures, there’s also the chance of other issues such as i.e. accidentally spamming requests. VRChat’s API doesn’t like this and that could set off a ban trigger.

JayNasty commented 1 year ago

@BenjaminZehowlt @Myrkie Never asked him to rewrite it. Speaking of debugging? I saw like zero unit tests lol

Myrkie commented 1 year ago

your asking for a conversion to Maui, which would require are large rewrite of VRCX to accomplish. again look at app.js. do it yourself and it may get merged, if its so easy that is.

DubyaDude commented 1 year ago

Everything else aside

A rewrite has been thrown around for some time. The project in its current state is in need of it pretty badly. I'm not arguing with you on that.

I live and breathe everything .NET, so seeing MAUI I thought it'd be a great solution as well. However, I was disappointed to see that there are no plans to support the Linux Desktop. Someone tried to create a fork to achieve it, but it mainly got abandoned.

This project currently DOES somewhat support Linux. It takes some tweaking, but in the end, it does work. That's something I'd like to carry on into the rewrite.

That's something we should aim for in the rewrite, however, NET MAUI is not the solution for it.