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

Discourage harvesters from wandering too far from the refinery. #16935

Merged
merged 1 commit into from Aug 20, 2019

Conversation

@pchote
Copy link
Member

commented Aug 11, 2019

One of the easier to address complaints about the "new" harvester behaviour is that they still tend to prefer to harvest in straight lines, moving much further from the refinery and therefore taking longer than necessary to complete each harvesting run.

This PR adds a cost penalty to the cell being harvested so the pathfinder should prefer cells that are closer to the refinery.

@pchote pchote added this to the Next Release milestone Aug 11, 2019

@tovl

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2019

This doesn't quite work yet. You have to add the goal location as an extra pathing waypoint. See 073bcf7 which seems to work.

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2019

Can you explain why that is needed and how it helps? This seemed to work as intended during my testing - the goal wasn't to select the ore closest to the refinery, but for the otherwise equivalent options for the harvester to prefer the one in the direction of the refinery.

@tovl

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2019

Ok, I've looked some more at the pathfinding code and it turns out my previous suggestion is not really the right solution. The real problem with your first attempt was that the cost just wasn't steep enough. The custom cost is an extra cost on top of the regular movement cost and other considerations such as path smoothing so you have to make sure the custom cost is at least in that order of magnitude.

5b83e17 gives the result you want. The cost here might be a bit overkill but it works. For reference: base cost (before path smoothing) to enter cell with 100% movement speed is 10, 50% is 20.

Also, a side-effect of the way it is calculated now is that the directional preference becomes more dominant the farther it is from the refinery, but that might actually be a good thing.

@GraionDilach

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

I'd like to note that the Westwood games also have the line-harvesting behaviour.

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

I'm not really happy with the cost modifier being based on the distance from the refinery (a problem with both of my implementations so far, as well as @tovl's suggestion) - this is really a directional effect, so should be based on angles instead of distance.

I'm planning to rework this to calculate the angle between the harvester-cell and cell-refinery lines, then the modifier is just <base modifier> * cos(angle). If the angle is 180 degrees (i.e. all in a straight line) the maximum bonus is applied. If the angle is 0 degrees (i.e. the harvester has to drive away from the refinery) the maximum penalty is applied. The base modifier can be exposed to yaml so modders will be able to tweak the magnitude of the effect or disable it completely.

@pchote pchote force-pushed the pchote:fix-line-harvesting branch from 4fd0bc2 to 474c912 Aug 17, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2019

Fixed.

@pchote pchote force-pushed the pchote:fix-line-harvesting branch from 474c912 to 240fd17 Aug 18, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

Updated to make the cost modifier vary between 0-max, redefined the modifier as the total range instead of half, and doubled the default value to keep the previous behaviour.

@tovl
Copy link
Contributor

left a comment

Works.

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

This triggers a crash (reported on Discord):

Exception of type `System.DivideByZeroException`: Forsøkte å dele med null.
   ved OpenRA.Mods.Common.Activities.FindAndDeliverResources.<>c__DisplayClass17_0.<ClosestHarvestablePos>b__1(CPos loc)
   ved OpenRA.Mods.Common.Pathfinder.PathGraph.CalculateCellCost(CPos neighborCPos, CVec direction, Int32 movementCost)
   ved OpenRA.Mods.Common.Pathfinder.PathGraph.GetCostToNode(CPos destNode, CVec direction)
   ved OpenRA.Mods.Common.Pathfinder.PathGraph.GetConnections(CPos position)
   ved OpenRA.Mods.Common.Pathfinder.PathSearch.Expand()
   ved OpenRA.Mods.Common.Traits.PathFinder.FindPath(IPathSearch search)
   ved OpenRA.Mods.Common.Pathfinder.PathFinderUnitPathCacheDecorator.FindPath(IPathSearch search)
   ved OpenRA.Mods.Common.Activities.FindAndDeliverResources.ClosestHarvestablePos(Actor self)
   ved OpenRA.Mods.Common.Activities.FindAndDeliverResources.Tick(Actor self)
   ved OpenRA.Activities.Activity.TickOuter(Actor self)
   ved OpenRA.Traits.ActivityUtils.RunActivity(Actor self, Activity act)
   ved OpenRA.Actor.Tick()
   ved OpenRA.World.Tick()
   ved OpenRA.Game.InnerLogicTick(OrderManager orderManager)
   ved OpenRA.Game.LogicTick()
   ved OpenRA.Game.Loop()
   ved OpenRA.Game.Run()
   ved OpenRA.Game.InitializeAndRun(String[] args)
   ved OpenRA.WindowsLauncher.RunGame(String[] args)

@pchote pchote force-pushed the pchote:fix-line-harvesting branch from 240fd17 to 9b2bba8 Aug 18, 2019

@pchote pchote added the PR: Needs +2 label Aug 18, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

Rebased and fixed the crash by adding non-zero checks to bSq and cSq.
These haven't appreciably changed the code so @tovls review still stands.

@teinarss teinarss merged commit ab94ea9 into OpenRA:bleed Aug 20, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pchote pchote deleted the pchote:fix-line-harvesting branch Aug 26, 2019

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