Strip surrounding whitespace from player names#1526
Strip surrounding whitespace from player names#1526Rainyan wants to merge 16 commits intoNeotokyoRebuild:masterfrom
Conversation
Rainyan
changed the title
There's a disconnect between what the settings page tells us our display name is vs what our display name actually is. |
Rainyan
force-pushed
the
bug/empty-name
branch
2 times, most recently
from
200bee3 to
0b647c4
Compare
This should be now fixed. |
Rainyan
requested review from
a team
and removed request for
AdamTadeusz and
sunzenshen
|
Never mind I see this gets applied in real time, cool (edit) one problem with that I guess is that if I want multiple spaces in the middle of my name (which technically is allowed), I have to write the name without those spaces and then move my cursor back to the middle and then put all those spaces in. |
|
One thing that confuses me is why whenever I place a breakpoint in say V_IsMeanSpaceW where the return value is set to true, and enter a character that causes that breakpoint to be hit, a breakpoint in VarTrimmer inside the block called when predicate() returns true still isn't hit 8mb.video-uPg-TJJCbzYn.mp4 |
|
ok i figured it out its for the first character only, but still a bit confused |
This is not an issue with this pr specifically but more with NeoUI::TextEdit, where large unicode characters take up more space than standard characters, so in this example the string ąąąąąąąąąąąąąąąąąąąąąąąąąąąąąąą takes up double the number of characters as aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa and so the name is invalid, but the TextEdit input doesn't stop me from entering more characters earlier. Nagrywanie.ekranu.2026-03-18.124512.mp4Anyway, good to see that it doesn't cause any problems in the end 👍 |
Block all bad characters
NeoName and NeoClantag wchar conversions via g_pVGuiLocalize->ConvertANSIToUnicode will return "#empty" for an empty input. In those cases, move the null terminator to the beginning of the string, so that we get: L"#empty" -> L"" because we don't want to display the value "#empty" for the user in the UI.
Remove the typo'd redundant line.
MAX_PLACE_NAME_LENGTH -> MAX_PLAYER_NAME_LENGTH
After the VarTrimmer input is moved one position to the left, the loop index and the loop condition index must be decremented by one to compensate for it. This happened to work previously by coincidence in the IMGUI loop, since it would fix 1 trailing character at a time before incorrectly exiting the loop, and continue the work on the next UI draw call. But indeed the correct behaviour would be to process the whole input in one loop.
|
Rebased to master and addressed the bugs found by @sunzenshen Also fixed a case where the char -> wchar conversion via the vguilocalize utility could return New changes (aside from the rebase): |
src/game/client/cdll_client_int.h
Outdated
| ConVarRef cvarRef(cvar); | ||
| V_strcpy_safe(mutStr, cvarRef.GetString()); | ||
|
|
||
| if (V_strlen(cvarRef.GetString()) >= STR_LIMIT_SIZE) |
There was a problem hiding this comment.
Optional nit:
Just to sanity check, but is it intended that Q_UnicodeRepair will be called only when the string is over STR_LIMIT_SIZE?
I did the see the SDK comment:
// Repair invalid Unicode strings by dropping truncated characters and fixing improperly-double-encoded UTF-16 sequences.
// Unlike conversion functions which replace with '?' by default, a repair operation assumes that you know that something
// is wrong with the string (eg, mid-sequence truncation) and you just want to do the best possible job of cleaning it up.
// You can pass a REPLACE or FAIL policy if you would prefer to replace characters with '?' or clear the entire string.
// Returns nonzero on success, or 0 if the policy is FAIL and an invalid sequence was found.
And I suppose this should only be called when something is known to be wrong, but is the string size the only error case we know of in this context?
There was a problem hiding this comment.
I can't remember why that length check is there, but yeah we should probably just always call that function.
| static void NeoConVarFixPrintable(IConVar *cvar, const char *, float) | ||
| { | ||
| // prevent reentrancy | ||
| static bool bStaticCallbackChangedCVar = false; |
There was a problem hiding this comment.
Optional nit because to my understanding, only 1 translation unit from cdll_client_int.cpp is using NeoConVarFixPrintable, but if cdll_client_int.h is #included in other contexts, wouldn't this static get duplicated?
Would an inline have accomplished the same intentions?
template <int STR_LIMIT_SIZE>
requires (STR_LIMIT_SIZE > 0)
inline void NeoConVarFixPrintable(IConVar *cvar, const char *, float)
{ ... }There was a problem hiding this comment.
Oh whoops, you're right. For some reason I assumed the template with the same STR_LIMIT_SIZE would resolve to the same definition but that's obviously not the case here.
|
|
||
| if (hasBadCharInPos || hasDoubleBlankInPos) | ||
| { | ||
| std::wmemmove(&input[i], &input[i + 1], zeroIdx-i); |
There was a problem hiding this comment.
optional nit:
supposedly Q_memmove is the equivalent Valve utility, e.g.
Q_memmove(&input[i], &input[i + 1], (zeroIdx - i) * sizeof(wchar_t));
|
Also fixed some instances where the empty string ( |
4 participants
Description
Strip surrounding whitespace from player name variables
name(Steam name),neo_name, andneo_clantag.Toolchain
Linked Issues