jamulussoftware / jamulus

Jamulus enables musicians to perform real-time jam sessions over the internet.
https://jamulus.io
Other
1k stars 223 forks source link

Add a gui button and entry in ini file to enable/disable basic IPv6 support (client and server) #1973

Open ann0see opened 3 years ago

ann0see commented 3 years ago

Has this feature been discussed and generally agreed?

Kind of. See headline

Describe the solution you'd like The user should be able to enable/disable the newly introduced IPv6 support by @softins via GUI in server and client.

Describe alternatives that have been considered Only use the cli arguments. That's not very GUI user friendly

Note: this issue has been opened for r 3.8.1

jp8 commented 3 years ago

The IPv6 pull request is #1938.

I plan to propose a re-arrangement of the settings dialog, splitting the controls into the following 3 tabs:

I could add the button during that effort, but I wasn't planning to look at it until after 3.8.1

ann0see commented 3 years ago

I assume that's up to others (= separate issue). Please keep https://github.com/jamulussoftware/jamulus/discussions/1343#discussioncomment-529307 in mind!

jp8 commented 3 years ago

Good point - I'll definitely make a strong justification for any UI changes I propose.... and then see what others think.

For this button, I would say leaving it as a command line option in 3.8.1 would give a nice chance for technical users to test the functionality thoroughly, before the more 'casual' users see it in the UI in 3.8.2

ann0see commented 3 years ago

Ok. So un-tagging this for the next release.

gilgongo commented 2 years ago

Minor point, but rather than a button, I would think this would be a checkbox to toggle IPv6 on or off (so [ ] IPv6 basically)

pljones commented 2 years ago

Looking at the existing start up code (I've started on a rewrite of how settings are handled), it would currently need a restart of the application to get this working, as the socket is created and bound once when the Client or Server start up.

You wouldn't want to do it in the middle of a jam, either. Doing the network renegotiation would break things up.

So even if it were in the GUI, it wouldn't have any noticeable effect until the application was restarted. That could be confusing for people.

(Currently, each "connection" (a Channel) gets passed the same Socket instance to use. The Server must have the Socket without any connections, in order to work at all, of course.)

gilgongo commented 2 years ago

@pljones I think the convention there is usually to have some text next to the checkbox control. So:

[ ] IPv6 (requires restart)

And leave the user to take the action as they see fit.

pljones commented 2 years ago

OK, my rewrite of the start up will help, anyway. Currently, reading Settings requires the Client or Server to have been created, which means the Socket has been created and it's too late to use whatever was in the Settings. I'm hoping to get all the defaults, settings and command line processing done before creating the Client or Server...

ann0see commented 2 years ago

Un-Tagged since I don't think the redesign is finished for the upcoming release.

pgScorpio commented 2 years ago

@ann0see

I have some rework on commandline options and settings handling in progress that would allow you to store most commandline parameters in the ini file. I think that that would also solve this problem.

In this case the general idea would be to run Jamulus with a --store parameter once. i.e.: Jamulus --inifile <pathtomyini> --port 50001 --enableipv6 --clientname Guitar --store

In which case Jamulus will immediately return with:

Jamulus: Commandline options stored:
--port 50001 --enableipv6 --clientname Guitar

Now each time you just run Jamulus with: Jamulus --inifile <pathtomyini>

Jamulus will use the stored commandline parameters as if they where given on the commandline.

To clear the stored parameters again just run Jamulus with --store and no other parameters: Jamulus --inifile <pathtomyini> --store

Which will return with: Jamulus: Stored commandline options cleared.

P.S.: Only valid parameters can be stored. (Jamulus will exit with an error on invalid parameters.) The only parameters that won't be stored (long or short names of coarse) are: --help --version (On which Jamulus will exit immediately with the info anyway.) --inifile (Since this one is needed to know where to store/recall.) --server --nogui (Since these define the startup mode of Jamulus and so also the valid parameters.) --store (obvious.)

I will open a issue or PR for this as soon as my other PR's on messages, commandline options and exception handling are merged, since this new settings and commandline implementation depends on all of them.

ann0see commented 2 years ago

Yeah, there has been a lot of discussion about the ini file writing feature. Volker disliked headless servers having a config file while I think pljones was supporting them. I think it's a step in the right direction.

pljones commented 2 years ago

Moving as notabug.

pljones commented 1 year ago

Dropping milestone and back to Triage until someone picks the work up.