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 Mobile a PausableConditionalTrait #16262

Merged
merged 1 commit into from Mar 7, 2019

Conversation

@tovl
Copy link
Contributor

commented Mar 3, 2019

Made Mobile into a PausableConditionalTrait. Split from and prerequisite to #16142.

@tovl tovl force-pushed the tovl:mobile-pausable branch from a57d47e to 2ceb2f5 Mar 3, 2019

@pchote
Copy link
Member

left a comment

Looking mostly good, just one big oversight and a couple of small ones to fix.

OpenRA.Mods.Common/Traits/Mobile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Mobile.cs Show resolved Hide resolved
@@ -190,7 +190,7 @@ public override Activity Tick(Actor self)
if (IsCanceled && self.Location.Layer != CustomMovementLayerType.Tunnel)
return NextActivity;

if (mobile.IsTraitDisabled)
if (mobile.IsTraitDisabled || mobile.IsTraitPaused)

This comment has been minimized.

Copy link
@pchote

pchote Mar 3, 2019

Member

Does line 376 below need this too?

This comment has been minimized.

Copy link
@tovl

tovl Mar 4, 2019

Author Contributor

I'm not sure. It depends on what the desired behaviour is. Should for instance EMPed units freeze immediately when in between cells or should the unit always 'coast' to the next cell? I think you can argue either way.

@tovl tovl force-pushed the tovl:mobile-pausable branch 2 times, most recently from 47f764c to 9b31c20 Mar 4, 2019

@pchote
Copy link
Member

left a comment

A couple of minor final nits, then LGTM

OpenRA.Mods.Common/OpenRA.Mods.Common.csproj Outdated Show resolved Hide resolved
mods/d2k/rules/vehicles.yaml Outdated Show resolved Hide resolved

@tovl tovl force-pushed the tovl:mobile-pausable branch from 9f99fc8 to 53c643d Mar 6, 2019

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2019

The update since @pchote's review was minor, so I consider that 👍 still valid. LGTM.

@reaperrr reaperrr merged commit 2e5e7c2 into OpenRA:bleed Mar 7, 2019

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:mobile-pausable branch Mar 7, 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.