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 nudger and locomotor cache aware of temporary immovable units. #17131

Merged
merged 3 commits into from Oct 28, 2019

Conversation

@tovl
Copy link
Contributor

tovl commented Sep 19, 2019

Follow-up to #17114

Fixes #17117.

Even though deployed units cannot be nudged, they were still regarded as movable for the purpose of pathfinding and cascading nudges. This PR changes units with the mobile trait disabled or paused to be regarded as immovable.

@tovl tovl force-pushed the tovl:nudgedeploy branch from 60ed20c to abc13b7 Sep 19, 2019
@tovl tovl force-pushed the tovl:nudgedeploy branch from abc13b7 to ccb3d3c Sep 20, 2019
@tovl tovl force-pushed the tovl:nudgedeploy branch 3 times, most recently from 2e3388e to f214cbb Oct 1, 2019
@tovl

This comment has been minimized.

Copy link
Contributor Author

tovl commented Oct 1, 2019

Update:
Divorced this logic from RequireForceMove so it also works correctly for deployable units that do not require alt to enable move orders.

@@ -53,6 +53,10 @@ public class MobileInfo : PausableConditionalTraitInfo, IMoveInfo, IPositionable
[Desc("Boolean expression defining the condition under which the regular (non-force) move cursor is disabled.")]
public readonly BooleanExpression RequireForceMoveCondition = null;

[ConsumedConditionReference]
[Desc("Boolean expression defining the condition under which this actor cannot be nudged by other actors.")]
public readonly BooleanExpression ImmovableCondition = null;

This comment has been minimized.

Copy link
@pchote

pchote Oct 5, 2019

Member

In some places we use Immobile and others Immovable. It would be good if we could standardize on one or the other (my preference would be Immobile).

This comment has been minimized.

Copy link
@tovl

tovl Oct 5, 2019

Author Contributor

The name was chosen to be consistent with the usage in BlockedByActor.Immovable, CellFlag.HasMovableActor and CellCache.Immovable among others. Those names where deliberately chosen to express a subtle difference in meaning with immobile: These categories also include actors that are quite mobile, but aren't movable by others (such as enemy units).

Immobile is actually only used as the name of a trait and nowhere else.

Copy link

ghost left a comment

Fixes #17117 and I didn't notice any regressions. I have also run some AI games with multiple clients without desyncing.

Copy link
Contributor

reaperrr left a comment

Apart from those style issues, code looks good to me

@tovl tovl dismissed stale reviews from reaperrr and ghost via d57b3fe Oct 17, 2019
@tovl tovl force-pushed the tovl:nudgedeploy branch from 009f862 to d57b3fe Oct 17, 2019
OpenRA.Mods.Common/Traits/Mobile.cs Outdated Show resolved Hide resolved
@tovl tovl force-pushed the tovl:nudgedeploy branch from d57b3fe to 3b5f646 Oct 28, 2019
@teinarss teinarss merged commit d299124 into OpenRA:bleed Oct 28, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tovl tovl deleted the tovl:nudgedeploy branch Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.