Wrongly placed InEarth objects when main material is set with texture

[Remus76]

The documentation and source code allows setting C4S.Landscape.Material to a material/texture combination separated by a '-'. When this is done, e.g. Material=Sand-Smooth, the variable MEarth is not set correctly in C4Game::EnumerateMaterials. So it has a value of -1 and as a result in C4Game::PlaceInEarth InEarth objects are placed in the sky which is equivalent to material index -1.

System and configuration: not relevant as it happens ingame

Steps to reproduce:

• create a scenario and set Material=Sand-Smooth in the Landscape section of Scenario.txt be sure to use the (older) dynamic landscape and no Map.bmp or Landscape.txt
• set up some InEarth objects in the Landscape section of Scenario.txt
• start the scenario and see that they are placed wrongly

Actual results: InEarth objects are placed in the sky.

Expected results: InEarth objects should be placed in the set main material, in this case in Sand. In CE and older versions InEarth objects are placed only in Earth material but this has changed since CR including this issue.

Notes: Setting a material name only, e.g. Material=Sand, is a workaround but then you have to take care about the texture table entries. The code leading to this issue is https://github.com/legacyclonk/LegacyClonk/blob/master/src/C4Game.cpp#L1647. According to how this is done in the following four examples https://github.com/legacyclonk/LegacyClonk/blob/master/src/C4Landscape.cpp#L2745-L2746 https://github.com/legacyclonk/LegacyClonk/blob/master/src/C4Landscape.cpp#L2778-L2779 https://github.com/legacyclonk/LegacyClonk/blob/master/src/C4Texture.cpp#L210-L212 https://github.com/legacyclonk/LegacyClonk/blob/master/src/C4Texture.cpp#L356_L358 it can be fixed using something like

StdStrBuf defaultmat;
defaultmat.CopyUntil(C4S.Landscape.Material, '-');
MEarth = Material.Get(defaultmat.getData());

[maxmitti]

Thanks for the detailed analysis.

A short discussion lead to the following conclusion:

We don’t want to break old scenarios that rely on the broken behavior. A possible solution could consist of two parts:

• Log a warning when the broken behavior is triggered.
• Introduce a new replacement parameter (e.g. BaseMaterial) which works as expected.

[Remus76]

I understand the idea behind not breaking existing things so the first suggestion should be ok. As for the second one I'd prefer a switch instead of a replacement because C4S.Landscape.Material is already used correctly (including the texture) when creating a new dynamic landscape. So using this approach the possibility of breakage will be even less. You can take a look at the ForceInEarth property at my basematerial branch. (outdated as resolved)

[maxmitti]

Good reasoning for not duplicating.

I like your idea. However, I would prefer a more self explanatory name. It is not a good suggestion but for example “InEarthMaterialTextureFix”. It should somehow convey the relation between the two properties.