Skip to content

feat: Add chat prefixes to distinguish All and Observer messages #1764

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

feat: Add chat prefixes to distinguish All and Observer messages #1764

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
4f792db
feat: Add chat prefixes to distinguish All and Observer messages
Hamood-bot Oct 31, 2025
61a85a7
refactor: Use localization for team chat prefix and reverse logic
Hamood-bot Oct 31, 2025
5626ae0
refactor: Mirror GeneralsMD team-chat refactor into Generals Connecti…
Hamood-bot Oct 31, 2025
a8a5138
refactor(chat): use FETCH_OR_SUBSTITUTE_FORMAT localized chat formats…
Hamood-bot Oct 31, 2025
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
91 changes: 86 additions & 5 deletions Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,21 +619,102 @@ void ConnectionManager::processChat(NetChatCommandMsg *msg)
name = m_connections[playerID]->getUser()->GetName();
//DEBUG_LOG(("connection is non-NULL, using %ls", name.str()));
}
unitext.format(L"[%ls] %ls", name.str(), msg->getText().str());
// DEBUG_LOG(("ConnectionManager::processChat - got message from player %d (mask %8.8X), message is %ls", playerID, msg->getPlayerMask(), unitext.str()));

AsciiString playerName;
playerName.format("player%d", msg->getPlayerID());
const Player *player = ThePlayerList->findPlayerWithNameKey( TheNameKeyGenerator->nameToKey( playerName ) );
if (!player)
{
unitext.format(L"[%ls] %ls", name.str(), msg->getText().str());
TheInGameUI->message(UnicodeString(L"%ls"), unitext.str());
return;
}

Bool fromObserver = !player->isPlayerActive();
Bool amIObserver = !ThePlayerList->getLocalPlayer()->isPlayerActive();
Bool canSeeChat = (amIObserver || !fromObserver) && !TheGameInfo->getConstSlot(playerID)->isMuted();
// =============================================================
// ConnectionManager::processChat()
Copy link

@Skyaero42 Skyaero42 Nov 1, 2025

Choose a reason for hiding this comment

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

This feels a bit too much written by AI without a human checking the result.

Comment is incorrect, because it is not a refactor but a feature or enhancment.

// Refactored by TheSuperHackers @feature - 31/10/2025
// Simplified team chat detection + localizable format via FETCH_OR_SUBSTITUTE_FORMAT()
Copy link

@xezon xezon Oct 31, 2025

Choose a reason for hiding this comment

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

This block comment is too verbose and does not help reader.

// =============================================================

static Bool isTeamChat(const Player* sender, UInt32 mask)
Copy link

@Skyaero42 Skyaero42 Nov 1, 2025

Choose a reason for hiding this comment

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

Functions should not exist within other functions. Usr private functions in the class instead

{
Int allies = 0;
Int recipients = 0;

for (Int i = 0; i < MAX_SLOTS; ++i)
Copy link

@Skyaero42 Skyaero42 Nov 1, 2025

Choose a reason for hiding this comment

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

I dont think this is correct. You only need to verify the relationship between receiver and sender. Don't need to loop through all players for this.

Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

Hmm indeed maybe it is sufficient to test relationship between sender and local player?

{
if ((1 << i) & mask)
{
const Player* receiver = ThePlayerList->getConstSlot(i);
if (receiver && receiver->isPlayerActive())
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

I am not sure if the isPlayerActive condition is correct. What was the rationale for that? The Player Mask is created in ToggleInGameChat

{
recipients++;
if (sender->getRelationship(receiver->getDefaultTeam()) == ALLIES)
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

ToggleInGameChat also uses || player==localPlayer as condition. Is this necessary as well or redundant?

allies++;
}
}
}

// Team message if all recipients are allies and at least one exists
return (recipients > 0 && recipients == allies);
Copy link

@xezon xezon Oct 31, 2025

Choose a reason for hiding this comment

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

Remove ( )

}

void ConnectionManager::processChat(Int playerID, ChatMessage* msg)
{
const Player* player = ThePlayerList->getConstSlot(playerID);
if (!player) return;

const Player* localPlayer = ThePlayerList->getLocalPlayer();
Bool fromObserver = !player->isPlayerActive();

UnicodeString name(player->getDisplayName());
UnicodeString unitext;

// Determine whether this is a team message
Bool isTeamMessage = FALSE;
if (player->isPlayerActive())
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

Is this test necessary or would Bool isTeamMessage = isTeamChat(player, ...) also work?

{
isTeamMessage = isTeamChat(player, msg->getPlayerMask());
}

// Use localized formatted strings (via FETCH_OR_SUBSTITUTE_FORMAT)
if (isTeamMessage)
{
// In your .csf file, define:
// CHAT:TeamFormat = (%s) [%s] %s
UnicodeString teamPrefix = TheGameText->FETCH_OR_SUBSTITUTE("GUI:Team", L"TEAM");
Copy link

@xezon xezon Oct 31, 2025

Choose a reason for hiding this comment

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

"Team" to match loca string.

unitext = TheGameText->FETCH_OR_SUBSTITUTE_FORMAT(
"CHAT:TeamFormat",
L"(%ls) [%ls] %ls",
Copy link

@xezon xezon Oct 31, 2025

Choose a reason for hiding this comment

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

This does not look like it should be a loca string.

teamPrefix.str(),
name.str(),
msg->getText().str()
);
}
else
{
// In your .csf file, define:
// CHAT:GlobalFormat = [%s] %s
unitext = TheGameText->FETCH_OR_SUBSTITUTE_FORMAT(
"CHAT:GlobalFormat",
L"[%ls] %ls",
name.str(),
msg->getText().str()
);
}

// DEBUG_LOG(("ConnectionManager::processChat - got message from player %d (mask %8.8X), message is %ls",
// playerID, msg->getPlayerMask(), unitext.str()));

Bool amIObserver = !localPlayer->isPlayerActive();
Bool canSeeChat = (amIObserver || !fromObserver) && !TheGameInfo->getConstSlot(playerID)->isMuted();
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

Can move these Bools up to the other one so they are bundled together.


if (((1 << m_localSlot) & msg->getPlayerMask()) && canSeeChat)
{
TheInGameUI->message(UnicodeString(L"%ls"), unitext.str());
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

Just L"%ls" will do

}
}
;
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

Excess semicolon


if ( ((1<<m_localSlot) & msg->getPlayerMask() ) && canSeeChat )
{
Expand Down
90 changes: 85 additions & 5 deletions GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,21 +619,101 @@ void ConnectionManager::processChat(NetChatCommandMsg *msg)
name = m_connections[playerID]->getUser()->GetName();
//DEBUG_LOG(("connection is non-NULL, using %ls", name.str()));
}
unitext.format(L"[%ls] %ls", name.str(), msg->getText().str());
// DEBUG_LOG(("ConnectionManager::processChat - got message from player %d (mask %8.8X), message is %ls", playerID, msg->getPlayerMask(), unitext.str()));

AsciiString playerName;
playerName.format("player%d", msg->getPlayerID());
const Player *player = ThePlayerList->findPlayerWithNameKey( TheNameKeyGenerator->nameToKey( playerName ) );
if (!player)
{
unitext.format(L"[%ls] %ls", name.str(), msg->getText().str());
TheInGameUI->message(UnicodeString(L"%ls"), unitext.str());
return;
}

Bool fromObserver = !player->isPlayerActive();
Bool amIObserver = !ThePlayerList->getLocalPlayer()->isPlayerActive();
Bool canSeeChat = (amIObserver || !fromObserver) && !TheGameInfo->getConstSlot(playerID)->isMuted();
// =============================================================
// ConnectionManager::processChat()
// Refactored by TheSuperHackers @feature - 31/10/2025
// Simplified team chat detection + localizable format via FETCH_OR_SUBSTITUTE_FORMAT()
// =============================================================

static Bool isTeamChat(const Player* sender, UInt32 mask)
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

Can make it private function of ConnectionManager

{
Int allies = 0;
Int recipients = 0;

for (Int i = 0; i < MAX_SLOTS; ++i)
{
if ((1 << i) & mask)
{
const Player* receiver = ThePlayerList->getConstSlot(i);
if (receiver && receiver->isPlayerActive())
{
recipients++;
if (sender->getRelationship(receiver->getDefaultTeam()) == ALLIES)
allies++;
}
}
}

// Team message if all recipients are allies and at least one exists
return (recipients > 0 && recipients == allies);
}

void ConnectionManager::processChat(Int playerID, ChatMessage* msg)
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

const ChatMessage*

{
const Player* player = ThePlayerList->getConstSlot(playerID);
if (!player) return;
Copy link

@xezon xezon Nov 26, 2025

Choose a reason for hiding this comment

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

new line after condition


const Player* localPlayer = ThePlayerList->getLocalPlayer();
Bool fromObserver = !player->isPlayerActive();

UnicodeString name(player->getDisplayName());
UnicodeString unitext;

// Determine whether this is a team message
Bool isTeamMessage = FALSE;
if (player->isPlayerActive())
{
isTeamMessage = isTeamChat(player, msg->getPlayerMask());
}

// Use localized formatted strings (via FETCH_OR_SUBSTITUTE_FORMAT)
if (isTeamMessage)
{
// In your .csf file, define:
// CHAT:TeamFormat = (%s) [%s] %s
UnicodeString teamPrefix = TheGameText->FETCH_OR_SUBSTITUTE("GUI:Team", L"TEAM");
unitext = TheGameText->FETCH_OR_SUBSTITUTE_FORMAT(
"CHAT:TeamFormat",
L"(%ls) [%ls] %ls",
teamPrefix.str(),
name.str(),
msg->getText().str()
);
}
else
{
// In your .csf file, define:
// CHAT:GlobalFormat = [%s] %s
unitext = TheGameText->FETCH_OR_SUBSTITUTE_FORMAT(
"CHAT:GlobalFormat",
L"[%ls] %ls",
name.str(),
msg->getText().str()
);
}

// DEBUG_LOG(("ConnectionManager::processChat - got message from player %d (mask %8.8X), message is %ls",
// playerID, msg->getPlayerMask(), unitext.str()));

Bool amIObserver = !localPlayer->isPlayerActive();
Bool canSeeChat = (amIObserver || !fromObserver) && !TheGameInfo->getConstSlot(playerID)->isMuted();

if (((1 << m_localSlot) & msg->getPlayerMask()) && canSeeChat)
{
TheInGameUI->message(UnicodeString(L"%ls"), unitext.str());
}
}

if ( ((1<<m_localSlot) & msg->getPlayerMask() ) && canSeeChat )
{
Expand Down