Page MenuHomeWildfire Games

Enable garrisoning on gates
Needs ReviewPublic

Authored by wraitii on Mar 27 2018, 2:43 AM.

Details

Reviewers
temple
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#2679
#5151
Summary

Units garrisoned on a gate shouldn't interfere with the opening and closing of it, or for example block large units from passing through the gate. So let's disable block movement and block pathfinding for garrisoned units and ignore them when deciding whether we can open or close the gate.

Test Plan

Agree that it's wanted and test that it works.
Using Iberia for now, other civs can be added in a later patch.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
D13_outtakes
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8327
Build 13596: Vulcan BuildJenkins
Build 13595: arc lint + arc unit

Event Timeline

temple created this revision.Mar 27 2018, 2:43 AM
temple edited the summary of this revision. (Show Details)
Stan added a comment.EditedMar 27 2018, 7:18 AM

Thanks so much for working on this.

Stan awarded a token.Mar 27 2018, 7:19 AM
temple added inline comments.May 1 2018, 5:32 PM
binaries/data/mods/public/simulation/components/Gate.js
53–54
wraitii added a reviewer: Restricted Owners Package.May 14 2018, 11:53 AM
Imarok added a subscriber: Imarok.May 14 2018, 1:04 PM
Imarok added inline comments.
binaries/data/mods/public/simulation/components/Gate.js
53–54

There is a ticket now: #5151

wraitii requested changes to this revision.May 14 2018, 1:37 PM
wraitii added a subscriber: wraitii.

Nice bug.

My input:

  • This happens because we don't move visible-garrisoned units out of the world. This makes sense if these are somewhat independent, for example in the case of autonomous drones and so on.
  • It should probably be a GarrisonHolder property whether garrisoning removes your "collision" or not.

Thus the correct way to fix this, imo, is to add a parameter to GarrisonHolder (and other components that have the same behaviour), instead of changing Gate. We may want buildings where visible garrison points are obstructions.

On the precise mechanism, calling the pathfinder seems reasonable enough.

This revision now requires changes to proceed.May 14 2018, 1:37 PM
wraitii added inline comments.May 14 2018, 1:52 PM
binaries/data/mods/public/simulation/components/Gate.js
149

This seems unrelated?

Read this better, there's two things here. On the "remove obstruction" part, see above.

These units should indeed not be counted towards opening the gate. I think however your current method won't work: say we have two garrisoned gates very close to each other, I think they'll get counted. The distinctive criteria here is that garrisoned units cannot move, they're static. I'm not entirely sure that's easily transatable in code though, maybe we need to add a "CutMyLegsOff" to CmpUnitMotion like we have "moveOutOfWorld" for CmpPosition.

binaries/data/mods/public/simulation/components/Gate.js
149

Ah right no I understand now.

wraitii commandeered this revision.Jul 13 2019, 7:40 PM
wraitii edited reviewers, added: temple; removed: wraitii.
wraitii updated this revision to Diff 8862.Jul 13 2019, 7:43 PM

More involved fix involving four things:

  • Gates open for units blocking movement, not construction (my opinion is that this overall makes more sense too).
  • Gates open only if units nearby 'can move', a new unitAI state
  • UnitAI keeps track of whether it can move or not. Packing moves the entity to Immobile, garrisoning too.
  • Garrison holders remove the obstruction of garrisoned entities (this is the incidental fix for #5151)

Also use the opportunity to use this "AbleToMove" function in UnitAI elsewhere.
Regarding my above comment - if we want buildings where visible obstruction points _are_ obstructions, we'll handle that then.

This correctly handles the case of two gates close to each other with visible garrison units inside each of them, unlike the previous patch.

wraitii updated the Trac tickets for this revision.Jul 13 2019, 7:44 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/53/display/redirect