[PATCH] ConfigDB complains about empty strings

[elexis]

#3979 added the first two config entries that allow to enter strings.

However the current options code throws an error if the user saves an empty string to be used later:

ERROR: Encountered config setting 'playername.singleplayer' without value while parsing 'config/user.cfg' on line 12

Reproduce: Empty the singleplayername, close the dialog, enter the singleplayermenu. Reported by Imarok in irc on 2016-05-16.

It will replace the emptystring automatically with the OS username. But the message is still shown and misleading.

The message was added in r15980.

Adding a default.cfg entry doesn't influence the appearance of the message.

If default.cfg contains a non-empty string, the empty string from user.cfg won't replace it and the message is still shown.

20:51 <@leper> removing it works, but hides the fact that a higher priority config file cannot delete settings by just setting it to ""

It were consistent if the config system wouldn't care whether the given string was empty and just use that if specified (even when passing a comma-separated list).

[elexis]

If a comma separated list is given, the empty strings will be recognized correctly too, even if the list ends with a comma.

[elexis]

See comments in irc (​http://irclogs.wildfiregames.com/2016-06-14-QuakeNet-%230ad-dev.log)

[elexis]

Backlogging due to lack of progress.

[elexis]

Besides the empty SP/MP playername, the empty buddylist from ​https://code.wildfiregames.com/D209 also triggers this error.

From 2016-05-16:

20:32 < elexis> it's due to the defaults.cfg not having an entry for that
20:37 < elexis> we could just delete that LOGERROR
20:50 < elexis> it was added in r15980
20:50 < WildfireBot> r15980. Author: leper. Commit message: Use an FSM to parse our config files instead of using CParser. Refs <a class="closed ticket" href="/ticket/2589" title="task: Remove CParser (closed: fixed)">#2589</a>. Print error messages if we encountered an invalid setting.  http://trac.wildfiregames.com/changeset/15980
20:51 <@leper> removing it works, but hides the fact that a higher priority config file cannot delete settings by just setting it to ""
20:52 <@leper> which is the reason this message exists
20:54 < elexis> so it would overwrite the config value with emptystring
20:54 < elexis> not sure if we need a warning / error msg for that
21:14 <@leper> elexis: nope, it ignores the setting
21:15 <@leper> elexis: as it would ignore the last line otherwise I think it should stay
21:15 <@leper> and if people edit the config file manually (which they shouldn't have to) then they should fix warnings they cause
21:15 < elexis> ah, so if default.cfg has foo="bar" and user.cfg has foo="" it would yield foo="bar"? hm
21:17 <@leper> very likely, check the code to be sure

From 2016-05-19:

14:31 <@leper> if someone else fixes all the issues that will arise, meh
14:31 <@leper> if not, no
14:31 < elexis> which issues would arise?
14:31 < elexis> if the config file specifies empty string, it should load empty string, no?
14:31 <@leper> what happens if code expects a setting to actually be there?
14:32 <@leper> and being set to something
14:32 < elexis> then that function should be fixed
14:34 <@leper> likely, but then again there are likely countless places that will break in one way or another from someone feeding you bogus config data
14:34 <@leper> (which shouldn't be any more trusted than any other user input, which is why the config parser is conservative in accepting data
14:34 <@leper> )
14:40 < elexis> the users of config values shouldnt rely on not getting an empty string, I can check many occurances. (there are probably many cases for broken user data which are not checked also)
14:43 <@leper> true, but you have to start somewhere
14:44 < elexis> start: the user cant break anything from the GUI :P
14:46 <@leper> fix: check if a config setting exists before using it?
14:47 <@leper> (or unassign me from config related tickets and do whatever if I don't have to bother about any bugs arising from that)

From 2016-06-15:

17:14 <@leper> #3990 I still don't think that is the way to go, but if someone wants to handle and be responsible for all the bugs that will cause, meh