search

jamulussoftware / jamulus

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

Code: Introduce pClient->setChannelName() and make GUI and JSON-RPC use it #2438

Open hoffie opened 2 years ago

hoffie commented 2 years ago

(Initially raised by @pljones and @ann0see in https://github.com/jamulussoftware/jamulus/pull/1975/files#r808416974)

Has this feature been discussed and generally agreed?

Yes.

Describe the solution you'd like

https://github.com/jamulussoftware/jamulus/blob/master/src/clientsettingsdlg.cpp#L1082-L1098 + clientrpc.cpp should be refactored to use a new pClient->setChannelName method.

Describe alternatives that have been considered

Live with the duplication.

ann0see commented 2 years ago

@Rob-NY @dtinth does anyone want to open a PR on this issue?

pljones commented 2 years ago

I'll get there eventually - CClient and CServer will take a tighter grip on a lot of things, with the GUI becoming thinner as a result. But I'm quite a way off that.

Basically, anything that causes a protocol message to be sent or handles the update from a protocol message being received should be in CClient or CServer, in my view. These two should then use connect/sockets and emit/signals to expose things - because they can bounce between threads, rather than getters/setters, which are in the same thread.

Similarly, anything that makes the client to behave differently should be wired using connect/sockets and emit/signals -- this allows for a broader publish/subscribe interaction, which is much harder with getters/setters. This means a change made over jsonrpc can be reflected in the GUI and vice versa.

pljones commented 2 years ago

Won't happen during 3.9.1. Moving to 3.10.0.

pljones commented 1 year ago

Removing milestone and moving to Triage as no one seems interested in picking it up.

ann0see commented 2 months ago

Tagging for 4.0.0