Configfile include

[thelusiv]

I was reading some of the CONFIG class while updating the wiki page about the format. I noticed a new undocumented feature whereupon one file can include another file. I'm just curious what this is used for and how it behaves...

For instance, what if the current file defines setting "A" in section "1", and the included file has a section "1" with only a setting "B". From my reading of the code, it looks as if there will be no section "1" setting "A" at all. It also looks like it will clear out the "" section even if there is nothing in it.

I think because of the recursive nature of the implementation, it would enter an infinite loop with a pair of files that include each other.

I haven't tested this yet, but if my interpretation is correct, I think this should be fixed. A better way to handle includes might be to scan the file for include lines and load all the includes as separate CONFIG objects, then copy the sections of data into the current object. After all that, re-scan the file, ignoring all include lines, and adding to/overwriting settings in existing (imported) sections. Another way (which would avoid scanning the file twice) would be to enforce a limitation that includes must come before all settings and sections, logging a warning (and ignoring) any include lines that come after the first setting or section heading.

[NaN]

curious what this is used for and how it behaves

The idea is to be able to include an already existing config file and override some of its properties or extend it by adding properties and/or sections. It should be equivalent to copy/pasting the content of the included file into the including. Files are only included once. Circular references are ignored. The include path is relative to the including file. There is no way to remove/ignore existing sections/properties.

"A" in section "1", and the included file has a section "1" with only a setting "B"

You will get

Code:

[1]
A = ...
B = ...

In the LE.car the engine 1zz-fe is included. Engine position is defined/added in LE.car.

You could create a LE1.car by including LE.car and overriding the transmission settings e.g.

Conceptual issue:
If you include the same section twice(the engine) you will end up with a union of both sections.

[thelusiv]

Ah, ok, I missed this part in my first read-through:

Code:

else if (include.find(fname) != include.end())
        {
                return true;
        }
        include.insert(fname);

...but I'm still unclear on how this doesn't overwrite the contents of a previously loaded section:

Code:

iterator section = sections.insert(std::pair<std::string, SECTION>("", SECTION())).first;

What if sections[""] was already there, and had values? It looks to me like it would be overwritten by an empty SECTION object.

Shouldn't it be something like this:

Code:

iterator section = sections.find("");
if (section == sections.end())
{
        section = sections.insert(std::pair<std::string, SECTION>("", SECTION())).first;
}

It would certainly help my understanding if there was some test code that tested includes, or more comments describing how the include system works.

[NaN]

I've added a unit test.

I'm still unclear on how this doesn't overwrite the contents of a previously loaded section:

The funny thing about insert() is that it won't replace an already existing key, value.

[thelusiv]

Thanks! The test looks good and it passes on my machine.

I wasn't aware of that behavior for std::map.insert(). That makes sense though, I get it now, thanks for clearing it up for me. Now that I understand it better, I will add some more details to the wiki page.

edit: I spoke too soon...the test passes even if the test files aren't present. Once I did get the test files in place, and added checks to make sure they load, then the test passed fine and everything seems in order.

I added some details here, please confirm that this is accurate:
http://wiki.vdrift.net/Configfile_format...directives

[NaN]

Yeah, I didn't check for file load success.

By the way, I've found a bug in config code. Exposing iterators wasn't my brightest idea as there is no way to check if they are valid. Will have switch back to string only access even if it means a speed penalty.

[thelusiv]

Are CONFIG iterators being used in the game code significantly yet?

[NaN]

I've added some asserts and made sure that the iterators are initialized properly.

On a second thought the whole iterator issue is not that serious. I mean the stl is full of them. And using an uninitialized or foreign iterator on a container is a really bad idea anyway.

[thelusiv]

I noticed that the test config files you added do not end with newlines. I added a check for this and it passes fine. I guess getline will happily substitute EOF for a delimiter.

It seems like there are no conditions where the funciton bool CONFIG::Load(std::istream & f) will return false. Should it complain about 0-length input, perhaps?

To do much more checking than that, we'd have to make ProcessLine return false when it is unhappy about something. There isn't any error checking in ProcessLine. I know that the class is intended to be hard to break and take almost any input, and I can't think of many ways in which it should produce an error. Maybe it should error when an include fails to load? Or maybe it should log a warning and move on...?

[NaN]

I reused existing code, didn't want to spend too much time on it. Feel free to extend/add more checks/error output.

[thelusiv]

I checked in some changes which include more error checking. Some thoughts:

• came up with some error cases for sections and subsections but they turned out to be used in the game in different ways. they are commented out (config.cpp:186-213). i think that some of these things should be at least warnings, since users probably won't usually intend to duplicate a section, or other things like that.
  
• it would be a pain to get loggers in CONFIG, since CONFIG objects are created in many different places (and owner objects don't always have loggers either), so i am going to work on that later (or maybe try to write a script to do it)
  
• on sections, subsections, etc.: there is a way to find the children of a section (childrens' names are stored on config.cpp:201), but not to find a given section's parents. why store this hierarchy information, is it used anywhere?
  
• would it make any sense to make SECTION a custom class instead of just a map<string, string>? this would make it easier to facilitate things like finding parents (and siblings, and root nodes, etc.) to support a proper hierarchy. if this was implemented, it wouldn't be difficult to refer to other values stored in the file by their names. (would that be useful?)

[joevenzon]

[*]it would be a pain to get loggers in CONFIG, since CONFIG objects are created in many different places (and owner objects don't always have loggers either), so i am going to work on that later (or maybe try to write a script to do it)

No statics or globals please. In other places we've used standard ostreams passed in as an argument for logging.

[NaN]

his would make it easier to facilitate things like finding parents (and siblings, and root nodes, etc.) to support a proper hierarchy

If you need this functionality implement it.

The config file is a on demand implementation. I needed certain features. Access sections without having to iterate over them -> map. Access parameters without having to iterate over them -> map. Need to reference sections, users complained -> add subsections(hide the references from user, still use them internally).

By the way, I also would have no problems with an xml, json or symbolic expression parser as long as the users are happy.