"Modify" directive fix

[Ajaja]

There is currently a bug with the "Modify" directive.

Create, for example, test.ssc in extras\test with:

Modify "Moon:Earth I" "Sol/Earth"
{
    InfoURL "https://en.wikipedia.org/wiki/Test"
}

Load Celestia and go to the Moon - it appears completely flat because of the unconditional:

SetOrUnset(surface->appearanceFlags, Surface::ApplyBumpMap, (bumpTexture.has_value() || normalTexture.has_value()));

test.ssc does not define normalTexture or bumpTexture, so this parameter is unset.

I'm not sure this is the best way to fix the problem. This PR is just an example of what the fix should look like.

[ajtribick]

I don't think this approach works, because it removes the ability to re-set a boolean property (which is what the original implementation in #1352 was trying to address).

A better approach would be to consider whether the property was actually present in the file. So for the boolean properties we could do:

if (auto blendTexture = surfaceData->getBoolean("BlendTexture"); blendTexture.has_value())
    SetOrUnset(surface->appearanceFlags, Surface::BlendTexture, *blendTexture);

The string properties would need to be similar, presumably we can treat an empty path as a reset? (This would also require editing the GetFilename function further up the file, as currently we map empty paths to std::nullopt.

if (auto baseTexture = GetFilename(*surfaceData, "Texture"sv, "Invalid filename in Texture\n");
    baseTexture.has_value())
{
    SetOrUnset(surface->appearanceFlags, Surface::ApplyBaseTexture, !baseTexture->empty());
    surface->baseTexture = texturePaths.getHandle(*baseTexture, path, baseFlags);
}

The Surface::SpecularReflection property handling doesn't need to change: we already only update the specularColor property if the value is present: https://github.com/CelestiaProject/Celestia/blob/master/src/celengine/solarsys.cpp#L207-L208

The annoying one to deal with will be BumpHeight, as this affects the texture mapping and isn't stored directly. My suspicion is that we need to convert texture paths to handles after all files have been processed, but that's going to be a much larger reorganization of the code.

[Ajaja]

I suspected that a proper fix might not be so simple. The idea of using an empty path as a reset is also logical.

[AstroChara]

Setting this PR to draft for the reasons ajtribick stated and Ajaja (PR author) agreed.