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

Allow minelaying to be queued #16065

Merged
merged 1 commit into from Mar 13, 2019

Conversation

@tovl
Copy link
Contributor

commented Jan 15, 2019

Fixes #11582.

These actions can now be queued by holding the shift key:

  • Demotruck detonation via deployment EDIT: Superseded by #16101.
  • Aircraft returning to base via deployment EDIT: split into #16207.
  • minelayers laying a mine via deployment
  • minelayers laying a minefield using the Force Attack interface

The first two are only a few simple lines.
The changes to the minelayer also allow multiple minefields to be Ordered in a queue. To do so, the LayMines activity now has an extra argument passed to it for the locations of the minefield. Before, the activity code got this from the Minelayer trait at the moment the activity was executed so only the last minefield was remembered.

@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 16, 2019

moved further refactoring of the demotruck behaviour over to this PR from PR #16069

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 2639ebd to 40ee9d0 Jan 16, 2019

@pchote

This comment has been minimized.

Copy link
Member

commented Jan 19, 2019

The fix for #16061 requires a KillSelf-like activity, so it makes sense to merge this for the playtest as a prereq.

@pchote pchote added this to the Next Release milestone Jan 19, 2019

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 40ee9d0 to 92aaf4e Jan 20, 2019

@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2019

Now that PR #16101 supersedes the changes to AttackSuicides in this PR, should I Remove that part?

@tovl tovl closed this Jan 24, 2019

@tovl tovl reopened this Jan 24, 2019

@pchote pchote removed the PR: For stable label Jan 25, 2019

@pchote pchote removed this from the Next Release milestone Jan 25, 2019

@pchote

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

In the end we don't need this for the playtest, so lets leave this and #16060 for Next + 1.

@pchote pchote referenced this pull request Jan 27, 2019
@matjaeck matjaeck referenced this pull request Jan 28, 2019

@tovl tovl force-pushed the tovl:queue-all-deploy branch 3 times, most recently from b66e148 to 8c53209 Feb 10, 2019

@Punsho

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Demo trucks can already be deployable because of #16101

@reaperrr reaperrr changed the title Allow demotruck deploy, aircraft ReturnToBase and minelaying to be queued Allow aircraft ReturnToBase and minelaying to be queued Feb 15, 2019

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

@tovl Would you mind splitting the RTB commit (with the style fix I suggested) to a separate PR?
That one is a no-brainer that won't need much more reviewing/testing, and I have some Aircraft improvements of my own in the pipeline, so it would be good to get that merged as quickly as possible.

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 8c53209 to 30ebbb3 Feb 15, 2019

@tovl tovl changed the title Allow aircraft ReturnToBase and minelaying to be queued Allow minelaying to be queued Feb 15, 2019

@tovl tovl force-pushed the tovl:queue-all-deploy branch 2 times, most recently from 979e7d9 to 11e844f Feb 21, 2019

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 11e844f to 247a3cf Mar 3, 2019

@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2019

LayMines now uses Childactivities as well. Rebased onto #16246.

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 247a3cf to 8148abb Mar 4, 2019

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 8148abb to 9a6f14f Mar 9, 2019

@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Mar 9, 2019

Rebased.

@pchote pchote removed the PR: Rebase me! label Mar 9, 2019

@pchote

This comment has been minimized.

Copy link
Member

commented Mar 9, 2019

.fuse_hidden files again

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 9a6f14f to 9692cdc Mar 10, 2019

@@ -99,8 +99,7 @@ void IResolveOrder.ResolveOrder(Actor self, Order order)
{
minefieldStart = cell;
Minefield = new[] { cell };

This comment has been minimized.

Copy link
@pchote

pchote Mar 10, 2019

Member

This stomps over state needed for any already-queued PlaceMine order.

Repro case: queue up move, deploy, move, deploy, move, deploy - watch unit run around not knowing what to do.

I guess that this trait-level array will need to be removed, and the minefield cells pulled from the activity instead.

This comment has been minimized.

Copy link
@tovl

tovl Mar 10, 2019

Author Contributor

This is actually a recent regression due to some refactoring. It worked before, but I kinda forgot why I did what I did two months ago.

This doesn't actually overwrite the state for an already queued order; It just orders the mines on the cell that the minelayer is at the moment the queued order is placed instead of where it will be when the activity comes up.

It should now be fixed.

@tovl tovl force-pushed the tovl:queue-all-deploy branch from 9692cdc to 5c5d314 Mar 10, 2019

@reaperrr reaperrr merged commit da2e56e into OpenRA:bleed Mar 13, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.