From 3d3b8b6bc674189cd5afe9321305aca0aa5fac49 Mon Sep 17 00:00:00 2001 From: Sryder Date: Sat, 21 Mar 2020 19:00:00 +0000 Subject: [PATCH] Replace many cases of if else if statements with I_Asserts. This is done specifically on static functions that aren't used as callbacks. This is done to keep the code cleaner, and the I_Asserts get compiled out in Release builds. Logic is left in the "public" functions to avoid crashes as a result of the modules. --- src/k_bheap.c | 278 ++++++++--------------- src/k_pathfind.c | 85 +++---- src/k_waypoint.c | 571 +++++++++++++++++++---------------------------- 3 files changed, 357 insertions(+), 577 deletions(-) diff --git a/src/k_bheap.c b/src/k_bheap.c index 527af1bfc..cf6848f68 100644 --- a/src/k_bheap.c +++ b/src/k_bheap.c @@ -18,20 +18,12 @@ static boolean K_BHeapItemValidate(bheap_t *heap, bheapitem_t *item) { boolean heapitemvalid = false; - if (heap == NULL) + I_Assert(heap != NULL); + I_Assert(item != NULL); + + if ((item->data != NULL) && (item->heapindex < SIZE_MAX / 2) && (item->owner == heap)) { - CONS_Debug(DBG_GAMELOGIC, "NULL heap in K_BHeapItemValidate.\n"); - } - else if (item == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item in K_BHeapItemValidate.\n"); - } - else - { - if ((item->data != NULL) && (item->heapindex < SIZE_MAX / 2) && (item->owner == heap)) - { - heapitemvalid = true; - } + heapitemvalid = true; } return heapitemvalid; @@ -53,40 +45,21 @@ static boolean K_BHeapItemValidate(bheap_t *heap, bheapitem_t *item) static bheapitem_t *K_BHeapItemsCompare(bheap_t *heap, bheapitem_t *item1, bheapitem_t *item2) { bheapitem_t *lowervalueitem = NULL; - if (heap == NULL) + + I_Assert(heap != NULL); + I_Assert(K_BHeapValid(heap)); + I_Assert(item1 != NULL); + I_Assert(item2 != NULL); + I_Assert(K_BHeapItemValidate(heap, item1)); + I_Assert(K_BHeapItemValidate(heap, item2)); + + if (item1->value < item2->value) { - CONS_Debug(DBG_GAMELOGIC, "NULL heap in K_BHeapItemsCompare.\n"); - } - else if (K_BHeapValid(heap) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid heap in K_BHeapSwapItems.\n"); - } - else if (item1 == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item1 in K_BHeapItemsCompare.\n"); - } - else if (item2 == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item2 in K_BHeapItemsCompare.\n"); - } - else if (K_BHeapItemValidate(heap, item1) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid item1 in K_BHeapItemsCompare.\n"); - } - else if (K_BHeapItemValidate(heap, item2) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid item2 in K_BHeapItemsCompare.\n"); + lowervalueitem = item1; } else { - if (item1->value < item2->value) - { - lowervalueitem = item1; - } - else - { - lowervalueitem = item2; - } + lowervalueitem = item2; } return lowervalueitem; @@ -107,31 +80,13 @@ static bheapitem_t *K_BHeapItemsCompare(bheap_t *heap, bheapitem_t *item1, bheap --------------------------------------------------*/ static void K_BHeapSwapItems(bheap_t *heap, bheapitem_t *item1, bheapitem_t *item2) { - if (heap == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL heap in K_BHeapSwapItems.\n"); - } - else if (K_BHeapValid(heap) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid heap in K_BHeapSwapItems.\n"); - } - else if (item1 == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item1 in K_BHeapSwapItems.\n"); - } - else if (item2 == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item2 in K_BHeapSwapItems.\n"); - } - else if (K_BHeapItemValidate(heap, item1) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid item1 in K_BHeapSwapItems.\n"); - } - else if (K_BHeapItemValidate(heap, item2) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid item2 in K_BHeapSwapItems.\n"); - } - else + I_Assert(heap != NULL); + I_Assert(K_BHeapValid(heap)); + I_Assert(item1 != NULL); + I_Assert(item2 != NULL); + I_Assert(K_BHeapItemValidate(heap, item1)); + I_Assert(K_BHeapItemValidate(heap, item2)); + { size_t tempitemindex = item1->heapindex; bheapitem_t tempitemstore = *item1; @@ -170,18 +125,10 @@ static size_t K_BHeapItemGetParentIndex(bheapitem_t *item) { size_t parentindex = SIZE_MAX; - if (item == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item in K_BHeapItemGetParentIndex.\n"); - } - else if (item->heapindex >= (SIZE_MAX / 2)) - { - CONS_Debug(DBG_GAMELOGIC, "Bad item heapindex in K_BHeapItemGetParentIndex.\n"); - } - else - { - parentindex = (item->heapindex - 1U) / 2U; - } + I_Assert(item != NULL); + I_Assert(item->heapindex < (SIZE_MAX / 2)); + + parentindex = (item->heapindex - 1U) / 2U; return parentindex; } @@ -201,18 +148,10 @@ static size_t K_BHeapItemGetLeftChildIndex(bheapitem_t *item) { size_t leftchildindex = SIZE_MAX; - if (item == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item in K_BHeapItemGetLeftChildIndex.\n"); - } - else if (item->heapindex >= (SIZE_MAX / 2)) - { - CONS_Debug(DBG_GAMELOGIC, "Bad item heapindex in K_BHeapItemGetLeftChildIndex.\n"); - } - else - { - leftchildindex = (item->heapindex * 2U) + 1U; - } + I_Assert(item != NULL); + I_Assert(item->heapindex < (SIZE_MAX / 2)); + + leftchildindex = (item->heapindex * 2U) + 1U; return leftchildindex; } @@ -232,18 +171,10 @@ static size_t K_BHeapItemGetRightChildIndex(bheapitem_t *item) { size_t rightchildindex = SIZE_MAX; - if (item == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item in K_BHeapItemGetRightChildIndex.\n"); - } - else if (item->heapindex >= (SIZE_MAX / 2)) - { - CONS_Debug(DBG_GAMELOGIC, "Bad item heapindex in K_BHeapItemGetRightChildIndex.\n"); - } - else - { - rightchildindex = (item->heapindex * 2U) + 2U; - } + I_Assert(item != NULL); + I_Assert(item->heapindex < (SIZE_MAX / 2)); + + rightchildindex = (item->heapindex * 2U) + 2U; return rightchildindex; } @@ -262,38 +193,27 @@ static size_t K_BHeapItemGetRightChildIndex(bheapitem_t *item) --------------------------------------------------*/ static void K_BHeapSortUp(bheap_t *heap, bheapitem_t *item) { - if (heap == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL heap in K_BHeapSortUp.\n"); - } - else if (K_BHeapValid(heap) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid heap in K_BHeapSortUp.\n"); - } - else if (item == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item in K_BHeapSortUp.\n"); - } - else - { - if (item->heapindex > 0U) - { - size_t parentindex = SIZE_MAX; - do - { - parentindex = K_BHeapItemGetParentIndex(item); + I_Assert(heap != NULL); + I_Assert(K_BHeapValid(heap)); + I_Assert(item != NULL); - // Swap the nodes if the parent has a higher value - if (K_BHeapItemsCompare(heap, item, &heap->array[parentindex]) == item) - { - K_BHeapSwapItems(heap, item, &heap->array[parentindex]); - } - else - { - break; - } - } while (parentindex > 0U); - } + if (item->heapindex > 0U) + { + size_t parentindex = SIZE_MAX; + do + { + parentindex = K_BHeapItemGetParentIndex(item); + + // Swap the nodes if the parent has a higher value + if (K_BHeapItemsCompare(heap, item, &heap->array[parentindex]) == item) + { + K_BHeapSwapItems(heap, item, &heap->array[parentindex]); + } + else + { + break; + } + } while (parentindex > 0U); } } @@ -311,71 +231,59 @@ static void K_BHeapSortUp(bheap_t *heap, bheapitem_t *item) --------------------------------------------------*/ static void K_BHeapSortDown(bheap_t *heap, bheapitem_t *item) { - if (heap == NULL) + I_Assert(heap != NULL); + I_Assert(K_BHeapValid(heap)); + I_Assert(item != NULL); + + if (heap->count > 0U) { - CONS_Debug(DBG_GAMELOGIC, "NULL heap in K_BHeapSortDown.\n"); - } - else if (K_BHeapValid(heap) == false) - { - CONS_Debug(DBG_GAMELOGIC, "Invalid heap in K_BHeapSortDown.\n"); - } - else if (item == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL item in K_BHeapSortDown.\n"); - } - else - { - if (heap->count > 0U) + size_t leftchildindex = SIZE_MAX; + size_t rightchildindex = SIZE_MAX; + bheapitem_t *leftchild = NULL; + bheapitem_t *rightchild = NULL; + bheapitem_t *swapchild = NULL; + boolean noswapneeded = false; + + do { - size_t leftchildindex = SIZE_MAX; - size_t rightchildindex = SIZE_MAX; - bheapitem_t *leftchild = NULL; - bheapitem_t *rightchild = NULL; - bheapitem_t *swapchild = NULL; - boolean noswapneeded = false; + leftchildindex = K_BHeapItemGetLeftChildIndex(item); + rightchildindex = K_BHeapItemGetRightChildIndex(item); - do + if (leftchildindex < heap->count) { - leftchildindex = K_BHeapItemGetLeftChildIndex(item); - rightchildindex = K_BHeapItemGetRightChildIndex(item); - - if (leftchildindex < heap->count) + leftchild = &heap->array[leftchildindex]; + swapchild = leftchild; + if (rightchildindex < heap->count) { - leftchild = &heap->array[leftchildindex]; - swapchild = leftchild; - if (rightchildindex < heap->count) + rightchild = &heap->array[rightchildindex]; + // Choose the lower child node to swap with + if (K_BHeapItemsCompare(heap, leftchild, rightchild) == rightchild) { - rightchild = &heap->array[rightchildindex]; - // Choose the lower child node to swap with - if (K_BHeapItemsCompare(heap, leftchild, rightchild) == rightchild) - { - swapchild = rightchild; - } - } - - // Swap with the lower child, if it's lower than item - if (K_BHeapItemsCompare(heap, swapchild, item) == swapchild) - { - K_BHeapSwapItems(heap, item, swapchild); - } - else - { - noswapneeded = true; + swapchild = rightchild; } + } + // Swap with the lower child, if it's lower than item + if (K_BHeapItemsCompare(heap, swapchild, item) == swapchild) + { + K_BHeapSwapItems(heap, item, swapchild); } else { noswapneeded = true; } + } + else + { + noswapneeded = true; + } - if (noswapneeded) - { - break; - } + if (noswapneeded) + { + break; + } - } while (item->heapindex < (heap->count - 1U)); - } + } while (item->heapindex < (heap->count - 1U)); } } diff --git a/src/k_pathfind.c b/src/k_pathfind.c index d80f20f32..ae05930f0 100644 --- a/src/k_pathfind.c +++ b/src/k_pathfind.c @@ -23,14 +23,10 @@ static const size_t DEFAULT_CLOSEDSET_CAPACITY = 8U; static UINT32 K_NodeGetFScore(const pathfindnode_t *const node) { UINT32 fscore = UINT32_MAX; - if (node == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL node in K_PathfindNodeGetFScore."); - } - else - { - fscore = node->gscore + node->hscore; - } + + I_Assert(node != NULL); + + fscore = node->gscore + node->hscore; return fscore; } @@ -52,7 +48,7 @@ static void K_NodeUpdateHeapIndex(void *const node, const size_t newheapindex) { if (node == NULL) { - CONS_Debug(DBG_GAMELOGIC, "NULL node in K_PathfindNodeUpdateHeapIndex.\n"); + CONS_Debug(DBG_GAMELOGIC, "NULL node in K_NodeUpdateHeapIndex.\n"); } else { @@ -84,28 +80,20 @@ static pathfindnode_t *K_NodesArrayContainsNodeData( size_t nodesarraycount) { pathfindnode_t *foundnode = NULL; + size_t i = 0U; - if (nodesarray == NULL) + I_Assert(nodesarray != NULL); + I_Assert(nodedata != NULL); + + // It is more likely that we'll find the node we are looking for from the end of the array + // Yes, the for loop looks weird, remember that size_t is unsigned and we want to check 0, after it hits 0 it + // will loop back up to SIZE_MAX + for (i = nodesarraycount - 1U; i < nodesarraycount; i--) { - CONS_Debug(DBG_GAMELOGIC, "NULL nodesarray in K_NodesArrayContainsWaypoint.\n"); - } - else if (nodedata == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL nodedata in K_NodesArrayContainsWaypoint.\n"); - } - else - { - size_t i; - // It is more likely that we'll find the node we are looking for from the end of the array - // Yes, the for loop looks weird, remember that size_t is unsigned and we want to check 0, after it hits 0 it - // will loop back up to SIZE_MAX - for (i = nodesarraycount - 1U; i < nodesarraycount; i--) + if (nodesarray[i].nodedata == nodedata) { - if (nodesarray[i].nodedata == nodedata) - { - foundnode = &nodesarray[i]; - break; - } + foundnode = &nodesarray[i]; + break; } } return foundnode; @@ -127,28 +115,19 @@ static pathfindnode_t *K_NodesArrayContainsNodeData( static boolean K_ClosedsetContainsNode(pathfindnode_t **closedset, pathfindnode_t *node, size_t closedsetcount) { boolean nodeisinclosedset = false; + size_t i = 0U; - if (closedset == NULL) + I_Assert(closedset != NULL); + I_Assert(node != NULL); + // It is more likely that we'll find the node we are looking for from the end of the array + // Yes, the for loop looks weird, remember that size_t is unsigned and we want to check 0, after it hits 0 it + // will loop back up to SIZE_MAX + for (i = closedsetcount - 1U; i < closedsetcount; i--) { - CONS_Debug(DBG_GAMELOGIC, "NULL closedset in K_PathfindClosedsetContainsNode.\n"); - } - else if (node == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL node in K_PathfindClosedsetContainsNode.\n"); - } - else - { - size_t i; - // It is more likely that we'll find the node we are looking for from the end of the array - // Yes, the for loop looks weird, remember that size_t is unsigned and we want to check 0, after it hits 0 it - // will loop back up to SIZE_MAX - for (i = closedsetcount - 1U; i < closedsetcount; i--) + if (closedset[i] == node) { - if (closedset[i] == node) - { - nodeisinclosedset = true; - break; - } + nodeisinclosedset = true; + break; } } return nodeisinclosedset; @@ -227,15 +206,9 @@ static boolean K_ReconstructPath(path_t *const path, pathfindnode_t *const desti { boolean reconstructsuccess = false; - if (path == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL path in K_ReconstructPath.\n"); - } - else if (destinationnode == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL destinationnode in K_ReconstructPath.\n"); - } - else + I_Assert(path != NULL); + I_Assert(destinationnode != NULL); + { size_t numnodes = 0U; pathfindnode_t *thisnode = destinationnode; diff --git a/src/k_waypoint.c b/src/k_waypoint.c index 849c0e5c7..cbda77f31 100644 --- a/src/k_waypoint.c +++ b/src/k_waypoint.c @@ -345,21 +345,11 @@ static void K_DebugWaypointsSpawnLine(waypoint_t *const waypoint1, waypoint_t *c UINT32 numofframes = 1; // If this was 0 it could divide by 0 // Error conditions - if (waypoint1 == NULL || waypoint2 == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL waypoint in K_DebugWaypointsSpawnLine.\n"); - return; - } - if (waypoint1->mobj == NULL || waypoint2->mobj == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL mobj on waypoint in K_DebugWaypointsSpawnLine.\n"); - return; - } - if (cv_kartdebugwaypoints.value == 0) - { - CONS_Debug(DBG_GAMELOGIC, "In K_DebugWaypointsSpawnLine when kartdebugwaypoints is off.\n"); - return; - } + I_Assert(waypoint1 != NULL); + I_Assert(waypoint1->mobj != NULL); + I_Assert(waypoint2 != NULL); + I_Assert(waypoint2->mobj != NULL); + I_Assert(cv_kartdebugwaypoints.value != 0); waypointmobj1 = waypoint1->mobj; waypointmobj2 = waypoint2->mobj; @@ -603,15 +593,9 @@ static void K_UpdateNodesArrayBaseSize(size_t newnodesarraysize) static UINT32 K_DistanceBetweenWaypoints(waypoint_t *const waypoint1, waypoint_t *const waypoint2) { UINT32 finaldist = UINT32_MAX; - if (waypoint1 == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL waypoint1 in K_DistanceBetweenWaypoints.\n"); - } - else if (waypoint2 == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL waypoint2 in K_DistanceBetweenWaypoints.\n"); - } - else + I_Assert(waypoint1 != NULL); + I_Assert(waypoint2 != NULL); + { const fixed_t xydist = P_AproxDistance(waypoint1->mobj->x - waypoint2->mobj->x, waypoint1->mobj->y - waypoint2->mobj->y); @@ -1104,21 +1088,12 @@ static boolean K_CheckWaypointForMobj(waypoint_t *const waypoint, void *const mo boolean mobjsmatch = false; // Error Conditions - if (waypoint == NULL) + I_Assert(waypoint != NULL); + I_Assert(waypoint->mobj != NULL); + I_Assert(mobjpointer != NULL); + { - CONS_Debug(DBG_GAMELOGIC, "NULL waypoint in K_CheckWaypointForMobj.\n"); - } - else if (waypoint->mobj == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "Waypoint has NULL mobj in K_CheckWaypointForMobj.\n"); - } - else if (mobjpointer == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL mobjpointer in K_CheckWaypointForMobj.\n"); - } - else - { - mobj_t *mobj = (mobj_t *)mobjpointer; + mobj_t *const mobj = (mobj_t *)mobjpointer; if (P_MobjWasRemoved(mobj)) { @@ -1170,76 +1145,61 @@ static waypoint_t *K_TraverseWaypoints( waypoint_t *foundwaypoint = NULL; // Error conditions - if (condition == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL condition in K_TraverseWaypoints.\n"); - } - else if (conditionalfunc == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL conditionalfunc in K_TraverseWaypoints.\n"); - } - else if (visitedarray == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL visitedarray in K_TraverseWaypoints.\n"); - } - else - { + I_Assert(condition != NULL); + I_Assert(conditionalfunc != NULL); + I_Assert(visitedarray != NULL); searchwaypointstart: - if (waypoint == NULL) + I_Assert(waypoint != NULL); + + { + size_t waypointindex = K_GetWaypointHeapIndex(waypoint); + // If we've already visited this waypoint, we've already checked the next waypoints, no point continuing + if ((waypointindex != SIZE_MAX) && (visitedarray[waypointindex] != true)) { - CONS_Debug(DBG_GAMELOGIC, "NULL waypoint in K_TraverseWaypoints.\n"); - } - else - { - size_t waypointindex = K_GetWaypointHeapIndex(waypoint); - // If we've already visited this waypoint, we've already checked the next waypoints, no point continuing - if ((waypointindex != SIZE_MAX) && (visitedarray[waypointindex] != true)) + // Mark this waypoint as being visited + visitedarray[waypointindex] = true; + + if (conditionalfunc(waypoint, condition) == true) { - // Mark this waypoint as being visited - visitedarray[waypointindex] = true; - - if (conditionalfunc(waypoint, condition) == true) + foundwaypoint = waypoint; + } + else + { + // If this waypoint only has one next waypoint, set the waypoint to be the next one and jump back + // to the start, this is to avoid going too deep into the stack where we can + // Yes this is a horrible horrible goto, but the alternative is a do while loop with an extra + // variable, which is slightly more confusing. This is probably the fastest and least confusing + // option that keeps this functionality + if (waypoint->numnextwaypoints == 1 && waypoint->nextwaypoints[0] != NULL) { - foundwaypoint = waypoint; + waypoint = waypoint->nextwaypoints[0]; + goto searchwaypointstart; } - else + else if (waypoint->numnextwaypoints != 0) { - // If this waypoint only has one next waypoint, set the waypoint to be the next one and jump back - // to the start, this is to avoid going too deep into the stack where we can - // Yes this is a horrible horrible goto, but the alternative is a do while loop with an extra - // variable, which is slightly more confusing. This is probably the fastest and least confusing - // option that keeps this functionality - if (waypoint->numnextwaypoints == 1 && waypoint->nextwaypoints[0] != NULL) + // The nesting here is a bit nasty, but it's better than potentially a lot of function calls on + // the stack, and another function would be very small in this case + UINT32 i; + // For each next waypoint, Search through it's path continuation until we hopefully find the one + // we're looking for + for (i = 0; i < waypoint->numnextwaypoints; i++) { - waypoint = waypoint->nextwaypoints[0]; - goto searchwaypointstart; - } - else if (waypoint->numnextwaypoints != 0) - { - // The nesting here is a bit nasty, but it's better than potentially a lot of function calls on - // the stack, and another function would be very small in this case - UINT32 i; - // For each next waypoint, Search through it's path continuation until we hopefully find the one - // we're looking for - for (i = 0; i < waypoint->numnextwaypoints; i++) + if (waypoint->nextwaypoints[i] != NULL) { - if (waypoint->nextwaypoints[i] != NULL) - { - foundwaypoint = K_TraverseWaypoints(waypoint->nextwaypoints[i], conditionalfunc, - condition, visitedarray); + foundwaypoint = K_TraverseWaypoints(waypoint->nextwaypoints[i], conditionalfunc, + condition, visitedarray); - if (foundwaypoint != NULL) - { - break; - } + if (foundwaypoint != NULL) + { + break; } } } - else - { - // No next waypoints, this function will be returned from - } + } + else + { + // No next waypoints, this function will be returned from } } } @@ -1270,24 +1230,13 @@ static waypoint_t *K_SearchWaypointGraph( waypoint_t *foundwaypoint = NULL; // Error conditions - if (condition == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL condition in K_SearchWaypointGraph.\n"); - } - else if (conditionalfunc == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL conditionalfunc in K_SearchWaypointGraph.\n"); - } - else if (firstwaypoint == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "K_SearchWaypointsForMobj called when no first waypoint.\n"); - } - else - { - visitedarray = Z_Calloc(numwaypoints * sizeof(boolean), PU_STATIC, NULL); - foundwaypoint = K_TraverseWaypoints(firstwaypoint, conditionalfunc, condition, visitedarray); - Z_Free(visitedarray); - } + I_Assert(condition != NULL); + I_Assert(conditionalfunc != NULL); + I_Assert(firstwaypoint != NULL); + + visitedarray = Z_Calloc(numwaypoints * sizeof(boolean), PU_STATIC, NULL); + foundwaypoint = K_TraverseWaypoints(firstwaypoint, conditionalfunc, condition, visitedarray); + Z_Free(visitedarray); return foundwaypoint; } @@ -1339,30 +1288,19 @@ static waypoint_t *K_SearchWaypointHeap( waypoint_t *foundwaypoint = NULL; // Error conditions - if (condition == NULL) + I_Assert(condition != NULL); + I_Assert(conditionalfunc != NULL); + I_Assert(waypointheap != NULL); + + // Simply search through the waypointheap for the waypoint which matches the condition. Much simpler when no + // pathfinding is needed. Search up to numwaypoints and NOT numwaypointmobjs as numwaypoints is the real number of + // waypoints setup in the heap while numwaypointmobjs ends up being the capacity + for (i = 0; i < numwaypoints; i++) { - CONS_Debug(DBG_GAMELOGIC, "NULL condition in K_SearchWaypointHeap.\n"); - } - else if (conditionalfunc == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "NULL conditionalfunc in K_SearchWaypointHeap.\n"); - } - else if (waypointheap == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "K_SearchWaypointHeap called when no waypointheap.\n"); - } - else - { - // Simply search through the waypointheap for the waypoint which matches the condition. Much simpler when no - // pathfinding is needed. Search up to numwaypoints and NOT numwaypointmobjs as numwaypoints is the real number of - // waypoints setup in the heap while numwaypointmobjs ends up being the capacity - for (i = 0; i < numwaypoints; i++) + if (conditionalfunc(&waypointheap[i], condition) == true) { - if (conditionalfunc(&waypointheap[i], condition) == true) - { - foundwaypoint = &waypointheap[i]; - break; - } + foundwaypoint = &waypointheap[i]; + break; } } @@ -1408,37 +1346,30 @@ waypoint_t *K_SearchWaypointHeapForMobj(mobj_t *const mobj) --------------------------------------------------*/ static UINT32 K_SetupCircuitLength(void) { - if ((firstwaypoint == NULL) || (numwaypoints == 0U)) + I_Assert(firstwaypoint != NULL); + I_Assert(numwaypoints > 0U); + I_Assert(finishline != NULL); + + // The circuit length only makes sense in circuit maps, sprint maps do not need to use it + // The main usage of the circuit length is to add onto a player's distance to finish line so crossing the finish + // line places people correctly relative to each other + if ((mapheaderinfo[gamemap - 1]->levelflags & LF_SECTIONRACE) == LF_SECTIONRACE) { - CONS_Debug(DBG_GAMELOGIC, "K_SetupCircuitLength called with no waypoints.\n"); - } - else if (finishline == NULL) - { - CONS_Debug(DBG_GAMELOGIC, "K_SetupCircuitLength called with no finishline waypoint.\n"); + circuitlength = 0U; } else { - // The circuit length only makes sense in circuit maps, sprint maps do not need to use it - // The main usage of the circuit length is to add onto a player's distance to finish line so crossing the finish - // line places people correctly relative to each other - if ((mapheaderinfo[gamemap - 1]->levelflags & LF_SECTIONRACE) == LF_SECTIONRACE) - { - circuitlength = 0U; - } - else - { - // Create a fake finishline waypoint, then try and pathfind to the finishline from it - waypoint_t fakefinishline = *finishline; - path_t bestcircuitpath = {}; - const boolean useshortcuts = false; - const boolean huntbackwards = false; + // Create a fake finishline waypoint, then try and pathfind to the finishline from it + waypoint_t fakefinishline = *finishline; + path_t bestcircuitpath = {}; + const boolean useshortcuts = false; + const boolean huntbackwards = false; - K_PathfindToWaypoint(&fakefinishline, finishline, &bestcircuitpath, useshortcuts, huntbackwards); + K_PathfindToWaypoint(&fakefinishline, finishline, &bestcircuitpath, useshortcuts, huntbackwards); - circuitlength = bestcircuitpath.totaldist; + circuitlength = bestcircuitpath.totaldist; - Z_Free(bestcircuitpath.array); - } + Z_Free(bestcircuitpath.array); } return circuitlength; @@ -1460,35 +1391,27 @@ static UINT32 K_SetupCircuitLength(void) static void K_AddPrevToWaypoint(waypoint_t *const waypoint, waypoint_t *const prevwaypoint) { // Error conditions - if (waypoint == NULL) + I_Assert(waypoint != NULL); + I_Assert(prevwaypoint != NULL); + + waypoint->numprevwaypoints++; + waypoint->prevwaypoints = + Z_Realloc(waypoint->prevwaypoints, waypoint->numprevwaypoints * sizeof(waypoint_t *), PU_LEVEL, NULL); + + if (!waypoint->prevwaypoints) { - CONS_Debug(DBG_SETUP, "NULL waypoint in K_AddPrevToWaypoint.\n"); + I_Error("K_AddPrevToWaypoint: Failed to reallocate memory for previous waypoints."); } - else if (prevwaypoint == NULL) + + waypoint->prevwaypointdistances = + Z_Realloc(waypoint->prevwaypointdistances, waypoint->numprevwaypoints * sizeof(fixed_t), PU_LEVEL, NULL); + + if (!waypoint->prevwaypointdistances) { - CONS_Debug(DBG_SETUP, "NULL prevwaypoint in K_AddPrevToWaypoint.\n"); + I_Error("K_AddPrevToWaypoint: Failed to reallocate memory for previous waypoint distances."); } - else - { - waypoint->numprevwaypoints++; - waypoint->prevwaypoints = - Z_Realloc(waypoint->prevwaypoints, waypoint->numprevwaypoints * sizeof(waypoint_t *), PU_LEVEL, NULL); - if (!waypoint->prevwaypoints) - { - I_Error("K_AddPrevToWaypoint: Failed to reallocate memory for previous waypoints."); - } - - waypoint->prevwaypointdistances = - Z_Realloc(waypoint->prevwaypointdistances, waypoint->numprevwaypoints * sizeof(fixed_t), PU_LEVEL, NULL); - - if (!waypoint->prevwaypointdistances) - { - I_Error("K_AddPrevToWaypoint: Failed to reallocate memory for previous waypoint distances."); - } - - waypoint->prevwaypoints[waypoint->numprevwaypoints - 1] = prevwaypoint; - } + waypoint->prevwaypoints[waypoint->numprevwaypoints - 1] = prevwaypoint; } /*-------------------------------------------------- @@ -1509,52 +1432,42 @@ static waypoint_t *K_MakeWaypoint(mobj_t *const mobj) mobj_t *otherwaypointmobj = NULL; // Error conditions - if (mobj == NULL || P_MobjWasRemoved(mobj)) - { - CONS_Debug(DBG_SETUP, "NULL mobj in K_MakeWaypoint.\n"); - } - else if (waypointcap == NULL) - { - CONS_Debug(DBG_SETUP, "K_MakeWaypoint called with NULL waypointcap.\n"); - } - else if (numwaypoints >= numwaypointmobjs) - { - CONS_Debug(DBG_SETUP, "K_MakeWaypoint called with max waypoint capacity reached.\n"); - } - else - { - // numwaypoints is incremented later in K_SetupWaypoint - madewaypoint = &waypointheap[numwaypoints]; - numwaypoints++; + I_Assert(mobj != NULL); + I_Assert(!P_MobjWasRemoved(mobj)); + I_Assert(waypointcap != NULL); // No waypoint mobjs in map load + I_Assert(numwaypoints < numwaypointmobjs); // waypoint array reached max capacity - P_SetTarget(&madewaypoint->mobj, mobj); + // numwaypoints is incremented later in K_SetupWaypoint + madewaypoint = &waypointheap[numwaypoints]; + numwaypoints++; - // Go through the other waypoint mobjs in the map to find out how many waypoints are after this one - for (otherwaypointmobj = waypointcap; otherwaypointmobj != NULL; otherwaypointmobj = otherwaypointmobj->tracer) + P_SetTarget(&madewaypoint->mobj, mobj); + + // Go through the other waypoint mobjs in the map to find out how many waypoints are after this one + for (otherwaypointmobj = waypointcap; otherwaypointmobj != NULL; otherwaypointmobj = otherwaypointmobj->tracer) + { + // threshold = next waypoint id, movecount = my id + if (mobj->threshold == otherwaypointmobj->movecount) { - // threshold = next waypoint id, movecount = my id - if (mobj->threshold == otherwaypointmobj->movecount) - { - madewaypoint->numnextwaypoints++; - } + madewaypoint->numnextwaypoints++; } + } - // No next waypoints - if (madewaypoint->numnextwaypoints != 0) + // No next waypoints + if (madewaypoint->numnextwaypoints != 0) + { + // Allocate memory to hold enough pointers to all of the next waypoints + madewaypoint->nextwaypoints = + Z_Calloc(madewaypoint->numnextwaypoints * sizeof(waypoint_t *), PU_LEVEL, NULL); + if (madewaypoint->nextwaypoints == NULL) { - // Allocate memory to hold enough pointers to all of the next waypoints - madewaypoint->nextwaypoints = - Z_Calloc(madewaypoint->numnextwaypoints * sizeof(waypoint_t *), PU_LEVEL, NULL); - if (madewaypoint->nextwaypoints == NULL) - { - I_Error("K_MakeWaypoint: Out of Memory allocating next waypoints."); - } - madewaypoint->nextwaypointdistances = - Z_Calloc(madewaypoint->numnextwaypoints * sizeof(fixed_t), PU_LEVEL, NULL); - if (madewaypoint->nextwaypointdistances == NULL) - { - I_Error("K_MakeWaypoint: Out of Memory allocating next waypoint distances."); - } + I_Error("K_MakeWaypoint: Out of Memory allocating next waypoints."); + } + madewaypoint->nextwaypointdistances = + Z_Calloc(madewaypoint->numnextwaypoints * sizeof(fixed_t), PU_LEVEL, NULL); + if (madewaypoint->nextwaypointdistances == NULL) + { + I_Error("K_MakeWaypoint: Out of Memory allocating next waypoint distances."); } } @@ -1578,93 +1491,83 @@ static waypoint_t *K_SetupWaypoint(mobj_t *const mobj) waypoint_t *thiswaypoint = NULL; // Error conditions - if (mobj == NULL || P_MobjWasRemoved(mobj)) + I_Assert(mobj != NULL); + I_Assert(!P_MobjWasRemoved(mobj)); + I_Assert(mobj->type == MT_WAYPOINT); + I_Assert(waypointcap != NULL); // no waypoint mobjs in map load + + // If we have waypoints already created, search through them first to see if this mobj is already added. + if (firstwaypoint != NULL) { - CONS_Debug(DBG_SETUP, "NULL mobj in K_SetupWaypoint.\n"); + thiswaypoint = K_SearchWaypointHeapForMobj(mobj); } - else if (mobj->type != MT_WAYPOINT) + + // The waypoint hasn't already been made, so make it + if (thiswaypoint == NULL) { - CONS_Debug(DBG_SETUP, "Non MT_WAYPOINT mobj in K_SetupWaypoint. Type=%d.\n", mobj->type); - } - else if (waypointcap == NULL) - { - CONS_Debug(DBG_SETUP, "K_SetupWaypoint called with NULL waypointcap.\n"); - } - else - { - // If we have waypoints already created, search through them first to see if this mobj is already added. - if (firstwaypoint != NULL) + mobj_t *otherwaypointmobj = NULL; + UINT32 nextwaypointindex = 0; + + thiswaypoint = K_MakeWaypoint(mobj); + + if (thiswaypoint != NULL) { - thiswaypoint = K_SearchWaypointHeapForMobj(mobj); - } - - // The waypoint hasn't already been made, so make it - if (thiswaypoint == NULL) - { - mobj_t *otherwaypointmobj = NULL; - UINT32 nextwaypointindex = 0; - - thiswaypoint = K_MakeWaypoint(mobj); - - if (thiswaypoint != NULL) + // Set the first waypoint if it isn't already + if (firstwaypoint == NULL) { - // Set the first waypoint if it isn't already - if (firstwaypoint == NULL) - { - firstwaypoint = thiswaypoint; - } + firstwaypoint = thiswaypoint; + } - if (K_GetWaypointIsFinishline(thiswaypoint)) - { - if (finishline != NULL) - { - const INT32 oldfinishlineid = K_GetWaypointID(finishline); - const INT32 thiswaypointid = K_GetWaypointID(thiswaypoint); - CONS_Alert( - CONS_WARNING, "Multiple finish line waypoints with IDs %d and %d! Using %d.", - oldfinishlineid, thiswaypointid, thiswaypointid); - } - finishline = thiswaypoint; - } - - if (thiswaypoint->numnextwaypoints > 0) - { - waypoint_t *nextwaypoint = NULL; - fixed_t nextwaypointdistance = 0; - // Go through the waypoint mobjs to setup the next waypoints and make this waypoint know they're its - // next. I kept this out of K_MakeWaypoint so the stack isn't gone down as deep - for (otherwaypointmobj = waypointcap; - otherwaypointmobj != NULL; - otherwaypointmobj = otherwaypointmobj->tracer) - { - // threshold = next waypoint id, movecount = my id - if (mobj->threshold == otherwaypointmobj->movecount) - { - nextwaypoint = K_SetupWaypoint(otherwaypointmobj); - nextwaypointdistance = K_DistanceBetweenWaypoints(thiswaypoint, nextwaypoint); - thiswaypoint->nextwaypoints[nextwaypointindex] = nextwaypoint; - thiswaypoint->nextwaypointdistances[nextwaypointindex] = nextwaypointdistance; - K_AddPrevToWaypoint(nextwaypoint, thiswaypoint); - nextwaypoint->prevwaypointdistances[nextwaypoint->numprevwaypoints - 1] = nextwaypointdistance; - nextwaypointindex++; - } - if (nextwaypointindex >= thiswaypoint->numnextwaypoints) - { - break; - } - } - } - else + if (K_GetWaypointIsFinishline(thiswaypoint)) + { + if (finishline != NULL) { + const INT32 oldfinishlineid = K_GetWaypointID(finishline); + const INT32 thiswaypointid = K_GetWaypointID(thiswaypoint); CONS_Alert( - CONS_WARNING, "Waypoint with ID %d has no next waypoint.\n", K_GetWaypointID(thiswaypoint)); + CONS_WARNING, "Multiple finish line waypoints with IDs %d and %d! Using %d.", + oldfinishlineid, thiswaypointid, thiswaypointid); + } + finishline = thiswaypoint; + } + + if (thiswaypoint->numnextwaypoints > 0) + { + waypoint_t *nextwaypoint = NULL; + fixed_t nextwaypointdistance = 0; + // Go through the waypoint mobjs to setup the next waypoints and make this waypoint know they're its + // next. I kept this out of K_MakeWaypoint so the stack isn't gone down as deep + for (otherwaypointmobj = waypointcap; + otherwaypointmobj != NULL; + otherwaypointmobj = otherwaypointmobj->tracer) + { + // threshold = next waypoint id, movecount = my id + if (mobj->threshold == otherwaypointmobj->movecount) + { + nextwaypoint = K_SetupWaypoint(otherwaypointmobj); + nextwaypointdistance = K_DistanceBetweenWaypoints(thiswaypoint, nextwaypoint); + thiswaypoint->nextwaypoints[nextwaypointindex] = nextwaypoint; + thiswaypoint->nextwaypointdistances[nextwaypointindex] = nextwaypointdistance; + K_AddPrevToWaypoint(nextwaypoint, thiswaypoint); + nextwaypoint->prevwaypointdistances[nextwaypoint->numprevwaypoints - 1] = nextwaypointdistance; + nextwaypointindex++; + } + if (nextwaypointindex >= thiswaypoint->numnextwaypoints) + { + break; + } } } else { - CONS_Debug(DBG_SETUP, "K_SetupWaypoint failed to make new waypoint with ID %d.\n", mobj->movecount); + CONS_Alert( + CONS_WARNING, "Waypoint with ID %d has no next waypoint.\n", K_GetWaypointID(thiswaypoint)); } } + else + { + CONS_Debug(DBG_SETUP, "K_SetupWaypoint failed to make new waypoint with ID %d.\n", mobj->movecount); + } } return thiswaypoint; @@ -1684,51 +1587,43 @@ static boolean K_AllocateWaypointHeap(void) boolean allocationsuccessful = false; // Error conditions - if (waypointheap != NULL) + I_Assert(waypointheap == NULL); // waypointheap is already allocated + I_Assert(waypointcap != NULL); // no waypoint mobjs at map load + + // This should be an allocation for the first time. Reset the number of mobjs back to 0 if it's not already + numwaypointmobjs = 0; + + // Find how many waypoint mobjs there are in the map, this is the maximum number of waypoints there CAN be + for (waypointmobj = waypointcap; waypointmobj != NULL; waypointmobj = waypointmobj->tracer) { - CONS_Debug(DBG_SETUP, "K_AllocateWaypointHeap called when waypointheap is already allocated.\n"); + if (waypointmobj->type != MT_WAYPOINT) + { + CONS_Debug(DBG_SETUP, + "Non MT_WAYPOINT mobj in waypointcap in K_AllocateWaypointHeap. Type=%d\n.", waypointmobj->type); + continue; + } + + numwaypointmobjs++; } - else if (waypointcap == NULL) + + if (numwaypointmobjs > 0) { - CONS_Debug(DBG_SETUP, "K_AllocateWaypointHeap called with NULL waypointcap.\n"); + // Allocate space in the heap for every mobj, it's possible some mobjs aren't linked up and not all of the + // heap allocated will be used, but it's a fairly reasonable assumption that this isn't going to be awful + waypointheap = Z_Calloc(numwaypointmobjs * sizeof(waypoint_t), PU_LEVEL, NULL); + + if (waypointheap == NULL) + { + // We could theoretically CONS_Debug here and continue without using waypoints, but I feel that will + // require error checks that will end up spamming the console when we think waypoints SHOULD be working. + // Safer to just exit if out of memory + I_Error("K_AllocateWaypointHeap: Out of memory."); + } + allocationsuccessful = true; } else { - // This should be an allocation for the first time. Reset the number of mobjs back to 0 if it's not already - numwaypointmobjs = 0; - - // Find how many waypoint mobjs there are in the map, this is the maximum number of waypoints there CAN be - for (waypointmobj = waypointcap; waypointmobj != NULL; waypointmobj = waypointmobj->tracer) - { - if (waypointmobj->type != MT_WAYPOINT) - { - CONS_Debug(DBG_SETUP, - "Non MT_WAYPOINT mobj in waypointcap in K_AllocateWaypointHeap. Type=%d\n.", waypointmobj->type); - continue; - } - - numwaypointmobjs++; - } - - if (numwaypointmobjs > 0) - { - // Allocate space in the heap for every mobj, it's possible some mobjs aren't linked up and not all of the - // heap allocated will be used, but it's a fairly reasonable assumption that this isn't going to be awful - waypointheap = Z_Calloc(numwaypointmobjs * sizeof(waypoint_t), PU_LEVEL, NULL); - - if (waypointheap == NULL) - { - // We could theoretically CONS_Debug here and continue without using waypoints, but I feel that will - // require error checks that will end up spamming the console when we think waypoints SHOULD be working. - // Safer to just exit if out of memory - I_Error("K_AllocateWaypointHeap: Out of memory."); - } - allocationsuccessful = true; - } - else - { - CONS_Debug(DBG_SETUP, "No waypoint mobjs in waypointcap.\n"); - } + CONS_Debug(DBG_SETUP, "No waypoint mobjs in waypointcap.\n"); } return allocationsuccessful; @@ -1794,7 +1689,11 @@ boolean K_SetupWaypointList(void) finishline = firstwaypoint; } - (void)K_SetupCircuitLength(); + if (K_SetupCircuitLength() == 0 + && ((mapheaderinfo[gamemap - 1]->levelflags & LF_SECTIONRACE) != LF_SECTIONRACE)) + { + CONS_Alert(CONS_ERROR, "Circuit track waypoints do not form a circuit.\n"); + } setupsuccessful = true; }