[DONE][Suggestion] Terrain command as an array

[Solarius Scorch]

My request is to enable using the "terrain" command in mapScripts as an array, so a random terrain is chosen from a list.

So for example (not a real example, just for illustration), you can now do this:

Code: [Select]

  - type: JUNGLE
    commands:
    - type: addUFO
    - type: addCraft
    - type: addBlock
      terrain: URBAN
    - type: fillArea

So it generates a desert map with a single random block from URBAN.

What I would like is doing this:

Code: [Select]

  - type: JUNGLE
    commands:
    - type: addUFO
    - type: addCraft
    - type: addBlock
      terrain: [URBAN, FOREST, DESERT, MOUNT]
    - type: fillArea

So it generates a desert map with a single random block from one of these four terrains, selected at random.

The reason behind this is rather complex, it has to do with the new urban terrain we're building with Finnik which depends on a lot of "subterrains" called with the terrain command.

Thank you in advance for considering it

[Finnik]

I am supporting it as otherwise we need much more code with a lot of conditionals. And it becomes much harder to manage  if you already have lots of them, if you use masks and block replacements with checkBlock/removeBlock/ addBlock combination all over the big map

[Meridian]

I have nothing against it.

You can try to implement and test it yourself as with the addLine feature, or wait a week or two until I do it.

Is the RNG applied only once per script or is it rolled per each command? that was a stupid question

[Solarius Scorch]

Is the RNG applied only once per script or is it rolled per each command?

Per each command, as it gives more freedom.
The idea is to combine multiple terrains into one, not just mix two different terrains.

[Finnik]

Committed! https://github.com/MeridianOXC/OpenXcom/commit/a81e8b641d51b2c182b9f38ebf2582927ba41833
that `_terrain("")` you have in constructor confused me =)
I'm a bit warry that we will have different syntax in mapScripts (terrain:) and alienDeployments (terrains:), but processing the same way in my branch, could be confusing. But changing that would brake mods.
Anyway, sample ruleset I can process:

Code: [Select]

mapScripts:
  - type: URBAN_TEST
    commands:
    - type: addCraft
    - type: addBlock
      terrain: URBAN
      executions: 2
    - type: addBlock
      terrain: [FOREST, MOUNT, JUNGLE]
      executions: 10
    - type: fillArea


[Meridian]

1/ You can't just change the existing syntax, some mods use it already and will break... it will need to stay backwards-compatible supporting both single string and array of strings

2/ the test case in TestState needs to check all terrains, not just one

I'll fix both tomorrow and merge.

[Finnik]

1/ You can't just change the existing syntax, some mods use it already and will break... it will need to stay backwards-compatible supporting both single string and array of strings

Yes, ofc I can understand that. The existing syntax for modder was not changed, so it will have all backward-compatible. I just thought it is illogical now to alienDeployments, but I think we can live with it

2/ the test case in TestState needs to check all terrains, not just one

Sorry, my bad. Thank you for reviewing my code!

[Meridian]

Yes, ofc I can understand that. The existing syntax for modder was not changed, so it will have all backward-compatible. I just thought it is illogical now to alienDeployments, but I think we can live with it

Not sure what you mean. The syntax has changed, and is not backwards compatible in your commit.

The old syntax:

Code: [Select]

    - type: addBlock
      terrain: FOREST
will crash the game.

I will leave "terrain" as it was and add a new attribute "randomTerrain" for the new syntax.

[Finnik]

Ok, my bad 
It was not crashing, but just skipping the command. I need to test things much more carefully. Let me redo it properly.

[Finnik]

ok, i revented commit and made a new one.

https://github.com/FinnikXCF/OpenXcom/commit/af5b8b5f90033b5ecbdc5487925f0d9bb60b300b

now we are using

Code: [Select]

mapScripts:
  - type: URBAN_TEST
    commands:
    - type: addCraft
    - type: addBlock
      terrain: DESERT
      executions: 4
    - type: addBlock
      randomTerrains: [MARS, MOUNT]
      executions: 4
    - type: fillArea
      randomTerrains: [URBAN, FOREST]

I tested it with addLine also, looks ok.
Also, i tried to make changes into TestState, I hope i did it right, but I'm not sure, it is strange to me (I'm a noob in c++).
@Meridian, please, let me know if something needs to be corrected

[Yankes]

It look fine, btw your previews commit flaw was:

Code: [Select]

_terrain = node["terrain"].as<std::vector<std::string>>(_terrain);
but this could be fixed, by:

Code: [Select]

if (const auto& terranNode = node["terrain"])
{
  if (terranNode.isScalar())
  {
      _terrain = std::vector{ terranNode.as<std::string>(), };
  }
  else
  {
     _terrain  = terranNode.as<std::vector<std::string>>(_terrain);
  }
}



[Finnik]

Thank you! I just thought, that is affecting Otto's verticalLevels with it, and I'm glad Meridian choose a separate property for it, much more clear for me.
But I will remember, its handy. I need to dive more to methods we have about checking and converting data types.

[Meridian]

@Meridian, please, let me know if something needs to be corrected

I don't know how will it behave with oharty's vertical terrain... I'll check soon-ish.

[Finnik]

in the current commit - there is no intersections with it, verticalLevels do not use randomTerrains, and i didnt changed anything about methods it uses

[Meridian]

Code: [Select]

mapScripts:
  - type: URBAN_TEST
    commands:
    - type: addCraft
    - type: addBlock
      terrain: DESERT
      executions: 4
    - type: addBlock
      randomTerrains: [MARS, MOUNT]
      executions: 4
    - type: fillArea
      randomTerrains: [URBAN, FOREST]


Looking at your screenshots, I noticed that you roll a new terrain for each execution (of the SAME command).

For example this:

Code: [Select]

    - type: addBlock
      randomTerrains: [MARS, MOUNT]
      executions: 4
could generate a mix let's say 3x MARS and 1x MOUNT ... instead of 4x MARS or 4x MOUNT.

Is that intended and desired?
I would expect that RNG is rolled only once for the entire command (not entire map script, just entire command)?