front: call pathfinding only once #9733
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #9733 +/- ##
==========================================
- Coverage 37.82% 37.81% -0.02%
==========================================
Files 994 994
Lines 91126 91166 +40
Branches 1176 1176
==========================================
+ Hits 34470 34475 +5
- Misses 56202 56237 +35
Partials 454 454
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
anisometropie
force-pushed
the
vcs/fix-block-pathfinding-called-twice
branch
from
b2913ba to
8acddb2
Compare
|
Hm, won't this result in a pathfinding being started on stale data, and the latest via change will get ignored? |
| if (pathfindingState.running) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
It seems to fix the symptom but not the underlying issue.
from the test I have run, haven’t seen a case where it doesn’t use the latest data |
|
What about this case:
Latest user change is ignored here. I'd argue correctness (user input not ignored) is more important than performance (1 pathfinding request instead of 2). |
anisometropie
force-pushed
the
vcs/fix-block-pathfinding-called-twice
branch
4 times, most recently
from
012b40d to
6e08dde
Compare
Signed-off-by: Valentin Chanas <[email protected]>
anisometropie
force-pushed
the
vcs/fix-block-pathfinding-called-twice
branch
from
6e08dde to
9459907
Compare
| @@ -209,6 +226,12 @@ export const usePathfinding = ( | |||
| }, [vias]); | |||
|
|
|||
| useEffect(() => { | |||
| if ( | |||
| (previousOrigin !== origin || previousDestination !== destination) && | |||
There was a problem hiding this comment.
I don't think this is enough: an intermediary waypoint might've changed too, or rolling stock, or something else…
But, what is the root cause? Why does pathfinding run twice, which state change causes this?
There was a problem hiding this comment.
intermediary waypoint don’t do aything in this useEffect, and rolling don’t create the bug.
Root cause is pathfinding updating vias when it succeeds. So we need to ignore exactly one call made and resume listening after, which is what pathStepsCompletedWithPF does
There was a problem hiding this comment.
probably gonna change after the refacto #9924
closes #6719
Ignore the vias changes made automatically from the pathfinding result which resulted in a new pathfinding request.