search

NVlabs / FPSci

Aim Training Experiments
Other
67 stars 23 forks source link

Trial-level configuration #392

Closed bboudaoud-nv closed 1 year ago

bboudaoud-nv commented 2 years ago

This branch adds support for trial-level configuration inherited from the experiment and session in turn.

This is a first-step towards adding staircase designs to FPSci as described in #390.

Merging this PR closes #391.

jspjutNV commented 1 year ago

I believe there's a bug in the current version of this PR where the dummy target respawns at a different location after each trial, at least in some configurations. The one I first noticed it in is the sa2019_1hit sample.

Edit: I can confirm it's happening in other samples too. Seems pretty consistent.

Edit2: It appears to use the horizontal position of the cursor for the new dummy target. ~I believe I've only tested player space targets so far. I'll test world space shortly.~ This happens for both player and world space targets.

bboudaoud-nv commented 1 year ago

The bug @jspjutNV mentioned above has now been resolved. Specifically it only occurs when resetPlayerPositionBetweenTrials is false and the view moves away from the default view direction. Along with this change came some fixes for handling getting the correct spawn direction on the first trial of a session when resetPlayerPositionBetweenTrials is false.

jspjutNV commented 1 year ago

I created a pre-release for the current state of this PR. There's still some issues, but this should make it easier for non-developers to test their experiments and report bugs.

jspjutNV commented 1 year ago

I'm noticing a bug in the current version that seems to be related to having a scene name set without a spawnHeading, expecting the default value, or the one set in the scene for the spawn heading. The result is that the dummy target spawns somewhere unreachable in my test, but it may be somewhere reachable in others. Adding a spawnHeading to the config seems to resolve the issue, but is a regression compared to previous versions. We'll want to reflect this change in the documentation and release notes if this change is intentional.

bboudaoud-nv commented 1 year ago

@jspjutNV yes, this bug was introduced by my "fix" for the issue when resetPlayerPositionBetweenTrials is false. I need to think a bit more on all the places the spawn heading might come from and come up with a more robust solution for where to locate the reference target here.

bboudaoud-nv commented 1 year ago

I'm now running into issues with specifying the scene name parameter on a trial-by-trial basis. Will need to look into this configuration interaction.

bboudaoud-nv commented 1 year ago

I believe I've now fixed the issues with scene names changing between trials. @jspjutNV you should be able to test and see if the previous issue with spawnHeading not being specified.

jspjutNV commented 1 year ago

The latest commits fix most of the issues we saw before.

I've notice that 2 tests are failing, but haven't investigated the root cause yet. These failing tests are:

bboudaoud-nv commented 1 year ago

@jspjutNV both of these issues are now resolved.

FPSciTests.TestAutoFire was failing as it was still attempting to configure the weapon through the session-level config. I changed this to use the new trial-level config which is what takes effect. FPSciTests.TestFreshStart was failing due to the m_loadedScene.name not being cleared in FPSciApp::initExperiment() which meant that GApp::loadScene() was being called with an empty string as its argument (as you pointed out this also impacted selection of an experiment through the in-app drop down). I fixed this by clearing the m_loadedScene.name in FPSciApp::initExperiment().