Skip to content

Fix text chat functionality #8

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

Fix text chat functionality #8

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
49 changes: 42 additions & 7 deletions Minecraft.Client/ChatScreen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const wstring ChatScreen::allowedChars = SharedConstants::acceptableLetters;
ChatScreen::ChatScreen()
{
frame = 0;
caretPos = 0;
}

void ChatScreen::init()
Expand Down Expand Up @@ -47,18 +48,46 @@ void ChatScreen::keyPressed(wchar_t ch, int eventKey)
minecraft->setScreen(NULL);
return;
}
if (eventKey == Keyboard::KEY_BACK && message.length() > 0) message = message.substr(0, message.length() - 1);
if (allowedChars.find(ch) >= 0 && message.length() < SharedConstants::maxChatLength)
if (eventKey == Keyboard::KEY_LEFT)
{
if (caretPos > 0)
{
caretPos--;
}
return;
}
if (eventKey == Keyboard::KEY_RIGHT)
{
if (caretPos < message.length())
{
caretPos++;
}
return;
}
if (eventKey == Keyboard::KEY_BACK && caretPos > 0 && message.length() > 0)
{
message.erase(caretPos - 1, 1);
caretPos--;
return;
}
if (ch >= 32 && SharedConstants::acceptableLetters.find(ch) != wstring::npos && message.length() < SharedConstants::maxChatLength)
{
message += ch;
message.insert(caretPos, 1, ch);
caretPos++;
Comment on lines +73 to +76
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

This code correctly queries SharedConstants::acceptableLetters at runtime, but ChatScreen::allowedChars (defined at file scope) is still initialized from SharedConstants::acceptableLetters before SharedConstants::staticCtor() populates it, so allowedChars will remain an empty copy. Since it’s no longer used here, consider removing allowedChars (or making it a runtime reference/accessor) to avoid the original empty-allowed-chars bug resurfacing later.

Copilot uses AI. Check for mistakes.
}

}

void ChatScreen::render(int xm, int ym, float a)
{
wstring displayMessage = L"> " + message;
if (frame / 6 % 2 == 0)
{
displayMessage.insert(2 + caretPos, 1, L'_');
}

fill(2, height - 14, width - 2, height - 2, 0x80000000);
drawString(font, L"> " + message + (frame / 6 % 2 == 0 ? L"_" : L""), 4, height - 12, 0xe0e0e0);
drawString(font, displayMessage, 4, height - 12, 0xe0e0e0);

Screen::render(xm, ym, a);
}
Expand All @@ -69,15 +98,21 @@ void ChatScreen::mouseClicked(int x, int y, int buttonNum)
{
if (minecraft->gui->selectedName != L"") // 4J - was NULL comparison
{
if (message.length() > 0 && message[message.length()-1]!=L' ')
if (caretPos > 0 && message[caretPos - 1] != L' ')
{
message += L" ";
message.insert(caretPos, 1, L' ');
caretPos++;
}
message += minecraft->gui->selectedName;
message.insert(caretPos, minecraft->gui->selectedName);
caretPos += (unsigned int)minecraft->gui->selectedName.length();
unsigned int maxLength = SharedConstants::maxChatLength;
if (message.length() > maxLength)
{
message = message.substr(0, maxLength);
if (caretPos > message.length())
{
caretPos = (unsigned int)message.length();
}
}
}
else
Expand Down
1 change: 1 addition & 0 deletions Minecraft.Client/ChatScreen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class ChatScreen : public Screen
wstring message;
private:
int frame;
unsigned int caretPos;

public:
ChatScreen(); //4J added
Expand Down
6 changes: 6 additions & 0 deletions Minecraft.Client/ClientConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,12 @@ void ClientConnection::handleChat(shared_ptr<ChatPacket> packet)
message = replaceAll(message,L"{*PLAYER*}",playerDisplayName);
break;

case ChatPacket::e_ChatCustom:
// Raw text message (e.g. "<Player> message") — use first string arg directly
if (!packet->m_stringArgs.empty())
message = packet->m_stringArgs[0];
Comment on lines +1603 to +1604
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

In the e_ChatCustom branch, if m_stringArgs is unexpectedly empty, message remains empty and the function will still add a blank chat line. Consider returning early (or setting displayOnGui = false) when there’s no payload text to display.

Suggested change
if (!packet->m_stringArgs.empty())
message = packet->m_stringArgs[0];
if (!packet->m_stringArgs.empty())
{
message = packet->m_stringArgs[0];
}
else
{
// No payload text: don't display an empty chat line
displayOnGui = false;
}

Copilot uses AI. Check for mistakes.
break;

default:
message = playerDisplayName;
break;
Expand Down
23 changes: 7 additions & 16 deletions Minecraft.Client/Common/UI/UIScene_HUD.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,25 +656,16 @@ void UIScene_HUD::handleTimerComplete(int id)
if(pMinecraft->localplayers[m_iPad]!= NULL)
{
Gui *pGui = pMinecraft->gui;
//DWORD messagesToDisplay = min( CHAT_LINES_COUNT, pGui->getMessagesCount(m_iPad) );

// Chat messages are now rendered exclusively by Gui::render() (OpenGL path).
// Disable the Iggy/flash chat labels in the HUD scene to avoid duplicate rendering.
for( unsigned int i = 0; i < CHAT_LINES_COUNT; ++i )
{
float opacity = pGui->getOpacity(m_iPad, i);
if( opacity > 0 )
{
m_controlLabelBackground[i].setOpacity(opacity);
m_labelChatText[i].setOpacity(opacity);
m_labelChatText[i].setLabel( pGui->getMessagesCount(m_iPad) ? pGui->getMessage(m_iPad,i) : L"" );

anyVisible = true;
}
else
{
m_controlLabelBackground[i].setOpacity(0);
m_labelChatText[i].setOpacity(0);
m_labelChatText[i].setLabel(L"");
}
m_controlLabelBackground[i].setOpacity(0);
m_labelChatText[i].setOpacity(0);
m_labelChatText[i].setLabel(L"");
}

if(pGui->getJukeboxOpacity(m_iPad) > 0) anyVisible = true;
m_labelJukebox.setOpacity( pGui->getJukeboxOpacity(m_iPad) );
m_labelJukebox.setLabel( pGui->getJukeboxMessage(m_iPad) );
Expand Down
9 changes: 2 additions & 7 deletions Minecraft.Client/Gui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -906,13 +906,9 @@ void Gui::render(float a, bool mouseFree, int xMouse, int yMouse)
glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
glDisable(GL_ALPHA_TEST);

// 4J Stu - We have moved the chat text to a xui
#if 0
glPushMatrix();
// 4J-PB we need to move this up a bit because we've moved the quick select
//glTranslatef(0, ((float)screenHeight) - 48, 0);
glTranslatef(0.0f, (float)(screenHeight - iSafezoneYHalf - iTooltipsYOffset - 16 - 3 + 22) - 24.0f, 0.0f);
// glScalef(1.0f / ssc.scale, 1.0f / ssc.scale, 1);
// Position chat messages just above the armor/health bar row (40px above toolbar top)
glTranslatef(0.0f, (float)(screenHeight - iSafezoneYHalf - iTooltipsYOffset) - 40.0f, 0.0f);

// 4J-PB - we need gui messages for each of the possible 4 splitscreen players
if(bDisplayGui)
Expand Down Expand Up @@ -951,7 +947,6 @@ void Gui::render(float a, bool mouseFree, int xMouse, int yMouse)
}
}
glPopMatrix();
#endif

// 4J Stu - Copied over but not used
#if 0
Expand Down
5 changes: 4 additions & 1 deletion Minecraft.Client/Minecraft.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3598,10 +3598,13 @@ void Minecraft::tick(bool bFirst, bool bUpdateTextures)
// #endif


if(Keyboard::isKeyDown(options->keyChat->key))
static bool s_chatKeyWasDown = false;
bool chatKeyDown = Keyboard::isKeyDown(options->keyChat->key) || g_KBMInput.IsKeyDown('T');
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

chatKeyDown hardcodes the 'T' key via g_KBMInput.IsKeyDown('T'), which bypasses options->keyChat (key rebinding) and is redundant because Keyboard::isKeyDown(options->keyChat->key) already queries g_KBMInput on Win64. Remove the hardcoded 'T' check (or derive the VK from the configured key) so chat opens only for the configured chat hotkey.

Suggested change
bool chatKeyDown = Keyboard::isKeyDown(options->keyChat->key) || g_KBMInput.IsKeyDown('T');
bool chatKeyDown = Keyboard::isKeyDown(options->keyChat->key);

Copilot uses AI. Check for mistakes.
if (screen == NULL && chatKeyDown && !s_chatKeyWasDown)
{
setScreen(new ChatScreen());
}
s_chatKeyWasDown = chatKeyDown;

#if 0
// 4J - TODO - some replacement for input handling...
Expand Down
48 changes: 17 additions & 31 deletions Minecraft.Client/PlayerConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void PlayerConnection::disconnect(DisconnectPacket::eDisconnectReason reason)
send( shared_ptr<DisconnectPacket>( new DisconnectPacket(reason) ));
connection->sendAndQuit();
// 4J-PB - removed, since it needs to be localised in the language the client is in
//server->players->broadcastAll( shared_ptr<ChatPacket>( new ChatPacket(L"�e" + player->name + L" left the game.") ) );
//server->players->broadcastAll( shared_ptr<ChatPacket>( new ChatPacket(L"�e" + player->name + L" left the game.") ) );
if(getWasKicked())
{
server->getPlayers()->broadcastAll( shared_ptr<ChatPacket>( new ChatPacket(player->name, ChatPacket::e_ChatPlayerKickedFromGame) ) );
Expand Down Expand Up @@ -569,7 +569,7 @@ void PlayerConnection::onDisconnect(DisconnectPacket::eDisconnectReason reason,
if( done ) return;
// logger.info(player.name + " lost connection: " + reason);
// 4J-PB - removed, since it needs to be localised in the language the client is in
//server->players->broadcastAll( shared_ptr<ChatPacket>( new ChatPacket(L"�e" + player->name + L" left the game.") ) );
//server->players->broadcastAll( shared_ptr<ChatPacket>( new ChatPacket(L"�e" + player->name + L" left the game.") ) );
if(getWasKicked())
{
server->getPlayers()->broadcastAll( shared_ptr<ChatPacket>( new ChatPacket(player->name, ChatPacket::e_ChatPlayerKickedFromGame) ) );
Expand Down Expand Up @@ -636,38 +636,24 @@ void PlayerConnection::handleSetCarriedItem(shared_ptr<SetCarriedItemPacket> pac

void PlayerConnection::handleChat(shared_ptr<ChatPacket> packet)
{
// 4J - TODO
#if 0
wstring message = packet->message;
if (packet->m_stringArgs.empty()) return;

wstring message = packet->m_stringArgs[0];
if (message.length() > SharedConstants::maxChatLength)
{
disconnect(L"Chat message too long");
return;
}
message = message.trim();
for (int i = 0; i < message.length(); i++)
{
if (SharedConstants.acceptableLetters.indexOf(message.charAt(i)) < 0 && (int) message.charAt(i) < 32)
{
disconnect(L"Illegal characters in chat");
return;
}
}

if (message.startsWith("/"))
{
handleCommand(message);
} else {
message = "<" + player.name + "> " + message;
logger.info(message);
server.players.broadcastAll(new ChatPacket(message));
}
chatSpamTickCount += SharedConstants::TICKS_PER_SECOND;
if (chatSpamTickCount > SharedConstants::TICKS_PER_SECOND * 10)
{
disconnect("disconnect.spam");
}
#endif
// Trim leading/trailing spaces
size_t start = message.find_first_not_of(L' ');
size_t end = message.find_last_not_of(L' ');
if (start == wstring::npos) return;
message = message.substr(start, end - start + 1);
if (message.empty()) return;

// Format as "<PlayerName> message" and broadcast to all clients
wstring formatted = L"<" + player->name + L"> " + message;
Comment on lines +654 to +655
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

Server-side chat validation was removed: the code now accepts any characters from packet->m_stringArgs[0] (after trimming spaces only) and simply returns on oversize messages. Since clients can be modified, the server should still reject/control disallowed control characters (e.g., newlines, other < 0x20) and enforce policy consistently (sanitize or disconnect) to prevent chat log/UI injection issues.

Suggested change
// Format as "<PlayerName> message" and broadcast to all clients
wstring formatted = L"<" + player->name + L"> " + message;
// Remove disallowed control characters to prevent chat/log injection
wstring cleanMessage;
cleanMessage.reserve(message.size());
for (wstring::const_iterator it = message.begin(); it != message.end(); ++it)
{
wchar_t ch = *it;
// Allow printable characters only (exclude control chars < 0x20)
if (ch >= 0x20)
{
cleanMessage.push_back(ch);
}
}
if (cleanMessage.empty()) return;
// Format as "<PlayerName> message" and broadcast to all clients
wstring formatted = L"<" + player->name + L"> " + cleanMessage;

Copilot uses AI. Check for mistakes.
server->getPlayers()->broadcastAll(shared_ptr<ChatPacket>(new ChatPacket(formatted)));
}
Comment on lines 637 to 657
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

handleChat() no longer increments chatSpamTickCount, but PlayerConnection::tick() still decrements it every tick. This effectively disables the existing chat spam throttling logic and leaves a counter that never increases. Reintroduce spam tracking (increment per message and disconnect/throttle when over the threshold) or remove the counter/logic if spam protection is intentionally being dropped.

Copilot uses AI. Check for mistakes.

void PlayerConnection::handleCommand(const wstring& message)
Expand Down Expand Up @@ -740,13 +726,13 @@ int PlayerConnection::countDelayedPackets()
void PlayerConnection::info(const wstring& string)
{
// 4J-PB - removed, since it needs to be localised in the language the client is in
//send( shared_ptr<ChatPacket>( new ChatPacket(L"�7" + string) ) );
//send( shared_ptr<ChatPacket>( new ChatPacket(L"�7" + string) ) );
}

void PlayerConnection::warn(const wstring& string)
{
// 4J-PB - removed, since it needs to be localised in the language the client is in
//send( shared_ptr<ChatPacket>( new ChatPacket(L"�9" + string) ) );
//send( shared_ptr<ChatPacket>( new ChatPacket(L"�9" + string) ) );
}

wstring PlayerConnection::getConsoleName()
Expand Down
5 changes: 5 additions & 0 deletions Minecraft.Client/Screen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ void Screen::render(int xm, int ym, float a)
}
}

void Screen::injectKeyPressed(wchar_t eventCharacter, int eventKey)
{
keyPressed(eventCharacter, eventKey);
}

void Screen::keyPressed(wchar_t eventCharacter, int eventKey)
{
if (eventKey == Keyboard::KEY_ESCAPE)
Expand Down
1 change: 1 addition & 0 deletions Minecraft.Client/Screen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class Screen : public GuiComponent

Screen(); // 4J added
virtual void render(int xm, int ym, float a);
void injectKeyPressed(wchar_t eventCharacter, int eventKey);
protected:
virtual void keyPressed(wchar_t eventCharacter, int eventKey);
public:
Expand Down
51 changes: 51 additions & 0 deletions Minecraft.Client/Windows64/Windows64_Minecraft.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "..\..\Minecraft.World\ThreadName.h"
#include "..\..\Minecraft.Client\StatsCounter.h"
#include "..\ConnectScreen.h"
#include "..\Screen.h"
//#include "Social\SocialManager.h"
//#include "Leaderboards\LeaderboardManager.h"
//#include "XUI\XUI_Scene_Container.h"
Expand Down Expand Up @@ -301,6 +302,19 @@ HWND GetMinecraftWindowHWND()
return g_hWnd;
}

static int KeyboardConstFromVK(int vk)
{
for (int keyConst = Keyboard::KEY_A; keyConst <= Keyboard::KEY_RIGHT; ++keyConst)
{
if (Keyboard::toVK(keyConst) == vk)
{
return keyConst;
}
}

return -1;
}

static bool g_isFullscreen = false;
static RECT g_windowedRect = {};
static LONG g_windowedStyle = 0;
Expand Down Expand Up @@ -571,6 +585,20 @@ LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
vk = (lParam & (1 << 24)) ? VK_RCONTROL : VK_LCONTROL;
else if (vk == VK_MENU)
vk = (lParam & (1 << 24)) ? VK_RMENU : VK_LMENU;
Minecraft *pMinecraft = Minecraft::GetInstance();
if (pMinecraft != NULL && pMinecraft->screen != NULL)
{
int keyConst = KeyboardConstFromVK(vk);
if (keyConst >= 0)
{
if (vk == VK_ESCAPE || vk == VK_BACK || vk == VK_UP || vk == VK_DOWN || vk == VK_LEFT || vk == VK_RIGHT || vk == VK_TAB)
{
pMinecraft->screen->injectKeyPressed(0, keyConst);
return 0;
}
}
}

g_KBMInput.OnKeyDown(vk);
break;
}
Expand All @@ -588,6 +616,29 @@ LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
break;
}

case WM_CHAR:
{
Minecraft *pMinecraft = Minecraft::GetInstance();
if (pMinecraft != NULL && pMinecraft->screen != NULL)
{
wchar_t eventCharacter = (wchar_t)wParam;
int eventKey = 0;

if (wParam == VK_BACK)
{
return 0;
}
else if (wParam == VK_RETURN)
{
eventKey = Keyboard::KEY_RETURN;
}

pMinecraft->screen->injectKeyPressed(eventCharacter, eventKey);
return 0;
}
break;
}

case WM_LBUTTONDOWN:
g_KBMInput.OnMouseButtonDown(KeyboardMouseInput::MOUSE_LEFT);
break;
Expand Down