beyond-all-reason / spring

A powerful free cross-platform RTS game engine
https://beyond-all-reason.github.io/spring/
Other
190 stars 97 forks source link

Add directional pathing #1402

Closed marcushutchings closed 1 month ago

marcushutchings commented 4 months ago

Fixes https://github.com/beyond-all-reason/spring/issues/1398

Includes fixes from https://github.com/beyond-all-reason/spring/pull/1395 which Fixes: https://github.com/beyond-all-reason/spring/issues/1386

This change provides the capability to use directional pathing (HAFPS.) I.e. the result of a pathing query takes the direction of the search into account. So you can path down a hill you wouldn't be able to travel up for example.

This change also adds the capability to use shortest route path (QTPFS.) Normally paths are given according to the fastest route. Now it is possible to have units take the shorest route, which helps them follow the expected path of players better - for example, avoiding dangerous areas.

To use directional pathing:

  1. Ensure modrule: movement.allowDirectionalPathing = true, it is true by default.
  2. Use a compatible pathing system. Only HAPFS is compatible at the moment. system.pathFinderSystem = 0. it is 0 by default.
  3. Enable/disable allowDirectionalPathing field on any MoveDefs as you see fit. It is enabled by default.

Note: using an incompatible pathing system (like QTPFS) will disable directional pathing, regardless of other settings. Note: turning off allowDirectionalPathing in modrules will also disable directional pathing, regardless of other settings.

To use shortest route pathing:

  1. Ensure modrule: movement.preferShortestPath= true, it is false by default.
  2. Use a compatible pathing system. Only QTPFS is compatible at the moment. system.pathFinderSystem = 1. it is 0 by default.
  3. Enable/disable preferShortestPath field on any MoveDefs as you see fit. It is enabled by default.

Note: using an incompatible pathing system (like HAPFS) will disable shortest route pathing, regardless of other settings. Note: turning off preferShortestPath in modrules will also disable shortest route pathing, regardless of other settings.

Notes:

Using directional pathing has a performance COST on HAPFS. Using shortest route pathing has a performance BENEFIT on QTPFS.

marcushutchings commented 4 months ago

Sorry, I added the QTPFS stuff because the functional code changes is 3 lines. It was more effort to add the boilerplate code to enable the options!

GoogleFrog commented 4 months ago

This looks like it changes a bunch of behaviours by default. Also, precisely which settings are expensive?

marcushutchings commented 4 months ago

movement.allowDirectionalPathing = true while using HAPFS. The impact may be upto 2% depending on number of movedefs using it. You can disable it on a per moveDef if you want some to use it and some to not use it. You may not really notice the difference in terms of performance in practice.

sprunk commented 4 months ago

If both allowDirectionalPathing and preferShortestPath are now per-movedef then they shouldn't be modrules anymore. This is because it's confusing when there's two vars under the same name in different namespaces and one only works if the other is in a certain state. If somebody still wants a "global" switch then it's trivially done by putting this snippet in movedefs.lua:

for _, md in pairs(MoveDefs) do
  md.preferShortestPath = false
  md.allowDirectionalPathing = false
end
marcushutchings commented 4 months ago

If both allowDirectionalPathing and preferShortestPath are now per-movedef then they shouldn't be modrules anymore. This is because it's confusing when there's two vars under the same name in different namespaces and one only works if the other is in a certain state. If somebody still wants a "global" switch then it's trivially done by putting this snippet in movedefs.lua:

for _, md in pairs(MoveDefs) do
  md.preferShortestPath = false
  md.allowDirectionalPathing = false
end

That is a good point. I'll remove the modrules and I'll have the moveDefs default to false for both flags. That way current behaviour is preserved by default.