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

Remove default rally point from production structures #17625

Merged
merged 3 commits into from Feb 9, 2020

Conversation

@pchote
Copy link
Member

pchote commented Jan 26, 2020

This PR aims to pick up where #16701 left off, implementing the rally point behaviour from the original games (TS and later):

  • By default, structures will not display a rally point, and units will stop at the exit cell.
  • Rally points can be activated by clicking anywhere (we still have the rally point mouse cursor for discovery).
  • Rally points, once set, can't be cleared - it would have been nice if we could clear them by clicking on the structure, but this conflicts with the primary building order. None of the original games AFAIK offered a solution, so I think its fine that we don't either.

Mods can restore the previous behaviour by explicitly defining a Path on all production structures, and multiple offsets can be specified to provide a queued path.

This still suffers from the factory door problem raised in #16701, and this will need to be fixed before merging - I'm opening this now for discussion and testing (and so we can close #16701).

@pchote pchote added this to the Next+1 milestone Jan 26, 2020
@pchote pchote force-pushed the pchote:rally-point-polish branch from 8e9d4a0 to bd1e099 Jan 26, 2020
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Jan 26, 2020

As discussed on IRC: While fixing WithProductionDoorAnimation we should also see if we can fix the "door overlay loops when the damage state changes" bug. (If it is still present.)

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Jan 26, 2020

Actually this might not be caused by (only) the damage state change: #13489 and #9966

@reaperrr reaperrr mentioned this pull request Jan 27, 2020
@pchote pchote force-pushed the pchote:rally-point-polish branch from bd1e099 to 53fa0c4 Jan 27, 2020
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Jan 27, 2020

Updated to add the simple/quick-and-dirty door fix that @abcdefg30 suggested in IRC.

I wasn't able to reproduce #13489/#9966 so I suspect they were already accidentally fixed.

Copy link
Member

Mailaender left a comment

Works in-game also the AI seems fine with this.

@pchote pchote force-pushed the pchote:rally-point-polish branch from 53fa0c4 to ad79b5a Feb 1, 2020
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Feb 1, 2020

Added [Desc()].

if (rp.Actor.Owner != player)
continue;

var bi = rp.Actor.Info.TraitInfoOrDefault<BuildingInfo>();

This comment has been minimized.

Copy link
@abcdefg30

abcdefg30 Feb 9, 2020

Member

As discussed on IRC this should move inline again.

Copy link
Member

abcdefg30 left a comment

Lgtm otherwise.

@pchote pchote force-pushed the pchote:rally-point-polish branch from ad79b5a to 8f0fb8e Feb 9, 2020
@pchote pchote force-pushed the pchote:rally-point-polish branch from 8f0fb8e to a72aae9 Feb 9, 2020
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Feb 9, 2020

Rebased (first push) and fixed (second push).

@abcdefg30 abcdefg30 merged commit 585b8dc into OpenRA:bleed Feb 9, 2020
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
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Feb 9, 2020

@pchote pchote deleted the pchote:rally-point-polish branch Feb 15, 2020
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.

None yet

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