Re: [Wesnoth-dev] KISS Solution to user campaign stability problems

[ott]

On Fri, Jun 10, 2005 at 08:16:53PM -0500, David White wrote:
> The solution is to remove the {~data/campaigns} reference in the 
> game.cfg file. Instead, the game now reads data/game.cfg and then reads 
> each file in the pattern ~data/campaigns/*.cfg, and appends them to the 
> config object.

Unfortunately, this has completely broken user campaigns.  The problem
seems to be that none of the standard macros are available to user
campaigns.  As an example, MENU_IMG_TXT and MENU_IMG_TXT2 are not
available and hence no difficulty levels can be chosen.

Since 0.9.2 we also seem to have made the parser very fussy about missing
newlines at the end of file, at least when parsing the main campaign .cfg
file -- if the last line is not newline terminated it will apparently
be ignored, and since the last line is often #endif this is fatal for
many campaigns.  See my overview of the campaign server downloads at
    http://www.wesnoth.org/forum/viewtopic.php?t=6463
where many of the campaigns will at least pass the basic syntax check
if they get a final newline.

Some campaigns are borderline playable, but lack time of day images and a
day-night cycle (Flight to Freedom 2.9.1).  Others go straight to The End
(Under the Burning Suns 0.4.3 5/30, after the missing newline is fixed),
or determine a loss after the end of the first turn (Liberty 1.12).
It seems to me this correlates well with missing macros, although the
--log-info=all debug messages are not very helpful.  The one clear message
is the consistent attempt to stream the MENU_IMG_TXT and MENU_IMG_TXT2
macros as if they were files for reading, a clear indication that they
are not being treated as macros.

Finally, this change seems to have removed the helpful errors that were
generated with 0.9.2 when parsing the .cfg files, which often helped
to isolate the problem.  Now all we get is that the campaign is broken,
with very little else to go on.

I like the general idea of the change, but the details need further work
before this can be unleashed.

Silene proposed a quick fix on IRC before dashing off for a few days
(see the IRC logs for yesterday), but my implementation of this idea
doesn't seem to have achieved the desired result.  Here is the patch
I've been working with.

-- address@hidden

Index: src/game.cpp
===================================================================
RCS file: /cvsroot/wesnoth/wesnoth/src/game.cpp,v
retrieving revision 1.254
diff -u -b -r1.254 game.cpp
--- src/game.cpp        11 Jun 2005 12:47:09 -0000      1.254
+++ src/game.cpp        23 Jun 2005 18:47:52 -0000
@@ -1275,7 +1275,8 @@
                                std::cerr << "no valid cache found. Writing 
cache to '" << fname << "'\n";
 
                                //read the file and then write to the cache
-                               scoped_istream stream = 
preprocess_file("data/game.cfg", &defines);
+                               preproc_map tmp_defines(defines);
+                               scoped_istream stream = 
preprocess_file("data/game.cfg", &tmp_defines);
 
                                std::string error_log;
                                read(cfg, *stream, &error_log);
@@ -1286,7 +1287,8 @@
                                
get_files_in_dir(user_campaign_dir,&user_campaigns,NULL,ENTIRE_FILE_PATH);
                                for(std::vector<std::string>::const_iterator uc 
= user_campaigns.begin(); uc != user_campaigns.end(); ++uc) {
                                        try {
-                                               scoped_istream stream = 
preprocess_file(*uc,&defines);
+                                               preproc_map 
tmp_defines(defines);
+                                               scoped_istream stream = 
preprocess_file(*uc,&tmp_defines);
 
                                                config user_campaign_cfg;
                                                
read(user_campaign_cfg,*stream,NULL);
@@ -1336,7 +1338,8 @@
        }
 
        std::cerr << "caching cannot be done. Reading file\n";
-       scoped_istream stream = preprocess_file("data/game.cfg", &defines);
+       preproc_map tmp_defines(defines);
+       scoped_istream stream = preprocess_file("data/game.cfg", &tmp_defines);
        read(cfg, *stream);
 }
 
Index: src/serialization/preprocessor.cpp
===================================================================
RCS file: /cvsroot/wesnoth/wesnoth/src/serialization/preprocessor.cpp,v
retrieving revision 1.20
diff -u -b -r1.20 preprocessor.cpp
--- src/serialization/preprocessor.cpp  4 Jun 2005 19:16:06 -0000       1.20
+++ src/serialization/preprocessor.cpp  23 Jun 2005 18:47:53 -0000
@@ -710,18 +710,17 @@
 struct preprocessor_deleter: std::basic_istream<char>
 {
        preprocessor_streambuf *buf_;
-       preprocessor_deleter(preprocessor_streambuf *buf) : 
std::basic_istream<char>(buf), buf_(buf) {}
-       ~preprocessor_deleter() { rdbuf(NULL); delete buf_->defines_; delete 
buf_; }
+       bool delete_defines_;
+       preprocessor_deleter(preprocessor_streambuf *buf, bool delete_defines) 
: std::basic_istream<char>(buf), buf_(buf), delete_defines_(delete_defines) {}
+       ~preprocessor_deleter() { rdbuf(NULL); if (delete_defines_) delete 
buf_->defines_; delete buf_; }
 };
 
 std::istream *preprocess_file(std::string const &fname,
                               preproc_map const *defines)
 {
        log_scope("preprocessing file...");
-       preproc_map *defines_copy = new preproc_map;
-       if (defines)
-               *defines_copy = *defines;
+       preproc_map *defines_copy = defines ? defines : (new preproc_map);
        preprocessor_streambuf *buf = new preprocessor_streambuf(defines_copy);
        new preprocessor_file(*buf, fname);
-       return new preprocessor_deleter(buf);
+       return new preprocessor_deleter(buf, defines == NULL);
 }