Pathfinding Improvements by Causeless · Pull Request #444 · cortex-command-community/Cortex-Command-Community-Project-Source

Causeless · 2022-11-02T11:09:44Z

If a node has cost in any direction, that cost becomes spread a little out and averaged out. This encourages the AI to avoid hard surfaces, for example when jetpacking around. Likely needs some Lua script enhancements to get the full advantage, however.
Actors now have a much higher default diggability, even without digging tools. This means that actors will be far less likely to avoid soft debris (like body parts), as they'll be able to push though it.
Use a separate pathfinder per team. This allows us to do team-specific pathfinding (avoiding doors etc) without requiring that we constantly update the pathing node costs for each actor.
Improvements to brain placement detection. Now the brain path is no longer blocked by light debris or it's own team's doors.
Added change detection to the pathfinder. If the change to the node costs isn't significant (below some sentinel value), we ignore them. This allows us to skip doing the team-specific pathing updates in this case, and also means we don't need to Reset() MicroPather's cache unless things have changed
No longer do intermittent two-minute resets of the the entire pathfinding grid. This seems unnecessary and expensive, the partial updating should handle it fine.
Fixed issue where the AI woudn't pathfind around enemy doors (Actor AI refuses to pathfind around enemy doors #396)
Throttled AI to only update every other frame by default. Will likely tweak this value as I further improve and look at the AI, but this halves the time spent in AI script and seems to have no significant regressions in AI behaviour.

… different values Added test calculation to allocate different number of blocks based on map size. It causes slowdown though.

…caching

… doors, even when closed)

garethyr

Good work!
Various little cleanups and changes to do

garethyr · 2022-11-03T15:06:14Z

@@ -2891,7 +2905,25 @@ void Scene::ResetPathFinding()

 void Scene::UpdatePathFinding()


Assuming it's not too unwieldy, I'd rather have the method comment explain what's going on in a bit more detail, rather than all these inline comments.

I.e. explain that the method updates the shared pathfinder and, if there are changes, updates the individual ones, ensuring that doors for each team are ignored.

In this case I'd prefer that they're inline. Externally, the only thing that a user of the function needs to know is that this recalculates outdated pathing data - how it does that in this case (using several pathfinders and swapping around door materials and all that crap) are entirely internal implementation details that could change. The function contract is that this recalculates outdated pathing data, the details of how shouldn't matter.

Furthermore, having the comments be next to the actual lines of code means that if or when things do change, it's easier to remember to update them.

Hmmm, I don't really agree. Inline comments are valuable to explain

Why a seemingly odd decision was made

What some piece of unclear, complicated (or math-heavy) code is doing

If you need them for stuff other than that, things need to be renamed and fiddled with to make the code clearer, imo.

For the most part, these comments don't really do either of those 2 things. They're not as useless as some dancomments that would just, for example, say something like:

// Override door material for team g_MovableMan.OverrideMaterialDoors(true, team);

But they're also mostly explaining pretty clear things.
I guess the only part of it that's at all non-obvious is the shared pathfinder being NoTeam, and I'd rather that be more of a note that the shared pathfinder uses NoTeam, instead of basically saying "RecalculateAreaCosts updates the pathfinder". Also, I think the shared pathfinder using NoTeam is explained in other places anyway, right?

I think that the overriding material doors being used for team-specific pathing is not obvious. I didn't understand it initially, so that's why I'm being explicit.

That's an easy solve though isn't it - this method exists solely for this purpose, so you can just rename it to be more clear.
Heck, I don't even think the method really belongs in movableman, it's just there cause that's where Data put it. It'd be perfectly reasonable as a private Scene method imo - EraseOrRedrawDoorTerrainLayerForTeamPathfindinging or something like that.

…w ini fields added.

…r update.

Changing max to std::max in AHuman Tweaking formatting, whitespace, etc. in areas that were touched by existing changes Fixing up a few method comments Killing a few unnecessary inline comments, and tweaking others Other stuff

garethyr

Great work!

# Conflicts: # CHANGELOG.md - no real conflicts, solved manually

…en AI ticks, not necessarily true passed time) but that's an implementation detail.

…/cortex-command-community/Cortex-Command-Community-Project-Source into experimental-pathfinding-changes

MaximDude

Formatting junk for the most part

Made Scene::GetPathfinder private and moved its definition into header Made AIUpdateInterval lua property readwrite Fiddled with SettingsMan pathfinder grid node size stuff Fixed AIDeltaTime stuff in timerman Fixed region in controller Made magic number 8 a static constexpr in pathfinder, reorganized and made protected methods private

The comment doesn't make sense as a method remark, it's an internal detail, not related to actually using the method. Also tweaked the member variable comment for m_pPathFidners so it has a bit more detail

garethyr and others added 13 commits November 1, 2022 11:34

Made pathfinder grid node size readable from settings.ini to test out…

ca1ea4d

… different values Added test calculation to allocate different number of blocks based on map size. It causes slowdown though.

Forgot to stage a line

46e7979

Made pathfinder nodes have 8 neighbours instead of 6, and turned off …

f247b8b

…caching

Radiate costs out when pathing. This encourages avoiding hard objects

017ffaa

Allow AHuman to dig with default strength

e816d18

Some formatting changes

2605378

Replaced FLT_MAX with std::numeric_limits

0f1b769

ACrab now paths through light debris

03703ae

Spelling fixes

5d4e20f

Per-team pathfinders

901e464

Whoops...

58a15c9

Re-enabled caching on MicroPather

3e0ab9c

Brain path from orbit can now dig through light debris (and their own…

37be405

… doors, even when closed)

garethyr suggested changes Nov 3, 2022

View reviewed changes

garethyr requested a review from MaximDude November 3, 2022 18:57

garethyr and others added 4 commits November 4, 2022 15:50

Merge branch 'development' into experimental-pathfinding-changes

02e8a2a

Review feedback

3a16398

More review feedback.

549d110

Whoops. That's a nasty one

6b44471

garethyr reviewed Nov 6, 2022

View reviewed changes

Comment thread System/Constants.h Outdated

garethyr reviewed Nov 6, 2022

View reviewed changes

Comment thread Entities/Scene.cpp Outdated

Causeless added 4 commits November 6, 2022 16:58

Doxygen comment

7cffbf6

Removed auto

f614bdd

Changelog update - need Gacyr to add the specifics about the other ne…

fa5a41e

…w ini fields added.

Fixed issue where pathfinding ignored enemy doors.

b4183a5

Causeless linked an issue Nov 8, 2022 that may be closed by this pull request

Actor AI refuses to pathfind around enemy doors #396

Closed

Causeless added 4 commits November 11, 2022 11:23

Added lua bindings for Base Dig Strength

62ab267

Updated changelog, as per feedback

3768b01

Fixed missing const

bf735fd

Lua bindings for SceneMan:CastMaxStrengthRay()

b465b83

Causeless and others added 10 commits November 11, 2022 16:06

Added support for throttling AI, and defaulted to updating every othe…

a8d2f81

…r update.

Accessors for AI deltatime

e28c410

Renamed field to be more clear

0f67b2e

Feedback changes

b0b7723

Expose AIUpdateInterval to Lua

d0b88d0

Review feedback

f734926

CHANGELOG

996bf0a

changelog

c7a1f2f

Added a TODO

9503452

Cleanups in various places

9d124fe

Changing max to std::max in AHuman Tweaking formatting, whitespace, etc. in areas that were touched by existing changes Fixing up a few method comments Killing a few unnecessary inline comments, and tweaking others Other stuff

garethyr previously approved these changes Nov 17, 2022

View reviewed changes

garethyr dismissed their stale review via 1a47f2f November 17, 2022 05:28

garethyr force-pushed the experimental-pathfinding-changes branch from f198582 to 9d124fe Compare November 17, 2022 05:36

garethyr and others added 7 commits November 17, 2022 01:37

Fixed a typo in the changelog

bda21d1

Merge branch 'development' into experimental-pathfinding-changes

76b66c9

# Conflicts: # CHANGELOG.md - no real conflicts, solved manually

Merge branch 'development' into experimental-pathfinding-changes

ec88507

Updated changelog. Technically a lie (it gives the average time betwe…

6d1bdee

…en AI ticks, not necessarily true passed time) but that's an implementation detail.

Merge branch 'experimental-pathfinding-changes' of https://github.com…

bd7103a

…/cortex-command-community/Cortex-Command-Community-Project-Source into experimental-pathfinding-changes

Bah.

80275d7

li'l changelog wording/formatting tweaks

7ee5813

MaximDude suggested changes Nov 20, 2022

View reviewed changes

Causeless and others added 4 commits November 23, 2022 20:12

Review feedback

6611c83

Moving Scene::GetPathfinder definition back into cpp file

008b97a

The comment doesn't make sense as a method remark, it's an internal detail, not related to actually using the method. Also tweaked the member variable comment for m_pPathFidners so it has a bit more detail

Minor formatting and misc changes

f6bc2e6

garethyr approved these changes Nov 24, 2022

View reviewed changes

Merge branch 'development' into experimental-pathfinding-changes

51f3783

garethyr requested a review from MaximDude November 24, 2022 22:33

garethyr merged commit f6571ff into development Nov 24, 2022

garethyr deleted the experimental-pathfinding-changes branch November 24, 2022 22:34

		@@ -2891,7 +2905,25 @@ void Scene::ResetPathFinding()

		void Scene::UpdatePathFinding()

Conversation

Causeless commented Nov 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garethyr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

garethyr Nov 3, 2022

Choose a reason for hiding this comment

Uh oh!

Causeless Nov 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

garethyr Nov 5, 2022

Choose a reason for hiding this comment

Uh oh!

Causeless Nov 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

garethyr Nov 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

garethyr left a comment

Choose a reason for hiding this comment

Uh oh!

MaximDude left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Causeless commented Nov 2, 2022 •

edited

Loading

Causeless Nov 5, 2022 •

edited

Loading

Causeless Nov 6, 2022 •

edited

Loading