Add strict pathfinding setting for looping courses - #559
Conversation
|
Let me know what's preferred regarding the |
|
@Iwan1803 or @Axel32019, would one of you have time to take a look at this, or suggest someone who could? I'll rebase after you let me know what to do about the |
|
There is no need for an additional setting. |
|
Reverse drive not complete considered: |
|
Thanks! I also noticed it doesn't work in cases where the AD driver diverts (i.e. for gas), since it's not the end of a looping course. I'll cut a new revision that removes the setting, handles reverse, and handles diversions. Do you have any automated testing I missed, or something like a test save with prebuilt test routes? |
do not use lastUsedWayPoint as start point if destination reached in revers driving
|
Also @Iwan1803 was concerned about regressing behavior, so I want to call out that this change will affect pretty much everyone who uses looping courses. The existing logic to find a start node pretty much never picks the destination node as the start of the new route, because vehicles approaching the destination nearly always drive far enough forward that the destination node is rejected as a start candidate. |
|
|
||
| function ADDrivePathModule:setPathTo(wayPointId) | ||
| self:reset(self.atTarget) | ||
| self:reset(self.atTarget or self.isResumingFromLastWayPoint) |
There was a problem hiding this comment.
This doesn't reset self.isResumingFromLastWaypoint after setting the path, which I was a bit on the fence about. This way the flag remains in case it's useful for other decisions on the same course, but if there's any places I missed where setPathTo is called without resetting if necessary then we could wind up using the last waypoint in cases where we don't intend to.
|
Sorry about the closing/reopening, apparently github automatically closes PR's when you use their UI to sync forks? Since you've pushed the majority of the change, all that's left is to use this logic after refueling/repairing, which unfortunately required tracking some additional state since |
Adds a new setting "strictPathResume" to resume looping courses from the destination of the previous leg rather than searching for a new start point. This new setting is off by default for backwards compatibility. I didn't see any reason to make it per-vehicle, so right now it's a global setting.
This addresses the inconsistent/unexpected behavior mentioned in #558
I'm marking this PR as a draft to call attention to the large diff in
translation_tr.xml. It looks like thesync_translationsscript is making some whitespace changes to older sections that weren't previously committed, and I'm not sure if I should use the cleaned up automatically generated version or hand-edit to preserve the original whitespace.