-
-
Notifications
You must be signed in to change notification settings - Fork 534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor astar pt2 #3320
Refactor astar pt2 #3320
Conversation
fecb13e
to
75e5168
Compare
75e5168
to
1244324
Compare
All pathfinding requests are done in reverse now. It allows to effectively reuse previous searches for group orders. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of questions, from an initial quick look-over.
Looking forward to testing this!
I looked this over a few days ago and will give it some tests later this week. |
@dkargin: I can reproduce a terrible lag, and have attached a save game (and map) that exhibits it:
When loading this save game you should see lag that brings FPS down into single digits. (Culprit is the pathfinding thread.) The cause is group 1 - the group of units in the middle-right. (They should all be selected when loading the save.) If you select them all and order them to attack one of the scavengers weapons up on the right ridge, it will trigger it again (and continue until they are halted). Of note: that ridge is obviously a different "continent" (in WZ terms), so the game should already know the units can't physically move onto that spot? This is something new introduced in this PR, as testing the same save without the PR doesn't yield the pathological lag. Additional findings:
So some edge case situation started in the pt1 PR, and is dramatically worse with the changes in this PR. EDIT: The original culprit that introduced a (temporary) lag spike blip (from astar pt 1) is: a65e3f8 |
src/astar.cpp
Outdated
// We have tried going to tileDest before. | ||
if (pfContext.map[tileOrig.x + tileOrig.y * mapWidth].iteration == pfContext.iteration | ||
&& pfContext.map[tileOrig.x + tileOrig.y * mapWidth].visited) | ||
if (pfContext.isTileVisited(pt)) | ||
{ | ||
// Already know the path from orig to dest. | ||
endCoord = tileOrig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
endCoord = tileOrig; | |
endCoord = tileOrig; | |
break; |
Probably missing a break
here? (This seems to make it harder to trigger the endless lag case, but there must be another path because it's still possible.)
The following diff NOTE: You must first halt all of group 1 by pressing "H" after loading the save. diff --git a/src/astar.cpp b/src/astar.cpp
--- a/src/astar.cpp
+++ b/src/astar.cpp
@@ -479,7 +479,7 @@ ASR_RETVAL fpathAStarRoute(std::list<PathfindContext>& fpathContexts,
ASR_RETVAL retval = ASR_OK;
- bool mustReverse = false;
+ bool mustReverse = true;
const PathCoord tileOrig = psJob->blockingMap->worldToMap(psJob->origX, psJob->origY);
const PathCoord tileDest = psJob->blockingMap->worldToMap(psJob->destX, psJob->destY);
@@ -514,6 +514,8 @@ ASR_RETVAL fpathAStarRoute(std::list<PathfindContext>& fpathContexts,
{
// Already know the path from orig to dest.
endCoord = tileOrig;
+ mustReverse = false;
+ break;
}
else if (pfContext.nodes.empty()) {
// Wave has already collapsed. Consequent attempt to search will exit immediately.
@@ -529,6 +531,7 @@ ASR_RETVAL fpathAStarRoute(std::list<PathfindContext>& fpathContexts,
ExplorationReport report = fpathAStarExplore(pfContext, pred, costLayer);
if (report) {
endCoord = pred.nearestCoord;
+ mustReverse = false;
// Found the path! Don't search more contexts.
break;
} |
|
||
if (!context.isBlocked(tileOrig.x, tileOrig.y)) // If blocked, searching from tileDest to tileOrig wouldn't find the tileOrig tile. | ||
{ | ||
// Next time, search starting from nearest reachable tile to the destination. | ||
fpathInitContext(context, psJob->blockingMap, tileDest, pred.nearestCoord, tileOrig, dstIgnore); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be behind some of the issue?, given how the lag is produced and given the initial temporary lag spikes started in a65e3f8 which changed the value that could be passed in (pred.nearestCoord
after that commit does not always equal context.nearestCoord
before that commit)
1. Wrapped PF map cache into a standalone structure/instance 2. Moved all global state out of astar.cpp. 3. Using backward path search. It provides more opportunities for reusing previous contexts. 4. Introduced cost accessor to A* wave exploration
Implemented debug rendering for pathginding: - renderer for blocking layer - rendering of current paths - drawing of path context
77d8527
to
757c9a3
Compare
Closing as this seems to be moved to #3650. |
Making pathfinding contexts
greatreusable again.