Skip to content
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

Make actors sharing a cell also share paths #17300

Open
reaperrr opened this issue Nov 1, 2019 · 4 comments

Comments

@reaperrr
Copy link
Contributor

@reaperrr reaperrr commented Nov 1, 2019

The idea is that if a move order/move-including order is given to an actor with a locomotor with SharesCell (usually infantry), the first actor on that cell could cache its path and let other actors on that cell with the same locomotor just re-use that path (if they're given an order towards the same destination on the same tick).

If this was implemented, burst pathfinding cost for armies of infantry could be reduced by up to 80% in RA/TD/D2k and up to 66% in TS.

I had dabbled around with it here: https://github.com/reaperrr/OpenRA/tree/inf-sharepath

Note that the above approach doesn't really seem to mesh well with how getPath works, though.
I'm mostly putting it here as reference, hoping someone with a better grasp of the path-finding/movement code might be able to rework it or at least use it as inspiration for something better.

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

@reaperrr reaperrr commented Nov 1, 2019

@pchote

This comment has been minimized.

Copy link
Member

@pchote pchote commented Nov 1, 2019

We do actually have this, in principle (see PathFinderUnitPathCacheDecorator), but I don't know if anybody has verified whether it actually works as intended within the last few years.

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

@reaperrr reaperrr commented Nov 1, 2019

Well, the performance gain on my old test branch (https://github.com/reaperrr/OpenRA/tree/inf-sharepath-old) - before the recent BlockedByActor changes broke compatibility with bleed - was as large as I'd expect it to be, so PathFinderUnitPathCacheDecorator doesn't seem to work as intended (or works differently than what I had in mind).

@teinarss

This comment has been minimized.

Copy link
Contributor

@teinarss teinarss commented Nov 2, 2019

We do actually have this, in principle (see PathFinderUnitPathCacheDecorator), but I don't know if anybody has verified whether it actually works as intended within the last few years.

It only works when we are not blocked by actors because otherwise we cant guarantee that the path is valid . So the question is what possible regression this new solution will bring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.