[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Update of patch #4643 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Follow-up Comment #5, patch #4643 (project freeciv): The code is full of function pointers, so I'm sure I'm missing something, but how and where is PF_IMPOSSIBLE_MOVE checked? I'm happy to remove it, I just don't understand why I may. Looking for pf_map_move_cost(), I could only find two examples, neither of which seemed to map directly to this situation (and in find_beachhead, I wonder if the ai is actively seeking PF_IMPOSSIBLE_MC with some of the conditionals (cost best_cost, etc.), or if pf_map_move_cost() can't return that. I used pft_fill_unit_attack_param() because that's the best match to the logic for the old checks. Land units only check if it's the same continent, so for classic land units, this should be the same as anywhere they can attack. Sea units check for it being a connected ocean OR a city OR being able to attack non-native, which is similar to the logic for sea attack moves. Note that goto_is_sane() is mostly used as a gating function: that it still doesn't promise that something is entirely possible isn't worse than before (and in a fuller optimisation, goto_is_sane() would probably go away entirely: the majority of callers use some sort of pathfinding anyway, so don't need it, and the remainder would be better served by a pathfinding convenience function: bool path_exists_for_unit_to_tile() or similar). ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Follow-up Comment #6, patch #4643 (project freeciv): The code is full of function pointers, so I'm sure I'm missing something, but how and where is PF_IMPOSSIBLE_MOVE checked? I'm happy to remove it, I just don't understand why I may. pf_map_move_cost(), pf_map_position() and pf_map_path() do about the same thing. Until the tile is not unreachable and not reached, the map is iterated. (See pf_[normal|danger|fuel]_map_iterate_until() functions.) The difference is what they return: * pf_map_move_cost() return the total movement cost to reach the tile, or PF_IMPOSSIBLE_MC if the tile is not reachable ; (See pf_[normal|danger|fuel]_map_move_cost() functions.) * pf_map_position() return TRUE iff the tile is reachable and then fill the position ; (See pf_[normal|danger|fuel]_map_position() functions.) * pf_map_path() return the complete path to reach the tile, or NULL is the tile cannot be reached. (See pf_[normal|danger|fuel]_map_path() functions.) and in find_beachhead, I wonder if the ai is actively seeking PF_IMPOSSIBLE_MC with some of the conditionals (cost best_cost, etc.), or if pf_map_move_cost() can't return that. Checking this, it seems erroneous. If I understand correctly, if the first tile iterated by adjc_iterate() is not reachable, the result will be wrong. ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Follow-up Comment #7, patch #4643 (project freeciv): Thanks. I'll review, and update the patch to be less redundant. For find_beachhead, there's that problem. Also, if the first tile *is* reachable, but the next tile in the iteration isn't, then the unreachable tile will be selected. ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Follow-up Comment #8, patch #4643 (project freeciv): Digging through, I think I like the pf_map_move_cost() based solution best. Updated patch attached. (file #20485) ___ Additional Item Attachment: File name: check-goto-sanity-with-map-move-cost.patch Size:8 KB ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Update of patch #4643 (project freeciv): Status:None = Ready For Test Assigned to:None = pepeto Planned Release: = 2.6.0 ___ Follow-up Comment #9: Looks ready for test! ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
URL: http://gna.org/patch/?4643 Summary: Check goto sanity with pathfinding Project: Freeciv Submitted by: persia Submitted on: Fri 04 Apr 2014 10:08:44 PM JST Category: ai Priority: 5 - Normal Status: None Privacy: Public Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: ___ Details: The current goto sanity check uses move_type as part of the determination of whether a unit can get from somewhere to somewhere else, along with some internal information (e.g. continent identifiers). This fares poorly with unit classes that have complex nativity (may be UMT_BOTH, but may not be able to cross deep oceans, or may be UMT_LAND, but unable to traverse mountain ranges). The attached patch replaces the heuristic sanity check with a test of whether there is a valid path according to the pathfinding algorithms. Attack moves are selected because goto_is_sane() is sometimes used in situations where the unit will want to attack at the destination. Note that although this reduces the number of uses of UMT_LAND, UMT_SEA, and UMT_BOTH in the code, it won't actually correctly test goto sanity for units with complex nativity until patch #3901 lands, although this may be applied independently, and the ordering of the patches is not important. ___ File Attachments: --- Date: Fri 04 Apr 2014 10:08:44 PM JST Name: check-goto-sanity-with-pathfinding.patch Size: 9kB By: persia http://gna.org/patch/download.php?file_id=20470 ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Follow-up Comment #1, patch #4643 (project freeciv): in goto_is_sane(): I would suggest the usage of pf_map_position() instead of pf_map_path() because we don't need to fill all positions of the path. Checking the returned value of pf_map_position() should be faster. ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Follow-up Comment #2, patch #4643 (project freeciv): pf_map_move_cost() != PF_IMPOSSIBLE_MC should also do the stuff... ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4643] Check goto sanity with pathfinding
Follow-up Comment #3, patch #4643 (project freeciv): Thanks for the hints. Indeed, pf_map_position() seems better. I used pos.total_MC, rather than pf_map_move_cost(), because that matches the examples I could find in the code, but I'm happy to switch that if it could be done better. Are there other ways this patch could be improved? (file #20479) ___ Additional Item Attachment: File name: check-goto-sanity-with-pathfinding+postion.patch Size:8 KB ___ Reply to this item at: http://gna.org/patch/?4643 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
