Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix to addAgents for proc scenes. #1032

Merged
merged 5 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion unity/Assets/Editor/Build.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private static List<string> GetAllScenePaths() {
}

// uncomment for faster builds for testing
return scenes; //.Where(x => x.Contains("FloorPlan1_") || x.Contains("Procedural")).ToList();
return scenes; //.Where(x => x.Contains("FloorPlan") || x.Contains("Procedural.unity")).ToList(); //.Where(x => x.Contains("FloorPlan1_") || x.Contains("Procedural")).ToList();
}

private static List<string> GetScenesFromEnv() {
Expand Down
58 changes: 30 additions & 28 deletions unity/Assets/Scripts/AgentManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -276,16 +276,16 @@ public void Initialize(ServerAction action) {
if (action.alwaysReturnVisibleRange) {
((PhysicsRemoteFPSAgentController)primaryAgent).alwaysReturnVisibleRange = action.alwaysReturnVisibleRange;
}

//if multi agent requested, add duplicates of primary agent now
//note: for Houses, adding multiple agents in will need to be done differently as currently
//initializing additional agents assumes the scene is already setup, and Houses initialize the
//primary agent floating in space, then generates the house, then teleports the primary agent.
//this will need a rework to make multi agent work as GetReachablePositions is used to position additional
//agents, which won't work if we initialize the agent(s) before the scene exists
string sceneName = UnityEngine.SceneManagement.SceneManager.GetActiveScene().name;
if (!sceneName.StartsWith("Procedural")) {
StartCoroutine(addAgents(action));
addAgents(action);
this.agents[0].m_Camera.depth = 9999;
if (action.startAgentsRotatedBy != 0f) {
RotateAgentsByRotatingUniverse(action.startAgentsRotatedBy);
} else {
ResetSceneBounds();
}
this.agentManagerState = AgentState.ActionComplete;
}

private void SetUpLocobotController(ServerAction action) {
Expand Down Expand Up @@ -348,28 +348,30 @@ public BaseFPSAgentController PrimaryAgent {
get => this.primaryAgent;
}

private IEnumerator addAgents(ServerAction action) {
yield return null;
Vector3[] reachablePositions = primaryAgent.getReachablePositions(2.0f);
for (int i = 1; i < action.agentCount && this.agents.Count < Math.Min(agentColors.Length, action.agentCount); i++) {
action.x = reachablePositions[i + 4].x;
action.y = reachablePositions[i + 4].y;
action.z = reachablePositions[i + 4].z;
addAgent(action);
yield return null; // must do this so we wait a frame so that when we CapsuleCast we see the most recently added agent
}
for (int i = 0; i < this.agents.Count; i++) {
this.agents[i].m_Camera.depth = 1;
}
this.agents[0].m_Camera.depth = 9999;
private void addAgents(ServerAction action) {
if (action.agentCount > 1) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we change it to a non-async function? what impact does this have?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the async functionality was there just to ensure that the transforms' positions/rotations were updated as we do some collision checks in addAgents. I believe this should be solved by using Physics.SyncTransforms() rather than waiting for a frame but let me know if not.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok cool, yeah seems like that was the only purpose to wait till the end of frame this looks good.

// note: for procedural scenes, adding multiple agents in will need to be done differently as currently
// initializing additional agents assumes the scene is already setup, and Houses initialize the
// primary agent floating in space, then generates the house, then teleports the primary agent.
// this will need a rework to make multi agent work as GetReachablePositions is used to position additional
// agents, which won't work if we initialize the agent(s) before the scene exists
if (UnityEngine.SceneManagement.SceneManager.GetActiveScene().name.StartsWith("Procedural")) {
throw new NotImplementedException($"Procedural scenes only support a single agent currently.");
}

if (action.startAgentsRotatedBy != 0f) {
RotateAgentsByRotatingUniverse(action.startAgentsRotatedBy);
} else {
ResetSceneBounds();
Physics.SyncTransforms();
Vector3[] reachablePositions = primaryAgent.getReachablePositions(2.0f);
for (int i = 1; i < action.agentCount && this.agents.Count < Math.Min(agentColors.Length, action.agentCount); i++) {
action.x = reachablePositions[i + 4].x;
action.y = reachablePositions[i + 4].y;
action.z = reachablePositions[i + 4].z;
addAgent(action);
Physics.SyncTransforms(); // must do this so that when we CapsuleCast we see the most recently added agent
}
for (int i = 1; i < this.agents.Count; i++) {
this.agents[i].m_Camera.depth = 1;
}
}

this.agentManagerState = AgentState.ActionComplete;
}

public void ResetSceneBounds() {
Expand Down