Skip to content
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.

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

Fix crash when ending a chat line with ¬ #73

Closed
rherriman opened this issue Jun 10, 2020 · 1 comment
Closed

Fix crash when ending a chat line with ¬ #73

rherriman opened this issue Jun 10, 2020 · 1 comment
Labels
interface Issue with user interface

Comments

@rherriman
Copy link
Member

When outside of a game, you can type "¬" with impunity, unless it is the last character in the line when you press Enter.

The source of the issue seems to lie in std::string CPlayerManagerImpl::GetChatLine(), which examines the line buffer to grab the most recent line of chat by finding the last "¬" character (the assumption being that it marked the end of the line prior to the one we want). A substring is returned containing everything following that point. Unfortunately, when the last character in the buffer just happens to be a "¬", an invalid index is submitted to substr, hence the crash.

Checking for this specific condition is unfortunately not sufficient, because another issue would remain: typing "asdf ¬ qwerty" followed by Enter would result in "qwerty" being returned, rather than the full string.

Here's the backtrace:

* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00007fff702ab33a libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff70367e60 libsystem_pthread.dylib`pthread_kill + 430
    frame #2: 0x00007fff70232808 libsystem_c.dylib`abort + 120
    frame #3: 0x00007fff6d492458 libc++abi.dylib`abort_message + 231
    frame #4: 0x00007fff6d4838a7 libc++abi.dylib`demangling_terminate_handler() + 238
    frame #5: 0x00007fff6efbe5b1 libobjc.A.dylib`_objc_terminate() + 104
    frame #6: 0x00007fff6d491887 libc++abi.dylib`std::__terminate(void (*)()) + 8
    frame #7: 0x00007fff6d4941a2 libc++abi.dylib`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 27
    frame #8: 0x00007fff6d494169 libc++abi.dylib`__cxa_throw + 113
    frame #9: 0x00007fff6d4750c8 libc++.1.dylib`std::__1::__throw_out_of_range(char const*) + 56
    frame #10: 0x00007fff6d466b0e libc++.1.dylib`std::__1::__basic_string_common<true>::__throw_out_of_range() const + 16
    frame #11: 0x00007fff6d466daf libc++.1.dylib`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long, unsigned long, std::__1::allocator<char> const&) + 195
    frame #12: 0x00000001000f34f7 Avara`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::substr(this="¬", __pos=3, __n=18446744073709551615) const at string:3280:12
    frame #13: 0x00000001000f33b5 Avara`CPlayerManagerImpl::GetChatLine(this=0x0000000102905000) at CPlayerManager.cpp:745:20
    frame #14: 0x00000001000f2d47 Avara`CPlayerManagerImpl::RosterMessageText(this=0x0000000102905000, len=0, c="    \"normal\": [\n        -0.11374074921797517,\n        -0.9801174944686045,\n        0.1624628061341268\n      ],\n      \"tris\": [\n?-\x89\x01\x01") at CPlayerManager.cpp:710:90
    frame #15: 0x00000001000d862c Avara`CNetManager::ReceiveRosterMessage(this=0x00000001022a5ee0, slotId=0, len=1, c="\r    \"normal\": [\n        -0.11374074921797517,\n        -0.9801174944686045,\n        0.1624628061341268\n      ],\n      \"tris\": [\n?-\x89\x01\x01") at CNetManager.cpp:349:20
    frame #16: 0x0000000100026c34 Avara`CProtoControl::DelayedPacketHandler(this=0x000000010228d1c0, thePacket=0x0000000101892c80) at CProtoControl.cpp:89:21
    frame #17: 0x0000000100026879 Avara`DelayedProtoHandler(thePacket=0x0000000101892c80, userData="\x98K.") at CProtoControl.cpp:37:24
    frame #18: 0x000000010000ee24 Avara`CCommManager::ProcessQueue(this=0x0000000100f9d990) at CCommManager.cpp:363:33
    frame #19: 0x00000001000d7a41 Avara`CNetManager::ProcessQueue(this=0x00000001022a5ee0) at CNetManager.cpp:188:21
    frame #20: 0x00000001000c3360 Avara`CAvaraGame::GameTick(this=0x0000000102911c00) at CAvaraGame.cpp:815:13
    frame #21: 0x000000010005c3d9 Avara`CAvaraAppImpl::idle(this=0x00000001022612a0) at CAvaraApp.cpp:105:17
    frame #22: 0x0000000100220678 Avara`nanogui::mainloop(refresh=16) at common.cpp:82:25
    frame #23: 0x00000001002c02f7 Avara`main(argc=1, argv=0x00007ffeefbff5d8) at Avara.cpp:67:5
    frame #24: 0x00007fff70163cc9 libdyld.dylib`start + 1
    frame #25: 0x00007fff70163cc9 libdyld.dylib`start + 1
@assertivist
Copy link
Member

assertivist commented Jul 22, 2020

This is fixed but that code still lives on. I think it is needed to divide lines up for the chat tab.

I also don't really like the stuff that goes all the way back out to GUI code to make chat lines, but actually thats how the original did it too:

void CPlayerManagerImpl::FlushMessageText(
Boolean forceAll)
{
Str255 fullLine;
short maxLen;
short baseLen;
GrafPtr savedPort;
TextSettings savedTexs;
OSErr err;
GetPort(&savedPort);
SetPort(theRoster->itsWindow);
GetSetTextSettings(&savedTexs, geneva, 0, 9, srcOr);
do
{
BlockMoveData(playerName, fullLine, playerName[0] + 1);
#if 0
while(fullLine[0] && fullLine[fullLine[0]] == 32)
{ fullLine[0]--;
}
#endif
TruncString(kChatMessageWidth/3, fullLine, smTruncEnd);
fullLine[++fullLine[0]] = ':';
fullLine[++fullLine[0]] = ' ';
baseLen = fullLine[0];
maxLen = lineBuffer[0];
if(maxLen + baseLen > 250)
{ maxLen = 250 - baseLen;
}
BlockMoveData(lineBuffer+1, fullLine + 1 + baseLen, maxLen);
fullLine[0] += maxLen;
#if 1
err = WordWrapString(fullLine);
#else
err = TruncString(kChatMessageWidth, fullLine, smTruncEnd);
#endif
if(err == 1 || forceAll)
{ short outLen;
if(err == 1)
{ outLen = fullLine[0] - baseLen - 1;
}
else
{ outLen = maxLen;
}
if(outLen != 0) theRoster->NewChatLine(fullLine);
BlockMoveData(lineBuffer + outLen + 1, lineBuffer + 1, lineBuffer[0] - outLen);
lineBuffer[0] -= outLen;
if(err == 0) forceAll = false;
}
} while(err == 1 || forceAll);
isLocalPlayer = (theNetManager->itsCommManager->myId == slot);
if(isLocalPlayer)
{ theRoster->SetChatLine(fullLine);
}
RestoreTextSettings(&savedTexs);
SetPort(savedPort);
}

additionally, i was gonna say something about making it match by renaming the function to FlushMessage but then i realized that I ALSO ignored an old function RosterKeyPress when i was making special characters show up in the roster chat! so i have no room to talk! And i'm closing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interface Issue with user interface
Projects
None yet
Development

No branches or pull requests

2 participants