Wercator Weturns! (v. 0.6.3; Web interface back)

[Quensul]

martenzo: yacc is a parser-generator that I use to handle config files. On Ubuntu, a yacc-equivalent is provided by the 'bison' package. In theory, 'apt-get install bison' should do the trick. I had assumed it was part of the standard install, but I guess I'm wrong. yacc/bison generates C files from a parser description; in future releases, I'll just include the generated files. Let me know if you have any more trouble -- if you do, I'll rerelease tonight with the generated files included.

Shadow Master: Thanks for the glue.

[Iris]

martenzo: yacc is a parser-generator that I use to handle config files. On Ubuntu, a yacc-equivalent is provided by the 'bison' package. In theory, 'apt-get install bison' should do the trick. I had assumed it was part of the standard install, but I guess I'm wrong. yacc/bison generates C files from a parser description; in future releases, I'll just include the generated files. Let me know if you have any more trouble -- if you do, I'll rerelease tonight with the generated files included.

Shadow Master: Thanks for the glue.

1) Oh well, all that means my openSUSE laptop and desktop, both can handle it properly, as I did install bison (for compiling DOSEmu at some point IIRC). BTW, I didn't know bison could completely replace yacc; I suppose installing it creates a symbolic link at /usr/bin/yacc to /usr/bin/bison-(version).

2) It was jelly, not glue. You are welcome anyway.

[Quensul]

In retrospect, it was rather silly of me to not include the generated files -- this was the sort of problem I was hoping to catch with 0.5.

So, without further ado, here's Wercator 0.6.1! The only change is to add those files. It should no longer depend on yacc, bison, flex, or lex. Please let me know of any further installation problems.

BTW, as far as I know, bison is close enough to yacc for most purposes, and in fact on Ubuntu it does exactly as you describe -- /usr/bin/yacc points to bison.

[Iris]

I missed the 0.6.1 release, and can't test it today. But this is what I wrote at home, internet-less:

---------

Well Quensul, it seems you missed a small detail. Or quite a few. I got this error when compiling:

Chapter 1.

Code: Select all

WercMapFormat.cc: In function ‘WERC_MAP_FORMAT formatStringToEnum(std::string)’:
WercMapFormat.cc:14: error: ‘assert’ was not declared in this scope
WercMapFormat.cc: In function ‘std::string formatEnumToString(WERC_MAP_FORMAT)’:
WercMapFormat.cc:24: error: ‘assert’ was not declared in this scope

Stopping the build immediately. Fixed by adding the following line to /src/wercSlicer/WercMapFormat.h:

Code: Select all

#include <cassert>

I suppose it might be caused by my version of GCC and G++ (4.1) and/or the Standard C++ Library being more strict in their header inclusion sequence. In any case, I recommend putting that line there for the sake of compatibility. It would not hurt in any compiler that follows the common specifications.

Chapter 2.
Continuing the make process after fixing that flaw, I got this:

Code: Select all

In file included from WercRenderPathDeformation.cc:1:
WercRenderPathDeformation.h:93: error: extra qualification ‘WercRenderPathDeformation::’ on member ‘peek

That happens because GCC 4.1 adds some extra rules, that were in the standards, but never enforced before, to C++ constructs; I already ran across this issue with other programs, and the solution is to replace:

Code: Select all

pair<double, double> WercRenderPathDeformation::peek(double maxXOffset,
                                                       int* pointIOut = NULL,
                                                       double* posXOut = NULL,
                                                       int* repeatCountOut = NULL) const;

At line 90 of /src/wercRenderBase/WercRenderPathDeformation.h, with:

Code: Select all

pair<double, double> peek(double maxXOffset,
                                                  int* pointIOut = NULL,
                                                  double* posXOut = NULL,
                                                  int* repeatCountOut = NULL) const;

Which is the same thing without the extra qualifier. Note that this only applies to class member declarations, not the definitions (i.e. the actual code in the .cc files). Although this is a weird rule of GCC 4.1, let it be known that most C++ language references nowadays teach this syntax; I recall reading about extra qualifiers regarded as unnecessary redundant syntax. They have no effects and may even confuse the linker in some rare cases. Now, this syntax will work with GCC 3.x and later (possibly even the earlier, I don't know) and MS C++ compiler and linker versions 7.0 and later, as far as I know. I guess it works with earlier versions too, and with other very known compilers. But for the sake of compatibility, I recommend listening to GCC 4.1's advice.

---

After addressing these two issues, I could successfully compile Wercator 0.6 for the GIMP 2.2.17, on openSUSE 10.3 x86_64.
BTW, I'm not sure if this has anything to do with Bison bugs.

Chapter 3. The bugs with 1.3.10 map format

wesnoth_editor and wesnoth don't enforce a specific order for header lines, nor does wmllint. So my maps, that have already been tested ingame and with the editor, have usually this order:

Code: Select all

border_size=1
usage=map

<the terrain map starts here>

Which is not recognized by Wercator as a valid 1.3.10 format. It says "first line must be usage=[map|mask]". That's not good. If I revert the order of lines to what Wercator demands, they still work in wesnoth_editor and the actual game, and Wercator gets happy. However, since that is not the order generated by the map editor, and as theoretically they could be in any order as long as they are the first two lines, I recommend you to modify that behavior so that the line order does not matter, as long as they are the first two lines, again.

Chapter 4. The general bugs in x86_64 platform
After addressing that by hacking the map by hand, Wercator shows its progress bar and crashes about 20% (from what I get to see, it fades away really fast) completed. That is with the default Wercator 0.6 settings, and happens with both 1.3.9- map format and 1.3.10. I would like to know how to make the GIMP output more helpful info other than the path to the failed plugin's library.

Chapter 5. In the x86 (i386) platform
The crash mentioned above doesn't happen in a build for openSUSE 10.3 x86, the one of my desktop computer. And I could make quite a few maps of my campaign successfully after 'porting' them so that Wercator would recognize them. However, when using the border option with the default, Wercator crashes right when it's about to process it, after making the map background, outlines of terrains, etc.

---

Finally, it's all a great job, I wish it worked in openSUSE 10.3 x86_64, but it's fine as long as it works in x86 at home.

Cheers.

[Quensul]

Thanks for spending the time to fix the various issues -- my apologies; I thought I had written clean code. Comments interspersed:

I missed the 0.6.1 release, and can't test it today.

None of your problems appear to be Bison-related, so I doubt 0.6.1 would make any difference.

Code: Select all

WercMapFormat.cc: In function ‘WERC_MAP_FORMAT formatStringToEnum(std::string)’:
WercMapFormat.cc:14: error: ‘assert’ was not declared in this scope
WercMapFormat.cc: In function ‘std::string formatEnumToString(WERC_MAP_FORMAT)’:
WercMapFormat.cc:24: error: ‘assert’ was not declared in this scope

Huh. I have no idea how gcc 4.0.3 was able to compile that; assert.h/cassert wasn't anywhere in WercMapFormat's include tree. That's my bad, though -- when I add asserts during debugging, I sometimes forget to add the header. Patched.

Continuing the make process after fixing that flaw, I got this:

Code: Select all

In file included from WercRenderPathDeformation.cc:1:
WercRenderPathDeformation.h:93: error: extra qualification ‘WercRenderPathDeformation::’ on member ‘peek

Oops. When extending a class, I generally write the function definiton first, then copy the prototype up into the header; looks like I forgot to nuke the extra qualifier here, and 4.0.3 didn't complain. Definitely not intentional. Thanks; patched.

wesnoth_editor and wesnoth don't enforce a specific order for header lines, nor does wmllint.

Ah, I didn't realize that. All the examples on the Wiki and elsewhere have usage first; I assumed the ordering was part of the spec, but never looked through the source to verify. Both keys must be present, correct? Or may they be omitted? Assuming both must be present, replace wercSlicer/WercMap.cc:_readMapFromFile1310(...) with the following:

Code: Select all

bool WercMap::_readMapFromFile1310(ifstream& mapFile, string& errStr) {
  string line;
  vector<string> headerLines;
  int i;
  bool haveUsage = false;
  bool haveBorder = false;

  // Read in map header
  for(i = 0; i < 3; i++) {
    getline(mapFile, line);
    headerLines.push_back(line);
  }
  
  // Check the header to make sure it's *actually* a 1.3.10 header.
  for(vector<string>::iterator hlit = headerLines.begin();
      hlit != headerLines.end();
      hlit++) {
    // Skip whitespace
    for(i = 0; isspace((*hlit).c_str()[i]); i++) {}
    // Check against valid header lines
    if(strncasecmp((*hlit).c_str() + i, "usage=", strlen("usage=")) == 0)
      haveUsage = true;
    else if(strncasecmp((*hlit).c_str() + i, "border_size=", strlen("border_size=")) == 0)
      haveBorder = true;
  }

  errStr = "";

  if(!haveUsage)
    errStr += "A 1.3.10+ map must include a 'usage=[map|mask]' line in its header.\n";

  if(!haveBorder)
    errStr += "A 1.3.10+ map must include a 'border_size=[map|mask]' line in its header.\n";

  if(!haveUsage || !haveBorder)
    return false;

  // From there on, it's the same as 1.3.0-1.3.9
  return _readMapFromFile130to139(mapFile, errStr);
}

Sorry for the bother!

As for crashing bugs: blast. It hasn't crashed on me in quite a long time. I'll add a verbose mode tonight and release 0.6.2. However, if you could get me a backtrace, that would be extremely helpful. Gimp actually runs plugins as a separate process, which means we can play fun games with GDB. The details are here; the short version is to do the following (assuming csh/tcsh):

Code: Select all

terminal1> setenv GIMP_PLUGIN_DEBUG wercator_gimp_plugin,run
terminal1> gimp&
terminal2> gdb /usr/lib/gimp/2.0/plug-ins/wercator_gimp_plugin
gimpui> Run Wercator.  Wercator's PID will be printed out in terminal1.
terminal2> attach <pid>
terminal1> kill -CONT <pid>
terminal2> continue
terminal2> continue

At which point Wercator will be running under gdb. A pain, I know. Once it crashes, I trust you know how to poke around. A backtrace and any other information you deem relevant would be quite useful.

You could also try rebuilding with optimization disabled to see if that's biting us: uncomment the 'CXXFLAGS=-g' line in configure.in, then autoreconf; configure; make; sudo make install.

Thanks for the feedback; here's hoping that we can iron these problems out in short order.

[Iris]

I'll try debugging this weekend; real life and some work in svn trunk keep me busy right now.

Thanks for the patch, and no problem. It was fun while it lasted (the bug fixing).

I asked Mordante about the map format:

[13:26] <Shadow_Master> Mordante: in the new map format, are both "usage=" and "border_size=" mandatory, or not? (For Wercator's parser, this is)
[13:26] <Shadow_Master> I suppose at least one of them ought to be present
[13:27] <Mordante> Shadow_Master usage is mandatory, border_size not but set to 0 if omitted
[13:27] <Shadow_Master> Mordante: alright

Here you are. I would go and update the wiki, but it is currently out of order.

BTW, I might practice making an openSUSE 10.3 x86 and x86_64 RPM, the later if the big bug is fixed - I am suspicious on it being caused by openSUSE's gimp-devel package.

[Quensul]

Wercator 0.6.2 is now out. It incorporates no new functionality, but it does:

• * Patch all the bugs mentioned above (except the yet-to-be-diagnosed crashes).
  * Add a '--disable-optimization' configure flag to, well, disable optimization so we can see if that's the culprit.
  * Add a debug level option in the 'Config' tab to change the level of debug info that's printed to the terminal. At the moment, almost everything lives at DEBUG3. If you're having trouble, set this to DEBUG4 and send me the output.

Cheers!

[martenzo]

Compiled fine with default settings under Debian Sid. Haven't tested if it actually works, though.

EDIT: Renders the map, but crashes in the end and removes the parchment. Also, the parchment isn't large enough for the map to begin with, it's about 700 (estimate, measured by eye, not an actual measurement) pixels too short, leaving the left edge of the image on white background. I believe the reason for both is that my map is to wide.

EDIT2: Increased parchment size by 1.5x and the parchment beneath the image looked good. But it still crashed and turned the background transparent. Error messages:

Code: Select all

First error message:
Procedure 'gimp-image-set-active-layer' has been called with an invalid ID for argument 'active-layer'. Most likely a plug-in is trying to work on a layer that doesn't exist any longer.
And the second one:
Procedure 'script-fu-drop-shadow' has been called with an invalid ID for argument 'drawable'. Most likely a plug-in is trying to work on a layer that doesn't exist any longer.

[Quensul]

Hmm. By any chance is your map wider than 128-130 hexes or taller than 164-166 (which end of the range depends on the border settings)? None of my test maps are that big, so I missed this. I'll patch it now[1]; as a workaround, you could force the size to <= the size of the parchment image (about 2544x3252, although it varies a bit) with the 'brush placement' method.

If you could send me the map that's causing trouble, I'd like to see if I can duplicate the second crash here. Also, if you have time to grab 0.6.2, running it with the debug level maxed could provide useful information.

Thanks!

[1] In the Perl version, Wercator generated its own parchment for really big images, but that always takes forever, and never turns out as well as scanned textures, so I removed it.

[Quensul]

Curiouser and curiouser. What should have happened is that the parchment should have been stretched to fit, or, failing that, an error message should have popped up in a dialog. That's the behavior I see with an oversized map on my machine. If you could post or PM the troublesome map, I'd be grateful.

Thanks!

[martenzo]

Oh, heh, no worries, I manually streched the parchment. And the crash with the parchment layer getting deleted happens regardless of whether the map fits on the parchment or not. And the map is 153x70 hexes.

Also, I'm using Gimp 2.4.1

And the map is too big to add as an attachment so: Clickie

[Quensul]

Thanks; I'll have a look at the map. My first reaction would be that it's a Gimp 2.4 issue; I'm developing with 2.2. I'll install a copy of 2.4 over the weekend and see if I can track things down. 2.4 is supposed to fully support the 2.2 plugin API, but I'm taking that with a (large) grain of salt.

[Quensul]

Wercator 0.6.3 is out. It makes some changes to handling large maps that might fix martenzo's problems (although I haven't been able to test with Gimp 2.4 yet), cleans up some logging, and switches the Gimp frontend to using all gint32's instead of int's -- there's a chance this will fix your 64-bit problems, Shadow Master.

[Iris]

It didn't fix the issue in the AMD64, but it works fine in the 32-bit Pentium 4 at home. I have rendered quite a few maps that I have to test later.

To note however: the rendering algorithm is quirky at times. Complex watery layouts screw up like if the water stream had "lobes". Quite annoying.

Aside from that, it's quite excellent. (Tested with GIMP 2.2.13, openSUSE 10.3 x86, self-built kernel).

[Quensul]

To note however: the rendering algorithm is quirky at times. Complex watery layouts screw up like if the water stream had "lobes". Quite annoying.

Agreed -- tracing streams like that in an aesthetically pleasing way is tricky. Given that the underlying map has lobes (correct me if that's wrong, Shadow Master), what do people think Wercator should do? The options seem to be:

• 1. Be true to the map, and accept sometimes-funky-looking water.
  2. Add special-case code to cut off such one- or two-hex long inlets.

The current behavior is (obviously) (1); the issue with (2) of course is that sometimes you want to draw these sorts of features. I suppose it could be (yet another) option, but Wercator already has a ton of those.

The tracing code also tries to not draw over other terrain features; if the water has a one-hex border of 'blank' tiles (e.g. grass, sand, or anything else that isn't drawn explicitly), there's more room for the algorithm to smooth out corners and avoid drawing bottlenecks. Unfortunately, as soon as 'interesting' terrain (such as the mountains below) is placed next to water, the tracing code becomes much more constrained.