Segmentation fault when running terraingenerator if Terrain Image is not set

[bluecarrot16]

Describe the bug
I get a segmentation fault when running the terraingenerator tool. Seems the cause is that one or more terrains in the tileset don't have a Terrain Image assigned.

To Reproduce
Steps to reproduce the behavior:

1. Download and unzip
   terraingenerator-bug.zip
2. Navigate to the directory containing these files and run this script:

# replace with path to terraingenerator executable
TERRAINGENERATOR=/Applications/Tiled.app/Contents/MacOS/terraingenerator
$TERRAINGENERATOR --overwrite -o terrain-map-v10.tsx -s terrain-v10.tsx -c Water Dirt_Tan Sand -p Sand Dirt_Tan Water

3. Result: Segmentation fault: 11

Expected behavior
An error message saying something to the effect of "Must assign a Terrain Image for Terrain '$TERRAIN'. Open the tileset in Tiled, select the terrain, then right-click on a tile and choose 'Use Terrain Image'"

Setting a terrain image for all tiles in the tileset fixes the problem.

Specifications:

• OS: macOS 10.15.7
• Tiled Version: 1.8.2

Thanks for the great tool!

[eishiya]

Terrain images are entirely optional, so if that is indeed the problem, showing an error message would not be a good solution.

There's already a check for whether the Terrain image is set (https://github.com/mapeditor/tiled/blob/master/src/terraingenerator/main.cpp#L559-L560), too...

[bjorn]

There's already a check for whether the Terrain image is set

Right, a static analysis tool would have caught this error, because after checking that pointer for null it is used two lines later without the null check... so this bug was introduced in 02e092a, where I forgot about the null check.

Thanks for the report @bluecarrot16!

[bjorn]

After fixing the crash, I noticed this still doesn't work nicely with your use-case, since the code assumed that those images were each terrain's "base image", so it would result in a black tile for each of the listed terrains. I've now fixed up the code so that it also works as expected when the terrain images are not set.

[bluecarrot16]

Thanks for the quick response and the fix!

A related issue---if there are multiple "base images" for a given terrain (e.g. multiple tiles where all four corners have the same terrain type), only the tile marked as the "Terrain Image" would be included in the output. It would be nice if there was a mechanism for the other images to be included as well. The use case is for "accent" tiles (for example, tiles (0,5), (1,5), (2,5) in the example above) to be copied over into the assembled image. (Bonus points if the tile probability is also copied!)

I can't tell immediately from the code whether your change implements this behavior. If it's not hard to do while you're thinking about this, that change would also be much appreciated! I can also make a separate issue for that if you prefer.

[bjorn]

@bluecarrot16 I guess it makes more sense to set up a new issue about that. The thing is that the tool currently doesn't support variation tiles at all, and I'm not too fond of doing a quick implementation that just deals with the full terrain tiles currently. Also, note that the tool can extend an existing tileset (if you don't pass --overwrite), so you can manually add those variation tiles once. Hence, the need for this tool to copy those over should be somewhat low.

[bluecarrot16]

OK, thanks for that! I'll make a new issue for that request.