Skip to content

Own fader to first position #1809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
softins merged 1 commit into from
Nov 26, 2021
Merged

Own fader to first position #1809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/audiomixerboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,9 +1070,15 @@ void CAudioMixerBoard::ChangeFaderOrder ( const EChSortType eChSortType )
// create a pair list of lower strings and fader ID for each channel
QList<QPair<QString, int>> PairList;
int iNumVisibleFaders = 0;
int iMyFader = -1;

for ( int i = 0; i < MAX_NUM_CHANNELS; i++ )
Copy link
Copy Markdown
Member

@ann0see ann0see Jul 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator

@pljones pljones Jul 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
if ( vecpChanFader[i]->GetIsMyOwnFader() )
{
iMyFader = i;
}

if ( eChSortType == ST_BY_NAME )
{
PairList << QPair<QString, int> ( vecpChanFader[i]->GetReceivedName().toLower(), i );
Expand Down Expand Up @@ -1117,6 +1123,19 @@ void CAudioMixerBoard::ChangeFaderOrder ( const EChSortType eChSortType )
// sort the channels according to the first of the pair
std::stable_sort ( PairList.begin(), PairList.end() );

// move my fader to first position
if ( pSettings->bOwnFaderFirst )
{
for ( int i = 0; i < MAX_NUM_CHANNELS; i++ )
{
if ( iMyFader == PairList[i].second )
{
PairList.move ( i, 0 );
break;
}
}
Comment thread
ann0see marked this conversation as resolved.
}

// we want to distribute iNumVisibleFaders across the first row, then the next, etc
// up to iNumMixerPanelRows. So row wants to start at 0 until we get to some number,
// then increase, where "some number" means we get no more than iNumMixerPanelRows.
Expand Down
1 change: 1 addition & 0 deletions src/audiomixerboard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class CChannelFader : public QObject
int GetRunningNewClientCnt() { return iRunningNewClientCnt; }
void SetChannelLevel ( const uint16_t iLevel );
void SetIsMyOwnFader() { bIsMyOwnFader = true; }
bool GetIsMyOwnFader() { return bIsMyOwnFader; }
void UpdateSoloState ( const bool bNewOtherSoloState );

protected:
Expand Down
9 changes: 9 additions & 0 deletions src/clientdlg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,12 @@ CClientDlg::CClientDlg ( CClient* pNCliP,
// View menu --------------------------------------------------------------
QMenu* pViewMenu = new QMenu ( tr ( "&View" ), this );

// own fader first option: works from server version 3.5.5 which supports sending client ID back to client
Copy link
Copy Markdown
Contributor Author

@ngocdh ngocdh Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

QAction* OwnFaderFirstAction =
pViewMenu->addAction ( tr ( "O&wn Fader First" ), this, SLOT ( OnOwnFaderFirst() ), QKeySequence ( Qt::CTRL + Qt::Key_W ) );
Copy link
Copy Markdown
Member

@ann0see ann0see Nov 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Copy Markdown
Member

@gilgongo gilgongo Nov 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@jp8 jp8 Nov 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@ngocdh ngocdh Nov 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with every point you made here.


pViewMenu->addSeparator();

QAction* NoSortAction =
pViewMenu->addAction ( tr ( "N&o User Sorting" ), this, SLOT ( OnNoSortChannels() ), QKeySequence ( Qt::CTRL + Qt::Key_O ) );

Expand All @@ -325,6 +331,9 @@ CClientDlg::CClientDlg ( CClient* pNCliP,
QAction* ByCityAction =
pViewMenu->addAction ( tr ( "Sort Users by &City" ), this, SLOT ( OnSortChannelsByCity() ), QKeySequence ( Qt::CTRL + Qt::Key_T ) );

OwnFaderFirstAction->setCheckable ( true );
OwnFaderFirstAction->setChecked ( pSettings->bOwnFaderFirst );

// the sorting menu entries shall be checkable and exclusive
QActionGroup* SortActionGroup = new QActionGroup ( this );
SortActionGroup->setExclusive ( true );
Expand Down
5 changes: 5 additions & 0 deletions src/clientdlg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ public slots:
void OnOpenAdvancedSettings();
void OnOpenChatDialog() { ShowChatWindow(); }
void OnOpenAnalyzerConsole() { ShowAnalyzerConsole(); }
void OnOwnFaderFirst()
{
pSettings->bOwnFaderFirst = !pSettings->bOwnFaderFirst;
MainMixerBoard->SetFaderSorting ( pSettings->eChannelSortType );
}
void OnNoSortChannels() { MainMixerBoard->SetFaderSorting ( ST_NO_SORT ); }
void OnSortChannelsByName() { MainMixerBoard->SetFaderSorting ( ST_BY_NAME ); }
void OnSortChannelsByInstrument() { MainMixerBoard->SetFaderSorting ( ST_BY_INSTRUMENT ); }
Expand Down
9 changes: 9 additions & 0 deletions src/settings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,12 @@ void CClientSettings::ReadSettingsFromXML ( const QDomDocument& IniXMLDocument,
eChannelSortType = static_cast<EChSortType> ( iValue );
}

// own fader first sorting
if ( GetFlagIniSet ( IniXMLDocument, "client", "ownfaderfirst", bValue ) )
{
bOwnFaderFirst = bValue;
}

// number of mixer panel rows
if ( GetNumericIniSet ( IniXMLDocument, "client", "numrowsmixpan", 1, 8, iValue ) )
{
Expand Down Expand Up @@ -624,6 +630,9 @@ void CClientSettings::WriteSettingsToXML ( QDomDocument& IniXMLDocument )
// fader channel sorting
SetNumericIniSet ( IniXMLDocument, "client", "channelsort", static_cast<int> ( eChannelSortType ) );

// own fader first sorting
SetFlagIniSet ( IniXMLDocument, "client", "ownfaderfirst", bOwnFaderFirst );

// number of mixer panel rows
SetNumericIniSet ( IniXMLDocument, "client", "numrowsmixpan", iNumMixerPanelRows );

Expand Down
2 changes: 2 additions & 0 deletions src/settings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ class CClientSettings : public CSettings
bWindowWasShownSettings ( false ),
bWindowWasShownChat ( false ),
bWindowWasShownConnect ( false ),
bOwnFaderFirst ( false ),
Comment thread
ann0see marked this conversation as resolved.
pClient ( pNCliP )
{
SetFileName ( sNFiName, DEFAULT_INI_FILE_NAME );
Expand Down Expand Up @@ -167,6 +168,7 @@ class CClientSettings : public CSettings
bool bWindowWasShownSettings;
bool bWindowWasShownChat;
bool bWindowWasShownConnect;
bool bOwnFaderFirst;

protected:
// No CommandLineOptions used when reading Client inifile
Expand Down