Differential D443
Unit Motion: Revert vertex pathfinder changes from 2015 or so Authored by wraitii on May 7 2017, 10:42 AM.
Details
This adds back the "domain" edges to the short/vertex pathfinder. Not having them leads to weird paths in a few edge cases, and this particularly breaks with the new group-walk manager I'm implementing for D13. I believe this is more or less a reversion to 2015 state. Order a few units to walk around with the pathfinder overlay applied and noticed the domain edges have appeared. Test pathing, should work.
Diff Detail
Event TimelineComment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/1049/ for more details.
Comment Actions I was just cleaning up my screenshots and saw an example of the unlocked but closed gate bug, where units keep trying to get around a wall that they can't actually get around. (It's the enemy's gate, so it won't open for this unit.) So the gate's probably acting the same as a wall of units, and what should happen in this case is the short path should say impossible to reach, thus get a bigger range or give up. But instead because there's no domain edges it paths around the wall, outside the domain. When that short path fails because the unit runs into the wall, he asks for another short path which repeats all of this in the other direction.   (Haven't actually tested that adding back the domain edges fixes that, it's just my reading of the code.) Comment Actions Yeah that's the bug alright. You can see the domain edges by using the pathfinder overlay, but it only keeps one path so your unit has to be the only one moving (kind of annoying). Comment Actions Right, sorry. I was thinking in a22 where the domain edges don't exist. I tried the patch, looks fine, and I agree we should be restricting the paths to inside the domain square.
Comment Actions Successful build - Chance fights ever on the side of the prudent. Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files... Comment Actions Unit motion has problems of course, but I think it's correct to use domain edges here. Comment Actions Given my past experiences with the pathfinder, I'd rather commit this along D13 once it's ready, instead of risking breaking the game weirdly/in new ways in the meantime, as I haven't tested it extensively without D13 (and current pathfinder works well enough that I don't want to lose time doing that.) Thanks @temple for the reviews. Comment Actions Went digging a bit. I think this was removed in the first place because large obstructoions / chains of large obstructions can make short paths fails, resulting in stuck units with svn, and it's better to act a bit wonky than to get stuck. The issue of 'units getting stuck when short paths can't be returned' was indeed fixed in D13, so I don't think this should be committed until the fundamentals of D13 are. Comment Actions Rebased. This works well with my other unitMotion patches, though it doesn't fix the "unit go to weird places instead of the shortest path" problem which I thought it might :/ Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/1431/display/redirect Comment Actions It's actually a revert of rP17224, which is in 2015 :p . The original issue should not be happening with the latest UM changes, since the long-range pathfinder will correctly be called in case of pathing failures. | ||||||||||||||||||||||||||||||