search

NVlabs / FPSci

Aim Training Experiments
Other
70 stars 23 forks source link

Release v21.10.01 autogenerated experiment config fails to load #335

Closed jspjutNV closed 3 years ago

jspjutNV commented 3 years ago

Steps to reproduce:

  1. Download v21.10.01
  2. Double click FirstPersonScience.exe.
  3. Get the following launch error
C:\Users\jbspj\Downloads\FPSci.v21.10.01/experimentconfig.Any:6(26): Parse error: Name must be Vector3 or Point3

Expression: false
C:\g3d\G3D10\G3D-app.lib\source\GApp.cpp:868

You can work around this issue by

  1. Run FirstPersonScience.exe until the crash
  2. Open up the startupconfig.Any and change jsonAnyOutput to false
  3. Delete the experimentconfig.Any that was generated
  4. Run FirstPersonScience.exe again.
jspjutNV commented 3 years ago

Looking a bit more at this problem, it seems related to changes from #316 which caused the default experiment to be JSON formatted as well as something the scene fields, specifically the spawnPosition which needs to be specified as a Point3().

I tried removing the spawnPosition lines and ran into a secondary issue that the mouse no longer controls the player aim, but I didn't root cause that issue.

bboudaoud-nv commented 3 years ago

Scene has some parameters that appear to always serialize to Any due to NaN values not being compared correctly in the Scene::toAny() we should correct this by checking w/ isnan() directly.

bboudaoud-nv commented 3 years ago

We need a solution for dealing with JSON vs Any incompatibility for types like Vector2/Vector3 and Point2/Point3. These are the issues that keep JSON files from loading as valid configs currently. We could move towards Arrays (which work fine) or avoid JSON input for now, but we should certainly update and document as needed.

jspjutNV commented 3 years ago

336 may have at least partially addressed this issue.

bboudaoud-nv commented 3 years ago

340 is a potential fix to issues with Vector[X], Point[X], and Color[X] serialization issues w/ JSON described above.

jspjutNV commented 3 years ago

341 introduces a test to hopefully prevent this type of regression in the future.