-
Notifications
You must be signed in to change notification settings - Fork 254
refactor(pathfind): Optimize pathfind snippets for higher performance #3198
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
base: main
Are you sure you want to change the base?
Changes from all commits
be212f4
dac4aa9
65c9a50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,28 +173,6 @@ PathNode *PathNode::prependToList( PathNode *list ) | |
| return this; | ||
| } | ||
|
|
||
| //----------------------------------------------------------------------------------- | ||
| /// given a list, append this node, return new list. slow implementation. | ||
| /// @todo optimize this | ||
| PathNode *PathNode::appendToList( PathNode *list ) | ||
| { | ||
| if (list == nullptr) | ||
| { | ||
| m_next = nullptr; | ||
| m_prev = nullptr; | ||
| return this; | ||
| } | ||
|
|
||
| PathNode *tail; | ||
| for( tail = list; tail->m_next; tail = tail->m_next ) | ||
| ; | ||
|
|
||
| tail->m_next = this; | ||
| m_prev = tail; | ||
| m_next = nullptr; | ||
|
|
||
| return list; | ||
| } | ||
|
|
||
| //----------------------------------------------------------------------------------- | ||
| /// given a node, append new node to this. | ||
|
|
@@ -437,9 +415,17 @@ void Path::appendNode( const Coord3D *pos, PathfindLayerEnum layer ) | |
| node->setPosition( pos ); | ||
| node->setLayer(layer); | ||
|
|
||
| m_path = node->appendToList( m_path ); | ||
| if (!m_path) | ||
| { | ||
| m_path = node; | ||
| m_pathTail = node; | ||
|
|
||
| if (m_isOptimized && m_pathTail) | ||
| return; | ||
| } | ||
|
|
||
| m_pathTail->append(node); | ||
|
|
||
| if (m_isOptimized) | ||
| { | ||
| m_pathTail->setNextOptimized(node); | ||
| } | ||
|
|
@@ -6184,6 +6170,7 @@ struct ExamineCellsStruct | |
| const LocomotorSet *theLoco; | ||
| Bool centerInCell; | ||
| Bool isHuman; | ||
| Bool isCrusher; | ||
| Int radius; | ||
| const Object *obj; | ||
| PathfindCell *goalCell; | ||
|
|
@@ -6192,10 +6179,9 @@ struct ExamineCellsStruct | |
| /*static*/ Int Pathfinder::examineCellsCallback(Pathfinder* pathfinder, PathfindCell* from, PathfindCell* to, Int to_x, Int to_y, void* userData) | ||
| { | ||
| ExamineCellsStruct* d = (ExamineCellsStruct*)userData; | ||
| Bool isCrusher = d->obj ? d->obj->getCrusherLevel() > 0 : false; | ||
| if (d->thePathfinder->m_isTunneling) return 1; // abort. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this check necessary? Could maybe be changed to an assertion given that it's already checked at the call site.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment it is not necessary but removing this won't give any measurable performance boost, so I would say out of scope. I think it fits more with a refactor of the pathfinding code that Mauller and I are planning to do. |
||
| if (from && to) { | ||
| if (!d->thePathfinder->validMovementPosition( isCrusher, d->theLoco->getValidSurfaces(), to, from )) { | ||
| if (!d->thePathfinder->validMovementPosition( d->isCrusher, d->theLoco->getValidSurfaces(), to, from )) { | ||
| return 1; | ||
| } | ||
| if ( (to->getLayer() == LAYER_GROUND) && !d->thePathfinder->m_zoneManager.isPassable(to_x, to_y) ) { | ||
|
|
@@ -6297,6 +6283,7 @@ Int Pathfinder::examineNeighboringCells(PathfindCell *parentCell, PathfindCell * | |
| info.radius = radius; | ||
| info.obj = obj; | ||
| info.isHuman = isHuman; | ||
| info.isCrusher = isCrusher; | ||
| info.goalCell = goalCell; | ||
| ICoord2D start, end; | ||
| start.x = parentCell->getXIndex(); | ||
|
|
@@ -6322,6 +6309,11 @@ Int Pathfinder::examineNeighboringCells(PathfindCell *parentCell, PathfindCell * | |
|
|
||
| UnsignedInt newCostSoFar = 0; | ||
|
|
||
| Coord3D fromPos; | ||
| fromPos.x = parentCell->getXIndex() * PATHFIND_CELL_SIZE_F ; | ||
| fromPos.y = parentCell->getYIndex() * PATHFIND_CELL_SIZE_F ; | ||
| fromPos.z = TheTerrainLogic->getGroundHeight(fromPos.x , fromPos.y); | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
|
|
||
| for( int i=0; i<numNeighbors; i++ ) | ||
| { | ||
| neighborFlags[i] = false; | ||
|
|
@@ -6360,11 +6352,6 @@ Int Pathfinder::examineNeighboringCells(PathfindCell *parentCell, PathfindCell * | |
| // do the gravity check here | ||
| if ( locomotorSet.isDownhillOnly() ) | ||
| { | ||
| Coord3D fromPos; | ||
| fromPos.x = parentCell->getXIndex() * PATHFIND_CELL_SIZE_F ; | ||
| fromPos.y = parentCell->getYIndex() * PATHFIND_CELL_SIZE_F ; | ||
| fromPos.z = TheTerrainLogic->getGroundHeight(fromPos.x , fromPos.y); | ||
|
|
||
| Coord3D toPos; | ||
|
Skyaero42 marked this conversation as resolved.
|
||
| toPos.x = newCellCoord.x * PATHFIND_CELL_SIZE_F ; | ||
| toPos.y = newCellCoord.y * PATHFIND_CELL_SIZE_F ; | ||
|
|
@@ -6430,11 +6417,6 @@ Int Pathfinder::examineNeighboringCells(PathfindCell *parentCell, PathfindCell * | |
| } | ||
|
|
||
| if (newCell->getType() == PathfindCell::CELL_CLIFF && !newCell->getPinched() ) { | ||
| Coord3D fromPos; | ||
| fromPos.x = parentCell->getXIndex() * PATHFIND_CELL_SIZE_F ; | ||
| fromPos.y = parentCell->getYIndex() * PATHFIND_CELL_SIZE_F ; | ||
| fromPos.z = TheTerrainLogic->getGroundHeight(fromPos.x , fromPos.y); | ||
|
|
||
| Coord3D toPos; | ||
| toPos.x = newCellCoord.x * PATHFIND_CELL_SIZE_F ; | ||
| toPos.y = newCellCoord.y * PATHFIND_CELL_SIZE_F ; | ||
|
|
||
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.
The commit titles say "refactor" instead of "perf"