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 landing behaviour from force-move orders on selectable buildings. #17263

Merged
merged 1 commit into from Nov 2, 2019

Conversation

@pchote
Copy link
Member

pchote commented Oct 21, 2019

Fixes #17262.

Our definition for what constitutes a force move vs a force land is arbitrary to start with, so IMO it is perfectly reasonable to tweak that definition to be that force-move over a building† remains a move, and a force-move over anything else is a land.

† using "building" here from a player perspective, i.e. a player-built or civilian/tech structure, not trees or other actors that use the Building trait.

@Orb370

This comment has been minimized.

Copy link
Contributor

Orb370 commented Oct 21, 2019

Sounds good to me. Since force move was removed from helicopters, this will restore the behavior it was used for (hovering over enemy buildings to crash into them).

OrderID = "Land";
{
var building = bi.GetBuildingAt(location);
if (building == null || building.TraitOrDefault<Selectable>() == null || aircraft.CanLand(location, blockedByMobile: false))

This comment has been minimized.

Copy link
@pchote

pchote Oct 21, 2019

Author Member

Hinging this on Selectable may be too much of a hack. We could expose this as a flag on BuildingInfo instead?

This comment has been minimized.

Copy link
@GraionDilach

GraionDilach Oct 23, 2019

Contributor

What about a Footprint flag? Say, l and L for a landable x (since rest are obviously passable already).

This comment has been minimized.

Copy link
@pchote

pchote Oct 24, 2019

Author Member

This is an actor-level consideration, so I don't think it makes any sense to tie it to specific cells in the footprint.

Copy link
Contributor

matjaeck left a comment

LGTM and I'd welcome it if this could make it into next. There seem to be some conflicts with the prep-1908 branch however that need to be solved in this case.

Edit: User error.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Oct 27, 2019

It looks like we're going to need a new playtest anyway, so adding to the milestone.

@pchote pchote added this to the Next Release milestone Oct 27, 2019
@teinarss teinarss requested a review from abcdefg30 Nov 2, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Nov 2, 2019

I count two 👍s and no objections for a week, so I'll take this now and tag the playtest so @Baxxster can update the playtest servers.

@pchote pchote merged commit 27205b3 into OpenRA:bleed Nov 2, 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
@pchote

This comment has been minimized.

@pchote pchote deleted the pchote:fix-lmb-aircraft-forcemove branch Nov 2, 2019
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.