Wargus / stratagus

The Stratagus strategy game engine
GNU General Public License v2.0
619 stars 119 forks source link

Pathfinder: Bad handling of failure results #610

Closed zzam closed 6 months ago

zzam commented 6 months ago

Negative results of AStarFindPath

    PF_FAILED = -3,       /// This Pathfinder failed, try another
    PF_UNREACHABLE = -2,  /// Unreachable stop
    PF_REACHED = -1,      /// Reached goal stop

lead to function NewPath storing negative values be stored in PathFinderOutput::Length output.Length = std::min<int>(i, PathFinderOutput::MAX_PATH_LENGTH); but PathFinderOutput::Length being uint8_t. So 253, 254 or 255 are stored. This itself is only harmful where the values are used:

Saved games contain e.g. these bad pathes: "pathfinder-output", {"path", {6, 7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 88, -12, -4, -1, 56, -76, -4, -1, 24, 96, -4, -1, 0, 0, -4, -1, 0, 0, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 0, 0, 0, -63, 0, 0, 0, 0, 0, 0, 0, 80, -1, 77, 16, 0, 0, 0, 0, 32, -32, -12, -1, 0, 0, -4, -1, 0, 0, -80, -1, -4, -108, 0, -1, 97, 45, 102, 97, 115, 116, 45, 102, 97, 115, 116, 45, 102, 101, 114, 116, 105, 103, 46, 115, 97, 118, 46, 103, 122, 0, 92, -1, 0, 4, 68, -1, -52, 72, 12, -1, -96, 40, 4, -1, 116, 20, 0, -1, 76, 4, 0, -1, 0, 0, 0, 0, 0, 0, 0, 0, 97, 0, 0, 0, 0, 0, 0, 0, 97, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 20, -116, -8, -1, 16, 96, -56, -1, 16, 60, -104, -1, 12, 32, 108, -1, 12, 72, -52, -1, 4, 40, -96, -1, 0, 20, 116, -1, 0, 4, 76, -1, -64, 62, -92, 16, 0, 0, 0, 0, 16, -81, -103, 15, 0, 0, 0, 0, 20, 64, 104, -1, 32, 88, 124, -1, 48, 112, -112, -1, 0, 0, 0, -1, -80, -59, -24, 15, 0, 0, 0, 0, -63, 0, 0, 0, 0, 0, 0, 0, 80, 0, 0, },"cycles", 32},

zzam commented 6 months ago

The load/store is fixed with https://github.com/Wargus/stratagus/pull/611