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)

5266.cpp.patch (565 bytes ) - added by Amplikon 5 years ago.
Change to cpp file
5266.xml.patch (2.4 KB ) - added by Amplikon 5 years ago.
Apply max_length in three inputs: lobby chat window, game room chat window, game name creation

Download all attachments as: .zip

Change History (9)

by Amplikon, 5 years ago

Attachment: 5266.cpp.patch added

Change to cpp file

by Amplikon, 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 Amplikon, 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 elexis, 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 Amplikon, 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 smiley, 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 Stan, 5 years ago

Cc: Phab:D2377 added
Milestone: BacklogAlpha 24
Owner: set to Stan

comment:6 by Stan, 5 years ago

Cc: Phab:D2377 removed
Patch: Phab:D2377

comment:7 by Stan, 4 years ago

Resolution: fixed
Status: newclosed

In 23927:

Fix text input max_length attribute. fixes rP15785
Reviewed by: @wraitii
Fixes #5266
Differential Revision: https://code.wildfiregames.com/D2377

Note: See TracTickets for help on using tickets.