Own fader to first position #1809

ngocdh · 2021-06-02T09:28:40Z

This pull request set own fader always to 1st position, regardless of the sorting setting.
See discussion #1646

In case we want to have the choice to set or unset the option, then UI changes must be involved.

ann0see · 2021-06-02T10:17:00Z

Thanks for this. I think it should be possible to enable this in the UI to not confuse existing users. Tagging @gilgongo and @mulyaj here to voice their opinion on where to put it. I‘m still unsure where to put it since I fear that the new tabbed settings UI opens up cluttering the settings dialog with too many features.

ngocdh · 2021-06-02T10:20:53Z

Thanks for this. I think it should be possible to enable this in the UI to not confuse existing users. Tagging @gilgongo and @mulyaj here to voice their opinion on where to put it. I‘m still unsure where to put it since I fear that the new tabbed settings UI opens up cluttering the settings dialog with too many features.

A check box in the Edit menu, close to Sorting options

dcorson-ticino-com · 2021-06-02T10:45:57Z

Thanks @ngocdh, this was on my "to be done soon" list.
I agree that enabling/disabling is most sensibly in the edit menu with the other sorting choices.

ngocdh · 2021-06-02T14:29:01Z

Thanks @ngocdh, this was on my "to be done soon" list.

Cool thanks @dcorson-ticino-com. ~~When you get time, could you implement the menu?~~ It seems CClientSettings (to store/load the option) and CClientDlg (the Menu item and action) are involved.

ngocdh · 2021-06-10T15:25:30Z

So I got around to implement that Menu action :)

Note though: the feature will not work if connected to old servers, just like any other feature relying on OnClientIDReceived, because it seems old servers don't send client id back to each client?

softins · 2021-06-10T15:28:26Z

Note though: the feature will not work if connected to old servers, just like any other feature relying on OnClientIDReceived, because it seems old servers don't send client id back to each client?

If that's the case, could you determine which version server is the first to support that, and note it in a code comment and commit message? And mention it here too. Thanks!

ngocdh · 2021-06-10T15:35:35Z

Found: 3.5.5.
Ref: #148 (comment)

Also I put code comment in clientdlg.cpp, next to OwnFaderFirstAction definition.

ngocdh · 2021-06-10T15:45:14Z

src/clientdlg.cpp

    // Edit menu  --------------------------------------------------------------
    QMenu* pEditMenu = new QMenu ( tr ( "&Edit" ), this );

+    // own fader first option: works from server version 3.5.5 which supports sending client ID back to client


Feature doesn't work if connected to server prior to version 3.5.5.

pljones · 2021-06-19T08:40:17Z

Just want a status update on this. Is anything outstanding? (I don't appear to have complained about anything, so I guess I better read the code :) ).

pljones · 2021-09-15T08:08:42Z

This will need some refactoring following @jp8's menu changes for 3.8.1.

jp8 · 2021-09-15T08:32:30Z

Easy enough to move this setting to the View menu instead of the Edit menu.

But there's still a lot to think about, especially around the multi-row, sort-by-group views.

pljones · 2021-09-15T09:16:06Z

But there's still a lot to think about, especially around the multi-row, sort-by-group views.

For an initial implementation, a "first means first" by whatever sort order is in use approach should be taken. Keep it simple is part of the Jamulus creed.

Find "me"
Remove "me" from the list
Sort the list
Insert me first
Display the list

pljones · 2021-11-13T16:42:46Z

@ngocdh are you still able to pick this up?

The client creates the channel for itself as the very first channel. It is always channel zero. So finding it is "easy". Then you just apply the sort to the later channels in the list.

I had a thought on the GUI.

Following the menu reorganisation, I'd really like to get the check boxes for "Settings" and "Chat" removed, so that they're controlled through the menus (with shortcuts available).

That would give space for a check box for "Keep me first in the mixer" or "Mixer 1st" as it needs to be very short.

gilgongo · 2021-11-13T17:06:03Z

Is "Keep me first in the mixer" that important to be given such prominence? Seem a little odd given other settings we have.

jp8 · 2021-11-13T17:14:43Z

My view was this should be the default when"sort by arrival time" is selected. And "sort by arrival time" still needs a better name.

But not when "sort by instrument" or "sort by name", etc. are selected. That would be allowing the users to select two conflicting settings.

And no need for an individual checkbox for it.

pljones · 2021-11-13T17:19:00Z

My view was this should be the default when"sort by arrival time" is selected. And "sort by arrival time" still needs a better name.

But not when "sort by instrument" or "sort by name", etc. are selected. That would be allowing the users to select two conflicting settings.

And no need for an individual checkbox for it.

First, I'm never a fan of changing existing behaviour -- people have set expectations and shouldn't actively have to adjust a setting to restore to that behaviour.

I can't see why it shouldn't be a choice in all sort modes. It's fairly clear: put me first, always, then sort everyone else per whatever the sort rule is. So it's not exactly conflicting.

And there would need to be a way for the user to choose to have the behaviour or not. Maybe it should be a checkable option on the view menu. (Maybe I'd prefer that and clear even more check boxes off the mixer...)

gilgongo · 2021-11-13T17:21:28Z

@jp8 Yes, that seems sensible.

As to the wording, I would think that most people would expect the default to be by arrival/joining time left to right, so we could just call it "default" and explain in the docs for the curious?

gilgongo · 2021-11-13T17:28:42Z

put me first, always, then sort everyone else per whatever the sort rule is. So it's not exactly conflicting.

One for the settings panel I think, not the main screen though.

jp8 · 2021-11-13T17:29:10Z

First, I'm never a fan of changing existing behaviour -- people have set expectations and shouldn't actively have to adjust a setting to restore to that behaviour.

You are certainly right. But just to be clear, I wasn't suggesting they would be able to adjust a setting to restore the old way. My suggestion would be to modify the 'Default" sort order so that the user's own channel is at the left (with no setting anywhere for the user to turn that off), and leave all the other sort orders unchanged.

My reasoning is that people who use the "Default" sort order just want to have the faders remain in the same place, and not jump around when musicians join. Putting own fader at the left doesn't (according to my theory) inconvenience them.

jp8 · 2021-11-13T17:32:14Z

put me first, always, then sort everyone else per whatever the sort rule is. So it's not exactly conflicting.

One for the settings panel I think, not the main screen though.

The reason i disagree with this is because if the view menu says "sort by name", and the user selects that, but the channels are not sorted by name.... it's a non-intuitive user interface.

pljones · 2021-11-13T17:34:12Z

put me first, always, then sort everyone else per whatever the sort rule is. So it's not exactly conflicting.

One for the settings panel I think, not the main screen though.

The reason i disagree with this is because if the view menu says "sort by name", and the user selects that, but the channels are not sorted by name.... it's a non-intuitive user interface.

Not if there's a check item on the same menu saying "Own channel first", surely?

And the same would apply to any sort mode -- so you're essentially saying "I don't want this feature"?

jp8 · 2021-11-13T17:38:52Z

Not if there's a check item on the same menu saying "Own channel first", surely?

Right - if the setting exists I think it needs to be a checkbox on the View menu, right below the selectable views (like you first suggested).

But I am not convinced that users of the 'Default' sort order need an option to have their own fader somewhere other than the left position.

And I am not convinced that users who have sorted by name, instrument, etc, need an option to make an exception for their own fader. In my view, it should always go to the correct position for the chosen sort order.

jp8 · 2021-11-13T17:55:32Z

so you're essentially saying "I don't want this feature"?

No I didn't mean to come across as not wanting the feature. But I would like the Jamulus interface to be clear and consistent for the users, so I am trying to think carefully about it.

pljones · 2021-11-13T18:17:00Z

I just don't like arbitary decisions made on behalf of the user.

jp8 · 2021-11-13T18:28:21Z

Yeah I just want to think carefully, get a sensible default, and only add choices in the UI if they are really necessary.

The original requirement was "own fader should be easy to find".... maybe we should have discussed highlighting it visually but leaving it in position.

ghost · 2021-11-13T21:49:26Z

If there are several sort criteria, they should be prioritised by the user
in the user settings.
eg. sort A third, sort B first, sort C none, sort D Nth sort E none

No sort check box should be on the main menu.
eg. the DIR command in MSDOS can be configured perfectly in
autoexec.bat or config.sys to perfect the directory listing.
(it appears modern operating systems have lost this ability)

ngocdh · 2021-11-14T21:25:09Z

Team, this feature is actually simple: if user wants their fader first on the mixer, then check the option, regardless of other sorting options.
I don't think there should be a checkbox on the mixer for this though.

softins · 2021-11-15T13:00:48Z

Following the menu reorganisation, I'd really like to get the check boxes for "Settings" and "Chat" removed, so that they're controlled through the menus (with shortcuts available).

Not sure I like this idea. I don't think I have ever opened the Settings and Chat windows via the menu bar, always with the buttons in the mixer.

pljones · 2021-11-15T17:16:57Z

Following the menu reorganisation, I'd really like to get the check boxes for "Settings" and "Chat" removed, so that they're controlled through the menus (with shortcuts available).

Not sure I like this idea. I don't think I have ever opened the Settings and Chat windows via the menu bar, always with the buttons in the mixer.

That's because before the menu reorganisation, finding the menu items was hard and now you're used to it the way it was easiest.

getismyownfader move own fader to 1st position Own fader first with menu action

softins · 2021-11-20T18:17:16Z

I like this option the way it is, and have just been testing it. The PR was branched from 3.8.0rc2, and I have just squashed and rebased it to current master. @ngocdh is it ok if I push the rebased version to your branch?

ngocdh · 2021-11-20T18:43:45Z

Sure Tony

softins

Rebased on master and tested.

pljones

Looks okay.

ann0see · 2021-11-24T21:09:59Z

src/clientdlg.cpp


+    // own fader first option: works from server version 3.5.5 which supports sending client ID back to client
+    QAction* OwnFaderFirstAction =
+        pViewMenu->addAction ( tr ( "O&wn Fader First" ), this, SLOT ( OnOwnFaderFirst() ), QKeySequence ( Qt::CTRL + Qt::Key_W ) );


@gilgongo Not sure if we could improve the wording here?
Own fader first is not 100% percise in my opinion.
Own fader at first position?

Not sure - "Show own fader first" perhaps? "Put your fader first"? (I don't think the initial capitals are in style BTW) At least, I don't think "first" is ambiguous so that it needs "position" as well: "They came first in the race" vs "They came in first position in the race". "To put your own interests first" vs "To put your own interests in first position". "Position" feels redundant.

Talking of being 100% precise...

I am still of the opinion that having the user's own fader at the left is good in the default (unsorted) sort, but I don't see any reason for the user to have the option to turn it on and off.

If I play violin, and I sort by instrument, I would expect my fader to be right there with the other violins.

If I am in London and I sort by city, I would expect my fader to be right there with the other London people.

I also haven't seen any convincing argument of why a user would need to both:

sort by instrument, and also

have their own fader appear at the left instead of with the others of the same instrument

Users don't have that option now. But is there somebody that really wants to do exactly that ? And if so, is that need strong enough to justify an extra setting ? Especially one that (in my view, at least) conflicts with the wording of the existing sort options ...?

I personally would really struggle to come up with 100% precise accurate wording for all the sort options that would no longer be strictly correct.

I agree with every point you made here.

src/settings.h

src/audiomixerboard.cpp

ann0see · 2022-07-24T10:52:39Z

src/audiomixerboard.cpp

    int                        iNumVisibleFaders = 0;
+    int                        iMyFader          = -1;

    for ( int i = 0; i < MAX_NUM_CHANNELS; i++ )


@ngocdh
There might be issues with the code here (see #2738) can you please look at this again?

(And note, again, I'd prefer if all this "state" information was held by the client - the UI should be responsible for no more than displaying that information as held.)

ngocdh commented Jun 10, 2021

View reviewed changes

hoffie added this to the Release 3.9.0 milestone Jun 13, 2021

Update audiomixerboard.h

3e23035

getismyownfader move own fader to 1st position Own fader first with menu action

softins force-pushed the ownfader1st branch from 92d92c8 to 3e23035 Compare November 20, 2021 20:46

softins approved these changes Nov 20, 2021

View reviewed changes

softins requested a review from pljones November 20, 2021 20:48

pljones approved these changes Nov 20, 2021

View reviewed changes

ann0see reviewed Nov 24, 2021

View reviewed changes

softins merged commit c633a54 into jamulussoftware:master Nov 26, 2021

gilgongo modified the milestones: Release 3.9.0, 3.8.2 Jan 17, 2022

hoffie mentioned this pull request Jul 18, 2022

Improve algorithm to find in use channels in O(n) instead O(n²) #2738

Merged

7 tasks

ann0see reviewed Jul 24, 2022

View reviewed changes

Own fader to first position #1809

Own fader to first position #1809

Uh oh!

Conversation

ngocdh commented Jun 2, 2021

Uh oh!

ann0see commented Jun 2, 2021

Uh oh!

ngocdh commented Jun 2, 2021

Uh oh!

dcorson-ticino-com commented Jun 2, 2021

Uh oh!

ngocdh commented Jun 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ngocdh commented Jun 10, 2021

Uh oh!

softins commented Jun 10, 2021

Uh oh!

ngocdh commented Jun 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ngocdh Jun 10, 2021

Choose a reason for hiding this comment

Uh oh!

pljones commented Jun 19, 2021

Uh oh!

pljones commented Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jp8 commented Sep 15, 2021

Uh oh!

pljones commented Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pljones commented Nov 13, 2021

Uh oh!

gilgongo commented Nov 13, 2021

Uh oh!

jp8 commented Nov 13, 2021

Uh oh!

pljones commented Nov 13, 2021

Uh oh!

gilgongo commented Nov 13, 2021

Uh oh!

gilgongo commented Nov 13, 2021

Uh oh!

jp8 commented Nov 13, 2021

Uh oh!

jp8 commented Nov 13, 2021

Uh oh!

pljones commented Nov 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jp8 commented Nov 13, 2021

Uh oh!

jp8 commented Nov 13, 2021

Uh oh!

pljones commented Nov 13, 2021

Uh oh!

jp8 commented Nov 13, 2021

Uh oh!

ghost commented Nov 13, 2021

Uh oh!

ngocdh commented Nov 14, 2021

Uh oh!

softins commented Nov 15, 2021

Uh oh!

pljones commented Nov 15, 2021

Uh oh!

softins commented Nov 20, 2021

Uh oh!

ngocdh commented Nov 20, 2021

Uh oh!

softins left a comment

Choose a reason for hiding this comment

Uh oh!

pljones left a comment

Choose a reason for hiding this comment

Uh oh!

ngocdh commented Jun 2, 2021 •

edited

Loading

ngocdh commented Jun 10, 2021 •

edited

Loading

pljones commented Sep 15, 2021 •

edited

Loading

pljones commented Sep 15, 2021 •

edited

Loading

pljones commented Nov 13, 2021 •

edited

Loading

gilgongo Nov 27, 2021 •

edited

Loading