Page MenuHomeWildfire Games

Allow conversion of config values to u32
ClosedPublic

Authored by elexis on Jun 5 2018, 10:49 PM.

Details

Summary

We want to load some millisecond value from the config files, 60 seconds in D1513.
But int is the only integer type supported in Config.db and that has 16bit or more:
http://en.cppreference.com/w/cpp/language/types
So if we want to allow increase from 60s to beyond 65s, we need to either use a float and then convert to u32 in enet, or change the config system.

Test Plan

JS number max is 2^53 -1
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
So there is the assumption that the number in the config file is in the u32 range, otherwise the user doesn't get the number he wanted.

Where is the Get function?

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.Jun 5 2018, 10:49 PM
Vulcan added a subscriber: Vulcan.Jun 5 2018, 10:59 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/635/display/redirect

Bit of discussion on irc yesterday.

So the question is how sane that conversion is, in which cases it will fail.

stringstream
http://www.cplusplus.com/reference/sstream/stringstream/
stringstream >> operator
http://www.cplusplus.com/reference/istream/istream/operator%3E%3E/
(1) arithmetic types

Extracts and parses characters sequentially from the stream to interpret them as the representation of a value of the proper type, which is stored as the value of val.
it calls num_get::get (using the stream's selected locale) to perform both the extraction and the parsing operations

http://www.cplusplus.com/reference/locale/num_get/get/
The function stops reading characters from the sequence as soon as one character cannot be part of a valid numerical expression

Anyway u32 should give enough room for anyone who legitimately wants to use this.
Who passes something other than u32 either asked to go back in time or for more than 136 days of lag tolerance.

There are many CFG_GET_VAL calls that look like they might be better of with u32 as well.

source/ps/ConfigDB.cpp
65 ↗(On Diff #6732)

This template function is called for numeric types.

Stan added a subscriber: Stan.Jun 6 2018, 12:17 PM
Stan added inline comments.
source/ps/ConfigDB.cpp
112 ↗(On Diff #6732)

Why not i32 while you are at it ?

elexis added inline comments.Jun 6 2018, 6:37 PM
source/ps/ConfigDB.cpp
112 ↗(On Diff #6732)

i64 u64 i16 u16 could have use cases too
mostly someone with time could go through the calls and check for each type if we really want an int or something possibly larger.

vladislavbelov accepted this revision.Jun 6 2018, 7:52 PM
This revision is now accepted and ready to land.Jun 6 2018, 7:52 PM
This revision was automatically updated to reflect the committed changes.