path_finding.h: * DEFINITIONS: * - step: one movement step which brings us from one tile to an adjacent one * - turn: turns spent to reach a tile. Since movement rules involve * randomness, we use different "turn modes" to get an estimate of this * number should be rewritten thusly: * DEFINITIONS: * step: one movement step which brings us from one tile to an adjacent one * turn: turns spent to reach a tile. Since movement rules involve * randomness, we use different "turn modes" to get an estimate of * this number sp. * - extra cost (EC): extra cost of a _sinlge_ tile. A user supplied value * which is >=0 this is also nonsensical. * FORMULAE: * - calculating total_MC * in TM_NONE * total_MC = sum of MC * in TM_CAPPED * total_MC = sum of MIN(MC, move_rate) * in TM_*_TIME * total_MC = ((turn + 1) * move_rate - moves_left this makes no sense. * - calculating total combined cost * total_CC = PF_TURN_FACTOR * total_MC + move_rate * tot used PF_TURN_FACTOR before defining * A) the caller knows the goal position and wants to know the path: * * struct pf_parameter parameter; * pf_map_t pf_map; * struct pf_path path; * * // fill parameter * * pf_map = pf_get_map(¶meter); * * pf_get_path(pf_map, goal_x, goal_y, &path); * * if(path.is_valid) { * // success * } else { * // no path could be found * } why is pf_map even here? why isn't goal_x and goal_y in pf_parameter? * // fill parameter * should be explained what you're filling this with. I really need some examples for the cost functions or where to find such animals. * 1. It is useful to limit the expansion of unknown tiles with * the get_TB callback because the MC of unknown tiles is 0. not understandable. * 2. If there are two paths of the same cost to a tile (x,y), you are * not guaranteed to get the one with the least steps in it. If you care, * specifying EC to be 1 will do the job. why not for either point? EC is not explained at all. * 3. To prevent AI from thinking that it can pass through "chokepoints" * controlled by enemy cities, you should specify TB of each enemy city to * be TB_DONT_LEAVE. 'checkpoints' or 'waypoints' would be better here. also s/TB/the tile behavior/ * Maximal MC. If exceeded, tile is randed unreachable. ^branded sp. /* * Maximal number of steps in a path. */ #define MAX_PATH_LENGTH 200 this should be 363 (or dynamic which would be ideal) /* * The factor which is used to multiple TEC in the total_CC calculation. See * definition of total_CC above. */ #define PF_TURN_FACTOR 65536 this is a circular definition TB_DONT_LEAVE, /* Paths can lead _to_ such tile, * but are not allowed to go _through_ */ better to be named TB_CANT_LEAVE * No turn numbers or moves_left are used at all. The fields "turn" * and "moves_left" will always be -1. */ TM_NONE, "moves_left" being -1 means nothing to me in this context. /* * Callback to get MC of a move from (from_x, from_y) to * (to_x, to_y) and inthe direction dir. */ int (*get_MC) (int from_x, int from_y, enum direction8 dir, int to_x, int to_y, void *); sp. 'inthe' also enum direction8 dir as a parameter is confusing. It seems that either this is not needed, or two such parameters are. more explanation in the comment is certainly required. void *get_TB_data; /* Passed as last parameter to get_cost */ all three have this comment, are you sure? typedef struct pf_map *pf_map_t; I don't like this at all. It's very confusing, but considering... * over all paths. Does not perform any computations itself, just sets * everything up. */ pf_map_t pf_get_map(const struct pf_parameter *const parameter); hmm, don't like the name here. You're allocating memory, but the name of the function doesn't lead you to believe it. Better pf_create_map() and rename the internal function something else. the bevy of pf_next_*: the definitions of these are worrisome. I think it's because 'next' and 'last' aren't being used precisely. 'internal buffer' should be explained briefly as well. /* * Print the path via freelog(LOG_NORMAL,...) for debugging purposes. */ void pf_print_path(const struct pf_path *path); if you can use 1 line to comment, please do so instead of using 3. also loglevel should be passed as a parameter here. path_finding.c: typedef short index_t; typedef unsigned char utiny_t; unexplained why we require these. * ===================== Hidden structures ====================== */ perhaps "Internal structures" would be better? /* * The map structure itself. */ struct pf_map { the comment should explain its use (because path_finding.h surely doesn't explain why we don't need just a pf_parameter...). /* Establish the "known" status of node */ if (params->omniscience) { node->node_known_type = TILE_KNOWN; /* } else if (params->get_known) { node->node_known_type = params->get_known(x, y, params->owner); */ } else { what is this? /********************************************************************* After usage the map must be destroyed. *********************************************************************/ void pf_destroy_map(pf_map_t pf_map) pf_map->status isn't deallocated? pf_tools: /********************************************************************** Switch on one tile overlapping into the sea **********************************************************************/ void pft_fill_unit_overlap_param(struct pf_parameter *parameter, struct unit *punit) totally doesn't explain why this is needed or what it does. ********************************************************************** Fill general use parameters to defaults ***********************************************************************/ void pft_fill_default_parameter(struct pf_parameter *parameter) more stuff should be set. (like owner, moves_left_initially, etc)