jamulus icon indicating copy to clipboard operation
jamulus copied to clipboard

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

Open hoffie opened this issue 2 years ago • 5 comments

(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.

hoffie avatar Feb 26 '22 23:02 hoffie

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

ann0see avatar Apr 23 '22 20:04 ann0see

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 avatar May 22 '22 17:05 pljones

Won't happen during 3.9.1. Moving to 3.10.0.

pljones avatar Aug 29 '22 16:08 pljones

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

pljones avatar Apr 19 '23 14:04 pljones