Paths

Table of Contentst

Buddy list empty strings
ClosedPublic
Actions

Authored by elexis on Apr 29 2017, 7:18 AM.

Details

Reviewers

causative
vladislavbelov
ffffffff

Commits

rP19492: Don't save an empty string to the user config if the lobby buddy list is empty…

Summary

As reported by Imarok and fatherbushido, when adding and removing a buddy (Refs D209 / rP19433) so that the buddy list is empty,the code saves an empty string to the user config, which the config system is not happy about. Having empty strings is also an issue with the playername and in theory it might considered desirable to save an empty string #3990.

Imarok proposed to use JSON instead of CSV, but I prefered to have something that is as easy as possible for the user to read in case he opens the config file with a text editor.
The placeholder for the empty string is the delimiter comma which is already in use as an empty string workaround in default.cfg, so this patch is only completing a forgotton case in D209 (not adding a brand new workaround).

Test Plan

Have a local.cfg without a buddy list. Enter the lobby, set someone as a buddy, remove it again and logout.
The error occurs when saving the config. Do the same with the patch and see the issue being gone.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Apr 29 2017, 7:18 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/920/ for more details.

Harbormaster completed remote builds in B1326: Diff 1526.Apr 29 2017, 9:58 AM

elexis mentioned this in rP19433: Mark players as buddies in the lobby and add a symbol to identify them in the….Apr 30 2017, 3:13 PM

elexis retitled this revision from Summary: As reported by Imarok and fatherbushido, when adding and removing a buddy (Refs D209 / rP19433) so that the buddy list is empty, the code saves an empty string to the user config, which the config system is not happy about. Having empty... to Buddy list empty strings.

elexis edited the summary of this revision. (Show Details)

This would implementation with JSON

patch2 KBDownload

It would result in config values like "[\"Hannibal_Baraq\",\"Ratings\",\"user1\"]" which still don't seem that nice to me

It works as expected, no any error. This solution is valid while we don't have a parser for empty strings.

This revision is now accepted and ready to land.May 1 2017, 12:35 AM

ffffffff resigned from this revision.May 1 2017, 12:54 AM

Tested and it works. As discussed on IRC, if a nick has a " in it, it will be escaped with a backslash when written to user.cfg. The fact that the escaped nick is then read correctly from user.cfg confuses me, because I don't see how ConfigDB.cpp unescapes the string when reading it - it seems to do a character by character parse with no consideration given to escapes. However... the string does get unescaped, somehow.

In D393#16602, @causative wrote:

Tested and it works. As discussed on IRC, if a nick has a " in it, it will be escaped with a backslash when written to user.cfg. The fact that the escaped nick is then read correctly from user.cfg confuses me, because I don't see how ConfigDB.cpp unescapes the string when reading it - it seems to do a character by character parse with no consideration given to escapes. However... the string does get unescaped, somehow.

JS itself unescapes, so no need for ConfigDB to do it.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation

If you add warn("\causative"); to the JS code and execute it or execute that in the JS console, it will print "causative".

Thanks for the reviews and tests you two!

Closed by commit rP19492: Don't save an empty string to the user config if the lobby buddy list is empty… (authored by elexis). · Explain WhyMay 1 2017, 6:59 AM

This revision was automatically updated to reflect the committed changes.

In D393#16602, @causative wrote:

Tested and it works. As discussed on IRC, if a nick has a " in it, it will be escaped with a backslash when written to user.cfg. The fact that the escaped nick is then read correctly from user.cfg confuses me, because I don't see how ConfigDB.cpp unescapes the string when reading it - it seems to do a character by character parse with no consideration given to escapes. However... the string does get unescaped, somehow.

Check https://code.wildfiregames.com/source/0ad/browse/ps/trunk/source/ps/ConfigDB.cpp;19493$319 is where the magic happens. I get that this is relatively C-like code right there (including doing things in a condition).

Ah, right.

Revision Contents
Changeset List

Path

Size

ps/

trunk/

binaries/

data/

mods/

public/

gui/

lobby/

lobby.js

3 lines

Diff 1576

View Options

ps/trunk/binaries/data/mods/public/gui/lobby/lobby.js

Buddy list empty stringsClosedPublicActions