-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Slightly rework pathfinding obstacle avoidance and handling #38056
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Also—not sure if it's out of scope of this PR but I can add it if it's needed—as far as I can tell we don't have a way to tell if we can damage something without actually uh, damaging it. This means the pathfinder will see a wall that has 200 health as something that can be beaten past with fists, even though the damage reduction will of course nullify any damage. This means that if there's a long enough chain of plain glass windows versus some standard walls, the AI will try to fruitlessly punch at the wall instead of going through the breakable windows. |
|
Looks like it resolves #36490 |
|
Does this PR patch NPCs pathfinding over tiny fans into space and dying? |
Not if the fan leads out onto e.g., a catwalk (like in the dev map); there's no differentiation in the breadcrumbs between a tile (that isn't a "space" tile) that has atmos versus one that doesn't, from what I can tell. For pathfinding ending up entering actual space, that tends to happen due to (I think) the steering overshooting the target tile. |
| SteeringObstacleStatus status; | ||
|
|
||
| // Breaking behaviours and the likes. | ||
| lock (_obstacles) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought this lock wasn't needed since for some reason I thought this ran on main loop, but it gets parallelized in NPCSteeringSystem's update... but with a MaxDegreeOfParallelism of 1, which is probably why I didn't notice any issues. That being said, I'm also not sure what the lock is meant to be protecting, so would like to know if/why this needs to come back.
| _doorQuery = GetEntityQuery<DoorComponent>(); | ||
| _climbableQuery = GetEntityQuery<ClimbableComponent>(); | ||
| _destructibleQuery = GetEntityQuery<DestructibleComponent>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up putting these queries here, even though only the destructible query is used in more than one place. I'm still not entirely sure how cached entity queries work and where you /should/ initialize them, if that even matters. (Sometimes I see them initialized in an update loop, other times in a system's initialize.)
| } | ||
| } | ||
| // Interacts are bit nicer than bumps, so try interacting regardless | ||
| _interaction.InteractionActivate(uid, ent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still only handles bumps by kinda just... treating doors that are bumpable and accessible as a non-obstacle and hoping that the weights and speed of the NPC work out such that it bumps it. This is super obvious with slimes which will still do a bit of the back and forth wiggle when they see someone behind a door, but in testing they seem to consistently open it after a few tries.
| var obstacleEnts = _mapSystem.GetLocalAnchoredEntities(poly.GraphUid, grid, poly.Box).ToHashSet(); | ||
| FilterObstacleEntities((uid, component), mask, layer, obstacleEnts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this isn't the most performant way of going about this (though in profiling I haven't noticed any issues, though I just realized I dunno about memory usage), and would like to know what would be better if necessary.
|
In my testing this seems to cause turrets to continue shooting people who have rounded corners, wasting their ammo. |
|
@CrigCrag, I'm having trouble reproducing this. Can you show what your setup looks like? |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
About the PR
I semi-refactored how pathfinding obstacle avoidance works, specifically relating to how it handles doors (prying/bumps), vaulting, and brute force, and also adjusted some pathfinding weights to give slightly less stupid pathfinding choices when, for example, walled off. This also changes how multiple mobs will pathfind in situations that were previously "safe". As a notable example, slimes will now bum-rush doors if they see you, opening them if the doors were all access, and mobs that can interact will now check if a door is openable by them for their access level. Xenos especially are much, much more effective at chasing you down.
Also drive-by fix a thing for the route cost overlay since it wasn't working.
Why / Balance
For fun I was making a creepy plushie that could follow you and pry open doors and then noticed it wasn't prying open doors. Then I did this. Whoops.
Also realistic space aliens or something.
Fixes #36490, fixes #17764.
Technical details
There is now some degree of code shared between the danger weighting and the actual handling of these obstacles. I feel like... it could be better? I'm especially not happy with the duplication in logic between TryHandleFlags and FilterObstacleEntities, but I'd like a second opinion. Happy to rework things as needed, though it seems from the comments that a greater refactor for pathfinding is in the works for at least some point. Some other notes that I would like advice on:
forceSteerinSteerseems to be true in every code path. Not sure where it was originally meant to be false, or if it's still needed.Media
Potato video because github file size limit.
https://github.com/user-attachments/assets/daeb16f0-e3a9-4a42-92f1-13059263700a
Requirements
Breaking changes
Changelog
🆑