Author Topic: [DONE][Suggestion] Terrain comand as an array  (Read 1662 times)

Online Solarius Scorch

  • Global Moderator
  • Commander
  • *****
  • Posts: 9848
  • WE MUST DISSENT
    • View Profile
    • Nocturmal Productions modding studio website
[DONE][Suggestion] Terrain comand as an array
« on: January 11, 2020, 05:46:33 pm »
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 :)
« Last Edit: January 18, 2020, 04:33:02 pm by Meridian »

Offline Finnik

  • Colonel
  • ****
  • Posts: 143
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #1 on: January 11, 2020, 05:58:06 pm »
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

Online Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 6387
  • Aaand we're back!
    • View Profile
    • My Wiki
Re: [Suggestion] Terrain comand as an array
« Reply #2 on: January 11, 2020, 07:10:59 pm »
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 :)
« Last Edit: January 11, 2020, 07:16:49 pm by Meridian »

Online Solarius Scorch

  • Global Moderator
  • Commander
  • *****
  • Posts: 9848
  • WE MUST DISSENT
    • View Profile
    • Nocturmal Productions modding studio website
Re: [Suggestion] Terrain comand as an array
« Reply #3 on: January 11, 2020, 07:15:27 pm »
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.

Offline Finnik

  • Colonel
  • ****
  • Posts: 143
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #4 on: January 11, 2020, 11:32:26 pm »
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

Online Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 6387
  • Aaand we're back!
    • View Profile
    • My Wiki
Re: [Suggestion] Terrain comand as an array
« Reply #5 on: January 11, 2020, 11:57:07 pm »
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.

Offline Finnik

  • Colonel
  • ****
  • Posts: 143
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #6 on: January 12, 2020, 02:04:23 am »
Quote
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

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

Sorry, my bad. Thank you for reviewing my code!
« Last Edit: January 12, 2020, 02:09:54 am by Finnik »

Online Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 6387
  • Aaand we're back!
    • View Profile
    • My Wiki
Re: [Suggestion] Terrain comand as an array
« Reply #7 on: January 12, 2020, 09:28:27 am »
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.

Offline Finnik

  • Colonel
  • ****
  • Posts: 143
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #8 on: January 12, 2020, 02:26:56 pm »
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.

Offline Finnik

  • Colonel
  • ****
  • Posts: 143
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #9 on: January 12, 2020, 04:19:51 pm »
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
« Last Edit: January 12, 2020, 04:21:24 pm by Finnik »

Online Yankes

  • Moderator
  • Commander
  • ***
  • Posts: 2349
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #10 on: January 12, 2020, 05:00:19 pm »
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);
  }
}


Offline Finnik

  • Colonel
  • ****
  • Posts: 143
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #11 on: January 12, 2020, 05:26:19 pm »
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.

Online Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 6387
  • Aaand we're back!
    • View Profile
    • My Wiki
Re: [Suggestion] Terrain comand as an array
« Reply #12 on: January 12, 2020, 05:32:14 pm »
@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.

Offline Finnik

  • Colonel
  • ****
  • Posts: 143
    • View Profile
Re: [Suggestion] Terrain comand as an array
« Reply #13 on: January 12, 2020, 05:52:42 pm »
in the current commit - there is no intersections with it, verticalLevels do not use randomTerrains, and i didnt changed anything about methods it uses

Online Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 6387
  • Aaand we're back!
    • View Profile
    • My Wiki
Re: [Suggestion] Terrain comand as an array
« Reply #14 on: January 13, 2020, 03:46:13 pm »
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)?