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

Add LandOnCondition to the trait Aircraft which triggers a landing and prevents takeoffs while the condition is met #13434

Merged
merged 1 commit into from Aug 8, 2017

Conversation

Projects
None yet
7 participants
@jrb0001
Contributor

jrb0001 commented Jun 2, 2017

No description provided.

@chrisforbes

Looks like it does what it says on the tin.. need overall design feedback from @pchote though.

@chrisforbes chrisforbes requested a review from pchote Jun 3, 2017

jrb0001 added a commit to DoGyAUT/OpenRA that referenced this pull request Jun 3, 2017

@atlimit8

Nothing major, mostly nits.

@jrb0001

This comment has been minimized.

Show comment
Hide comment
@jrb0001

jrb0001 Jul 4, 2017

Contributor

This PR seems to cause a take-off of all helis on map start. Planes are not affected. Because the map editor doesn't allow placing flying aircrafts, it can actually be useful. But it should be the same for all aircrafts IMO. Which one is the expected behavior?

Contributor

jrb0001 commented Jul 4, 2017

This PR seems to cause a take-off of all helis on map start. Planes are not affected. Because the map editor doesn't allow placing flying aircrafts, it can actually be useful. But it should be the same for all aircrafts IMO. Which one is the expected behavior?

@atlimit8

The merge commit means you probably used git merge or git pull instead of git rebase which is expected for a clean history. Using an interactive rebase (git rebase -i) is also expected for clean changes to the history; the changes from the fix commit should be amended to their respective commits.

Lastly, the helicopter problem should be handled.

@jrb0001

This comment has been minimized.

Show comment
Hide comment
@jrb0001

jrb0001 Jul 4, 2017

Contributor

Lastly, the helicopter problem should be handled. problem

And how should it be handled? Preventing the take-off on map start? Do a take-off for planes too?

Contributor

jrb0001 commented Jul 4, 2017

Lastly, the helicopter problem should be handled. problem

And how should it be handled? Preventing the take-off on map start? Do a take-off for planes too?

@GraionDilach

This comment has been minimized.

Show comment
Hide comment
@GraionDilach

GraionDilach Jul 5, 2017

Contributor

I'd prevent the take-off, because that's what current bleed does and that's what TS/RA2 also did for sure.

Contributor

GraionDilach commented Jul 5, 2017

I'd prevent the take-off, because that's what current bleed does and that's what TS/RA2 also did for sure.

@MunWolf

This comment has been minimized.

Show comment
Hide comment
@MunWolf

MunWolf Jul 7, 2017

Contributor

I vote for preventing take-off, it is easier to force a take-off with scripts but harder to prevent it.

Contributor

MunWolf commented Jul 7, 2017

I vote for preventing take-off, it is easier to force a take-off with scripts but harder to prevent it.

@jrb0001

This comment has been minimized.

Show comment
Hide comment
@jrb0001

jrb0001 Jul 12, 2017

Contributor

Fixed takeoff on map start and rebased.

Contributor

jrb0001 commented Jul 12, 2017

Fixed takeoff on map start and rebased.

jrb0001 added a commit to DoGyAUT/OpenRA that referenced this pull request Jul 15, 2017

jrb0001 added a commit to DoGyAUT/OpenRA that referenced this pull request Jul 15, 2017

jrb0001 added a commit to DoGyAUT/OpenRA that referenced this pull request Jul 15, 2017

jrb0001 added a commit to DoGyAUT/OpenRA that referenced this pull request Jul 20, 2017

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Aug 8, 2017

Member

OpenRA.Mods.Common/Traits/Air/Aircraft.cs:L251: [SA1008] Invalid spacing around the opening parenthesis.

Member

abcdefg30 commented Aug 8, 2017

OpenRA.Mods.Common/Traits/Air/Aircraft.cs:L251: [SA1008] Invalid spacing around the opening parenthesis.

@jrb0001

This comment has been minimized.

Show comment
Hide comment
@jrb0001

jrb0001 Aug 8, 2017

Contributor

Looks like I forgot to commit & push the fix.

Contributor

jrb0001 commented Aug 8, 2017

Looks like I forgot to commit & push the fix.

@abcdefg30

Can you squash the commits, please?

Add LandOnCondition to the trait Aircraft which triggers a landing an…
…d prevents takeoffs while the condition is met
@jrb0001

This comment has been minimized.

Show comment
Hide comment
@jrb0001

jrb0001 Aug 8, 2017

Contributor

Rebased and squashed.

Contributor

jrb0001 commented Aug 8, 2017

Rebased and squashed.

Code changed

@atlimit8 atlimit8 merged commit 1d1802a into OpenRA:bleed Aug 8, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@atlimit8
Member

atlimit8 commented Aug 8, 2017

@jrb0001 jrb0001 deleted the jrb0001:landoncondition branch Aug 9, 2017

jrb0001 added a commit to DoGyAUT/OpenRA that referenced this pull request Aug 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment