Skip to content

Refactor server chat handling by extracting methods #3741

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

Open
dingodoppelt wants to merge 4 commits into
base: main
Choose a base branch
from
+26 −2
Open

Refactor server chat handling by extracting methods #3741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
562d6c6
Refactor server chat handling by extracting methods. Added range chec…
dingodoppelt Jun 17, 2026
16b1ade
Remove overhead
dingodoppelt Jun 18, 2026
4f9a532
Clarify comment
dingodoppelt Jun 18, 2026
1e1f54a
fix style
dingodoppelt Jun 18, 2026
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
18 changes: 16 additions & 2 deletions src/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1297,17 +1297,31 @@ void CServer::CreateAndSendChatTextForAllConChannels ( const int iCurChanID, con
const QString strActualMessageText = "<font color=\"" + sCurColor + "\">(" + QTime::currentTime().toString ( "hh:mm:ss AP" ) + ") <b>" +
ChanName.toHtmlEscaped() + "</b></font> " + strChatText.toHtmlEscaped();

// Send chat text to all connected clients ---------------------------------
SendChatTextToAllConChannels ( strActualMessageText );
}

void CServer::SendChatTextToAllConChannels ( const QString& strChatText )
{
// Send chat text to all connected clients ---------------------------------
for ( int i = 0; i < iMaxNumChannels; i++ )
{
if ( vecChannels[i].IsConnected() )
{
// send message
vecChannels[i].CreateChatTextMes ( strActualMessageText );
vecChannels[i].CreateChatTextMes ( strChatText );
}
}
}

void CServer::SendChatTextToConChannel ( const int iCurChanID, const QString& strChatText )
{
if ( MathUtils::InRange<int> ( iCurChanID, 0, iMaxNumChannels - 1 ) && vecChannels[iCurChanID].IsConnected() )
{
// send message
vecChannels[iCurChanID].CreateChatTextMes ( strChatText );
}
}

void CServer::CreateAndSendRecorderStateForAllConChannels()
{
// get recorder state
Expand Down
3 changes: 3 additions & 0 deletions src/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ class CServer : public QObject, public CServerSlots<MAX_NUM_CHANNELS>
void SetEnableDelayPanning ( bool bDelayPanningOn ) { bDelayPan = bDelayPanningOn; }
bool IsDelayPanningEnabled() { return bDelayPan; }

void SendChatTextToAllConChannels ( const QString& strChatText );
void SendChatTextToConChannel ( const int iCurChanID, const QString& strChatText );

protected:
// access functions for actual channels
bool IsConnected ( const int iChanNum ) { return vecChannels[iChanNum].IsConnected(); }
Expand Down
7 changes: 7 additions & 0 deletions src/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,13 @@ class MathUtils
return powf ( 10.0f, ( fInValueRange0_1 - 1.0f ) * AUD_MIX_FADER_RANGE_DB / 20.0f );
}
}

// return true if a value is within the lower and upper bounds (inclusively), otherwise false
template<typename T>
Comment thread
dingodoppelt marked this conversation as resolved.
static inline bool InRange ( T value, T lower, T upper )

@pljones pljones Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The Java/C# programmer in me wants this as an extension to Comparable<T>, so that

(<Comparable<Int>>32).inRange(0, 100)

returns true. Don't know if C++ can do better than what you've got here, though. (What you should do is constraint T somehow, really.)

@dingodoppelt dingodoppelt Jun 18, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Don't know if C++ can do better than what you've got here, though. (What you should do is constraint T somehow, really.)

This function cannot be called from the outside so I think we should be safe here.
The first placed I searched for range validating functions was the Jamulus source code, then Qt docs because I thought there must be something there. Then I wrote this :) If we don't need it and it might cause issues I'm happy to remove it and do the sanity checks locally.

{
return !( value < lower || value > upper );
}
};

/******************************************************************************\
Expand Down
Loading