Opened 6 years ago
Closed 4 years ago
#5266 closed defect (fixed)
Input max_length field doesn't work anymore
Reported by: | elexis | Owned by: | Stan |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 24 |
Component: | Core engine | Keywords: | simple |
Cc: | Patch: | Phab:D2377 |
Description
r1904 introduced max_length
field to CInput.cpp
r7909 used the max_length
field to limit chat to 80 characters
(r8494 introduced 256 character limit for chat in escapeText
)
r15785 broke the max_length
field in CInput.cpp
The field is useful because we want to restrict some userinput fields that are broadcasted such as the lobby chat length or the lobby gamename #3651.
Attachments (2)
Change History (9)
by , 5 years ago
Attachment: | 5266.cpp.patch added |
---|
by , 5 years ago
Attachment: | 5266.xml.patch added |
---|
Apply max_length in three inputs: lobby chat window, game room chat window, game name creation
comment:1 by , 5 years ago
I tested manually this change in three fields:
- lobby chat field
- game room chat field
- game creation name field
In all cases input allows to provide up to this number of characters which are specified in xlm files. Thought it's only CInput class logic, so nothing stops this strings to grow bigger, if there is another possibility to do this than by CInput.
comment:2 by , 5 years ago
Usually we use http://code.wildfiregames.com/ for patch reviews, either through the webinterface or the commandline tool arcanist (the latter being currently bugged I heard), see also wiki:SubmittingPatches
Don't limit those lobby chat messages to 256; the ticket was writen when that 256 limit was removed, r21853 :P
There should be a limit, but 256 is too few if one types like 3 sentences already. For the servername 256 is good, because the names should be names and not consist of more than one sentence.
The C++ patch seems incomplete, as there is also the SDL_TEXTINPUT
way of adding characters, or the copy-paste way, possibly more.
Also add yourself to programmers.json.
Good to see an enthousiastic lobby player pick up the code!
comment:3 by , 5 years ago
Ok, I will use Phabricator in the future, I just thought I don't need this because I'm posting the patch here.
Indeed 256 characters for chat message is pretty low. I guess 1024 would be ok, as someone posted in #3651? Anyway, if the limitation in server name exists as stated here r16895, then I guess limit on input should be at most same or less. And I will have a look at other methods to add characters.
I appreciate your fast answer as I did spend some time mostly trying to understand the code flow, at least I know I do something helpful.
comment:4 by , 5 years ago
Minor comments.
- Use static_cast<int> instead of (int).
- Unsigned integer probably.
- Sure it should not be a
break;
? (Same thing is in L308, Plus, from a quick skim,break;
makes more sense to me. Could be wrong of course.)
comment:5 by , 5 years ago
Cc: | added |
---|---|
Milestone: | Backlog → Alpha 24 |
Owner: | set to |
comment:6 by , 5 years ago
Cc: | removed |
---|---|
Patch: | → Phab:D2377 |
Change to cpp file