Skip to content

tweak(gamemessage): Reduce number of MSG_DESTROY_SELECTED_GROUP messages #2668

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
Caball009 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

tweak(gamemessage): Reduce number of MSG_DESTROY_SELECTED_GROUP messages #2668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6c8d336
tweak(msg): Reduce number of MSG_DESTROY_SELECTED_GROUP messages.
Caball009 Apr 30, 2026
db6a8e6
Removed function 'deselectAll'.
Caball009 Apr 30, 2026
4c917cd
Updated other callsites of `InGameUI::deselectAllDrawables` when used…
Caball009 Apr 30, 2026
4453191
Removed TSH comment.
Caball009 Apr 30, 2026
8c156f6
Added additional check for selected drawables.
Caball009 May 3, 2026
fcb6ef1
Added logic to avoid double deselection on left mouse click.
Caball009 May 3, 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
2 changes: 1 addition & 1 deletion GeneralsMD/Code/GameEngine/Include/GameClient/InGameUI.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ friend class Drawable; // for selection/deselection transactions
// Drawable selection mechanisms
virtual void selectDrawable( Drawable *draw ); ///< Mark given Drawable as "selected"
virtual void deselectDrawable( Drawable *draw ); ///< Clear "selected" status from Drawable
virtual void deselectAllDrawables( Bool postMsg = true ); ///< Clear the "select" flag from all drawables
virtual void deselectAllDrawables( Bool updateGameLogic = true ); ///< Clear the "select" flag from all drawables
virtual Int getSelectCount() { return m_selectCount; } ///< Get count of currently selected drawables
virtual Int getMaxSelectCount() { return m_maxSelectCount; } ///< Get the max number of selected drawables
virtual UnsignedInt getFrameSelectionChanged() { return m_frameSelectionChanged; } ///< Get the max number of selected drawables
Expand Down
1 change: 1 addition & 0 deletions GeneralsMD/Code/GameEngine/Include/GameClient/SelectionXlat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class SelectionTranslator : public GameMessageTranslator
friend Bool killThemKillThemAllWrapper( Drawable *draw, void *userData );
private:

Bool m_pendingDeselection;
Bool m_leftMouseButtonIsDown;
Bool m_dragSelecting;
UnsignedInt m_lastGroupSelTime;
Expand Down
2 changes: 1 addition & 1 deletion GeneralsMD/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ CBCommandStatus ControlBar::processCommandUI( GameWindow *control,
}

//deselect other units
TheInGameUI->deselectAllDrawables();
TheInGameUI->deselectAllDrawables(FALSE);

// create a new group.
GameMessage *teamMsg = TheMessageStream->appendMessage( GameMessage::MSG_CREATE_SELECTED_GROUP );
Expand Down
21 changes: 10 additions & 11 deletions GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3553,9 +3553,10 @@ void InGameUI::deselectDrawable( Drawable *draw )
//-------------------------------------------------------------------------------------------------
/** Clear all drawables' "select" status */
//-------------------------------------------------------------------------------------------------
void InGameUI::deselectAllDrawables( Bool postMsg )
void InGameUI::deselectAllDrawables( Bool updateGameLogic )
Comment thread
xezon marked this conversation as resolved.
{
const DrawableList *selected = getAllSelectedDrawables();
const Bool emptySelectedDrawables = selected->empty();
Copy link
Copy Markdown

@xezon xezon May 7, 2026

Choose a reason for hiding this comment

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

Maybe flip meaning to const Bool hasSelectedDrawables = !selected->empty();

Then it is a positive meaning


// loop through all the selected drawables
for ( DrawableListCIt it = selected->begin(); it != selected->end(); )
Expand All @@ -3576,16 +3577,14 @@ void InGameUI::deselectAllDrawables( Bool postMsg )
// our selection can no longer consist of exactly one angry mob
m_soloNexusSelectedDrawableID = INVALID_DRAWABLE_ID;


///@todo don't we want to not emit this message if there wasn't a group at all? (CBD)
/** @todo also, we probably are sending this message too much, we should come up with
some kind of "selections are dirty" status that we can check once per frame and send
the correct group info over the network ... could be tricky tho (or impossible) given
the order of operations of things happening in the code (CBD) */
if( postMsg )
if (updateGameLogic)
{
// TheSuperHackers @tweak Originally this message had one boolean argument, but it wasn't used for anything.
TheMessageStream->appendMessage( GameMessage::MSG_DESTROY_SELECTED_GROUP );
// TheSuperHackers @tweak Only send this message when objects are currently selected.
Copy link
Copy Markdown

@xezon xezon May 7, 2026

Choose a reason for hiding this comment

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

"were previously selected". They are no longer selected now.

if (!emptySelectedDrawables)
{
// TheSuperHackers @tweak Originally this message had one boolean argument, but it wasn't used for anything.
TheMessageStream->appendMessage(GameMessage::MSG_DESTROY_SELECTED_GROUP);
}
}
}

Expand Down Expand Up @@ -5888,7 +5887,7 @@ void InGameUI::selectNextIdleWorker()
if(selectThisObject)
{
DEBUG_ASSERTCRASH(selectThisObject->getContainedBy() == nullptr, ("InGameUI::selectNextIdleWorker Selected idle object should not be contained"));
deselectAllDrawables();
deselectAllDrawables(FALSE);
GameMessage *teamMsg = TheMessageStream->appendMessage( GameMessage::MSG_CREATE_SELECTED_GROUP );


Expand Down
10 changes: 5 additions & 5 deletions GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2585,7 +2585,7 @@ GameMessageDisposition CommandTranslator::translateGameMessage(const GameMessage
if(newDrawable != nullptr )
{
//deselect other units
TheInGameUI->deselectAllDrawables();
TheInGameUI->deselectAllDrawables(FALSE);

// create a new group.
GameMessage *teamMsg = TheMessageStream->appendMessage( GameMessage::MSG_CREATE_SELECTED_GROUP );
Expand Down Expand Up @@ -2701,7 +2701,7 @@ GameMessageDisposition CommandTranslator::translateGameMessage(const GameMessage
if(newDrawable != nullptr )
{
//deselect other units
TheInGameUI->deselectAllDrawables();
TheInGameUI->deselectAllDrawables(FALSE);
// select the unit

// create a new group.
Expand Down Expand Up @@ -2814,7 +2814,7 @@ GameMessageDisposition CommandTranslator::translateGameMessage(const GameMessage
if(newDrawable != nullptr )
{
//deselect other units
TheInGameUI->deselectAllDrawables();
TheInGameUI->deselectAllDrawables(FALSE);

// select the unit
// create a new group.
Expand Down Expand Up @@ -2929,7 +2929,7 @@ GameMessageDisposition CommandTranslator::translateGameMessage(const GameMessage
if(newDrawable != nullptr )
{
//deselect other units
TheInGameUI->deselectAllDrawables();
TheInGameUI->deselectAllDrawables(FALSE);
// select the unit

// create a new group.
Expand Down Expand Up @@ -2982,7 +2982,7 @@ GameMessageDisposition CommandTranslator::translateGameMessage(const GameMessage
if ( heroDraw == nullptr )
break;

TheInGameUI->deselectAllDrawables();
TheInGameUI->deselectAllDrawables(FALSE);

// create a new group.
GameMessage *teamMsg = TheMessageStream->appendMessage( GameMessage::MSG_CREATE_SELECTED_GROUP );
Expand Down
30 changes: 14 additions & 16 deletions GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,18 +216,6 @@ static Bool canSelectWrapper( Drawable *draw, void *userData )
return CanSelectDrawable( draw, dragSelecting );
}

//-----------------------------------------------------------------------------
/**
* Deselect all drawables, and emit a "TEAM_DESTROY" message, since
* the "team" was the group of currently selected units.
*/
static void deselectAll()
{

// deselect it all
TheInGameUI->deselectAllDrawables();
}

//-----------------------------------------------------------------------------
/**
* Select the given drawable, without playing its sound.
Expand All @@ -237,7 +225,7 @@ static Bool selectSingleDrawableWithoutSound( Drawable *draw )
{

// since we are single selecting a drawable, unselect everything else
deselectAll();
TheInGameUI->deselectAllDrawables();

// do the drawable selection
TheInGameUI->selectDrawable( draw );
Expand All @@ -261,6 +249,7 @@ SelectionTranslator *TheSelectionTranslator = nullptr;
//-----------------------------------------------------------------------------
SelectionTranslator::SelectionTranslator()
{
m_pendingDeselection = FALSE;
m_leftMouseButtonIsDown = FALSE;
m_dragSelecting = FALSE;
m_lastGroupSelTime = 0;
Expand Down Expand Up @@ -750,7 +739,8 @@ GameMessageDisposition SelectionTranslator::translateGameMessage(const GameMessa
{
if (!addToGroup)
{
deselectAll();
Comment thread
Caball009 marked this conversation as resolved.
m_pendingDeselection = FALSE;
TheInGameUI->deselectAllDrawables(FALSE);
}

GameMessage *newMsg = TheMessageStream->appendMessage(GameMessage::MSG_CREATE_SELECTED_GROUP);
Expand Down Expand Up @@ -951,7 +941,7 @@ GameMessageDisposition SelectionTranslator::translateGameMessage(const GameMessa
{
if( !TheInGameUI->getPreventLeftClickDeselectionInAlternateMouseModeForOneClick() )
{
deselectAll();
m_pendingDeselection = TRUE;
m_lastGroupSelGroup = -1;
}
else
Expand Down Expand Up @@ -1018,7 +1008,7 @@ GameMessageDisposition SelectionTranslator::translateGameMessage(const GameMessa
else if (!TheGlobalData->m_useAlternateMouse)
{
//No GUI command mode, so deselect everyone if we're in regular mouse mode.
deselectAll();
TheInGameUI->deselectAllDrawables();
}
}
}
Expand Down Expand Up @@ -1330,6 +1320,14 @@ GameMessageDisposition SelectionTranslator::translateGameMessage(const GameMessa
#endif
}

// TheSuperHackers @tweak Avoid double deselection when selecting a new object with another object selected,
// originally triggered by RAW_MOUSE_LEFT_BUTTON_UP and MOUSE_LEFT_CLICK, respectively.
if (msg->getType() == GameMessage::MSG_MOUSE_LEFT_CLICK && m_pendingDeselection)
Copy link
Copy Markdown

@xezon xezon May 7, 2026

Choose a reason for hiding this comment

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

Maybe it is not worth it to go through all this additional logic and new complexity just to save one small network message?

Copy link
Copy Markdown
Author

@Caball009 Caball009 May 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's worth it if there are no gameplay issues.

This small message is the most frequently used network message currently, probably by a large margin. At a quick glance it takes >20 bytes of data across the network for this particular message.

Copy link
Copy Markdown

@xezon xezon May 8, 2026

Choose a reason for hiding this comment

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

How much data is then saved across a full match?

Copy link
Copy Markdown

@xezon xezon May 8, 2026

Choose a reason for hiding this comment

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

I do not quite understand this condition.

Copy link
Copy Markdown
Author

@Caball009 Caball009 May 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

How much data is then saved across a full match?

So I checked the Golden Replay 1 and counted some messages.

  • MSG_NETWORK_MESSAGES: 58771

  • MSG_DESTROY_SELECTED_GROUP: 17366

  • MSG_CREATE_SELECTED_GROUP preceded by a single MSG_DESTROY_SELECTED_GROUP: 6289

  • MSG_CREATE_SELECTED_GROUP preceded by a double MSG_DESTROY_SELECTED_GROUP: 1925

For replays this message is currently 13 bytes (now that the unused argument is removed). With the number above it comes down 131807 bytes, so roughly 128KB. For perspective, the replay size is 1.23MB.

What's curious is that the game appears to write the frame for every message, and uses 4 bytes for the player index instead of 1.

Copy link
Copy Markdown

@xezon xezon May 8, 2026

Choose a reason for hiding this comment

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

Can this be moved into a switch case?

{
m_pendingDeselection = FALSE;
TheInGameUI->deselectAllDrawables();
}

return disp;
}

Expand Down
Loading