Agent's `set_destination` needs update in their logic and fix broken tailgating, but how?

[Daraan]

Description

Addresses #7341:
set_destination does not really use the start_location, only for boolean evaluation instead of allowing users to pass a location.

In my experiments this sometimes creates troubles when passing a starting point, e.g. for tailgating the agent performs U-Turns to an old way point.

Where has this been tested?

Note: Python code only

• Platform(s): Ubuntu 22.04
• Python version(s): 3.7, 3.10
• Unreal Engine version(s): 4.26

Possible Drawbacks

[update-docs[bot]]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update our CHANGELOG.md based on your changes.

[Daraan]

@glopezdiest could you give it a look when you have the chance and share your thoughts. Thanks a lot.

[glopezdiest]

Hello @Daraan. Yes, you are correct in that the start_location is no longer being used. After some though, I'm seeing three use-cases for the set_destination.

The first one is the more generic one. Both start_location and end_location are given, because a specific path is desired. In this case, the function should clean the previous queue, and just add the new route. In this case, the start_location can make the controller fail if it is a weird position, but that is acceptable and the fault relies on the inputs, not the function itself.

The second one is the rerouting one, where you just want to, for example, stop moving randomly, and start moving in a specific way.

The third one is the non-interference one, where you don't want to change the current behavior, and just add to it once it has finished.

The behavior agent uses the first approach, which uses the start_location, hence why it is failing. What do you think about it? These functions tend to be extended over time according to their needs, so I don't see any problem in changing it, as long as compatibility isn't broken.

[Daraan]

Thank you for your reply.

As summary, we have these cases and end_location is always not None:

The first one is the more generic one. Both start_location and end_location are given, because a specific path is desired. In this case, the function should clean the previous queue, and just add the new route. In this case, the start_location can make the controller fail if it is a weird position, but that is acceptable and the fault relies on the inputs, not the function itself.

• start_location is not None
• clear_queue=True

The second one is the rerouting one, where you just want to, for example, stop moving randomly, and start moving in a specific way.

• start_location is None
• clear_queue=True

The third one is the non-interference one, where you don't want to change the current behavior, and just add to it once it has finished.

• start_location is None
• clear_queue=False

Theoretically this gives a fourth case similar to 3) with the problems of 1) and up to the input.

• start_location is not None
• clear_queue=False

---

I'll look again over it in the coming days/week

[glopezdiest]

That breakdown looks good to me

Theoretically this gives a fourth case similar to 3) with the problems of 1) and up to the input. start_location is not None clear_queue=False And yes, that's true, but I don't see an issue with that. It is a weird use-case from my point of view, but maybe someone can find a reasonable on later on

[glopezdiest]

@Daraan Would you be interested in changing the set_destination function as discussed? I think this would be a good fix

[Daraan]

Yes, I am just still held up with something else currently taking longer than expected. I'll look again in the next week (s)

[Daraan]

@glopezdiest

I've finished and updated the rework. Comments are currently left in for you and can/should be cleaned.

Case 2 start_location=None, clean_queue=True:

• Default A same as current behavior: However, it also checks if the local_planner.target_waypoint is set. This is currently always the case, but I think it could make sense in the future to set it to None in LocalPlanner.run_step when the queue is empty instead of keeping the last waypoint that was targeted -> fallback to vehicle location if it is not available.

Case 1 : start_location not None, clean_queue=True:

• Default B - New behavior : start_location is actually used instead of disregarding it for the vehicle location. For me this fixes the BehaviorAgents buggy tailgaiting behavior of randomly turning around because the queue, was not cleaned (#7341 )

Case 3: start_location=None, clean_queue=False:

• New Option, similar to the current one when start_location is not None: If the queue is not empty takes the last waypoint from the local planner's waypoint queue as starting location so that the new route is appended. In my opinion it makes no sense to plan from the current location, right?. If empty -> fallback to vehicle location.
  NOTE: This last waypoint will then be twice in the queue. Do you think this could cause a problem? Could also pop it at that location instead.

Case 4: start_location not None, clean_queue=False:

• New theoretical option as discussed above.

[glopezdiest]

Looks good, if you remove the comments we can merge it

[Daraan]

Looks good, if you remove the comments we can merge it

Take a look again. Are you find with the cleaned comments and type annotation or should I fully remove them?

[glopezdiest]

The small comment is fine, as a further clarification. I'll merge it ASAP