Add Vehicle option for Subwindow settings

[rajat2004]

Example Settings -

{
    "SeeDocsAt": "https://github.com/Microsoft/AirSim/blob/master/docs/settings_json.md",
    "SettingsVersion": 1.2,
    "SimMode": "Car",

    "Vehicles": {
        "Car1": {
          "VehicleType": "PhysXCar",
          "AutoCreate": true
        },
        "Car2": {
          "VehicleType": "PhysXCar",
          "AutoCreate": true,
          "X": 0, "Y": 10, "Z": 0
        }
    },
    "SubWindows": [
        {"WindowID": 0, "CameraName": "0", "ImageType": 3, "VehicleName": "Car1", "Visible": false},
        {"WindowID": 1, "CameraName": "0", "ImageType": 5, "VehicleName": "Car2", "Visible": false},
        {"WindowID": 2, "CameraName": "0", "ImageType": 0, "VehicleName": "Car1", "Visible": false}
    ],
}

I've tested this to make sure that earlier behaviour is unchanged, i.e. if Vehicle is not specified, first vehicle is used. And if no SubWindows settings are specified, then the default windows appear. Any testing would be great!

Closes #2838, closes #1172

[mhl787156]

Thanks for doing this! Made a comment in the code for a minor change to deal with an edge case I came into. Also, perhaps naming the new field 'VehicleName' would be better to stay consistent with 'CameraName'?

[mhl787156]

A couple of extra comments for minor changes and clean up!

[mhl787156]

Today I tried to make an identical change on my fork - had an issue where the default WindowID of zero was overwriting a specified window for some reason? (At least I think that's what was happening). Specifying WindowID as 0, 1 and 2 respectively seemed to fix it

[mhl787156]

Just reviewing, I believe It's the particular case where you only assign, say, Drone 2 to WindowID 0 and Drone 1 to WindowID 2. The defaults for the '3rd default' end up overwriting Drone 2 on WindowID 0, instead of doing nothing (or perhaps only initialising the windowID1).

[rajat2004]

Didn't understand this exactly, how to reproduce the problem?

[mhl787156]

Use the following settings:
"SubWindows": [
{"WindowID": 0, "CameraName": "0", "ImageType": 3, "VehicleName": "Car2", "Visible": true},
{"WindowID": 2, "CameraName": "0", "ImageType": 0, "VehicleName": "Car1", "Visible": true}
],

What you will see is Car 2's view in WindowID 0 being replaced by the default view (I assume Car1).

This is due to all initialised subwindows being initialised with WindowID 0. As only 2 of the default subwindows are overwritten, the third is still 'active'.

Is this explanation any clearer? [Edit made visible true instead of false]

[rajat2004]

Yup, that makes sense, I'll test it out. The second comment wasn't there when I replied :)

[rajat2004]

Thanks for the detailed explanation! I've changed the Window IDs to 0,1,2

[mhl787156]

Is this above if statement (lines 313 -326) necessary anymore?

[rajat2004]

They add default cameras if the lines below fail, so I think they're needed

[mhl787156]

Ah yes you are right, thats the safest option

[rajat2004]

Also, perhaps naming the new field 'VehicleName' would be better to stay consistent with 'CameraName'?

Changed to VehicleName, also added info in settings.md

[madratman]

/azp run microsoft.AirSim

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rajat2004]

Currently, Unity is crashing on startup, when subwindow settings are not there, or even if a Car vehicle is specified due to error in parsing the settings file. Only works if the settings are empty.
This will require extensive fixes in the Unity code, there's a PR #2303, maybe will try to bring it in and build on top of that. That'll be a different PR, and will take time. So either this and some other PRs can go in, and Unity fixed later, or fix the Unity part first, and then make the changes for this PR

[madratman]

Hmm, is a non-impl in Unity feasible? If so, we should add this feature in Unreal for this PR, and worry about Unity in a subsequent one.

[rajat2004]

@madratman Ideally, there shouldn't be any effect on Unity, since it does all the settings.json parsing itself, and should just ignore the VehicleName field. However, I'm unable to test this since Unity just fails in parsing for me when Subwindow settings are there, even without the new field. As mentioned, will require extensive digging and fixing Unity code.
Similar case with #2861, Unity probably shouldn't be broken any more than it already is

[rajat2004]

Ah hell, this PR broke the master CI build on Azure, no idea how though

[madratman]

uh yeah. weird that it didn't happen earlier

Building 11 actions with 6 processes...
  [1/11] Default.rc2
  [2/11] Default.rc2
  [3/11] SharedPCH.Engine.ShadowErrors.cpp
  [4/11] Blocks.cpp
  [5/11] UE4Editor-Blocks.lib
     Creating library C:\__w\14\s\Unreal\Environments\Blocks\Intermediate\Build\Win64\UE4Editor\Development\Blocks\UE4Editor-Blocks.lib and object C:\__w\14\s\Unreal\Environments\Blocks\Intermediate\Build\Win64\UE4Editor\Development\Blocks\UE4Editor-Blocks.exp
  [6/11] Module.AirSim.gen.cpp
  [7/11] Module.AirSim.cpp
  C:/__w/14/s/Unreal/Environments/Blocks/Plugins/AirSim/Source/SimHUD/SimHUD.cpp(340): error C4456: declaration of 'vehicle_sim_api' hides previous local declaration
  C:/__w/14/s/Unreal/Environments/Blocks/Plugins/AirSim/Source/SimHUD/SimHUD.cpp(322): note: see declaration of 'vehicle_sim_api'

the 2nd declaration of vehicle_sim_api inside the for loop is causing a compiler error in windows. Fix is to just rename it to anything else.

[rajat2004]

Opened #2998 which fixes this and a Sphinx error