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

Rework paradrop logic to be more robust. #16564

Merged
merged 1 commit into from May 31, 2019

Conversation

@pchote
Copy link
Member

commented May 19, 2019

The current paradrop logic works by having the aircraft blindly insert the actor into the world, relying on the Parachute activity to set the cell and position. The actor doesn't have a previously set position, so appears for one tick (before Parachute runs) at 0,0. I don't have a repro case for #9750/#15520 but i'm reasonably confident that these happen because something cancels Parachute before it has a chance to tick.

This PR fixes the mess by making the aircraft insert the unit directly into the desired cell, and now correctly accounts for sub-cell positions. Multiple units can now be dropped in the same cell, with a new DropInterval parameter controlling how spaced out they should be.

Fixes #3409
Fixes #9750
Fixes #15520

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

var dropActor = cargo.Peek(self);
var dropPositionable = dropActor.TraitOrDefault<IPositionable>();
var dropCell = self.Location;
var dropSubCell = dropPositionable.GetAvailableSubCell(dropCell);

This comment has been minimized.

Copy link
@reaperrr

reaperrr May 24, 2019

Contributor

Won't this crash if dropPositionable is null? In which case you might as well drop the OrDefault above.

@reaperrr
Copy link
Contributor

left a comment

Infantry spacing of RA paradrop looks a bit weird now

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

Also needs a rebase.

@pchote pchote force-pushed the pchote:fix-paradrop-logic branch from c00bb87 to 16dfe14 May 24, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

Fixed and rebased.

Infantry spacing of RA paradrop looks a bit weird now

This is a consequence of using subcells for positions, instead of the middle of each cell. I'm not sure there is much we can do about this.

@matjaeck
Copy link
Contributor

left a comment

Tested in RA and LGTM. I couldn't reproduce #15520 on 20190314.

@matjaeck

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

Infantry spacing of RA paradrop looks a bit weird now.

Not a problem IMO.

@Punsho
Copy link
Contributor

left a comment

This doesn't fully fix the corner bug. On bleed & release if "status bar" option is set to "always on" you are able to see health bars of soldiers in the corner. In this pr it is not the case, but infantry still provide vision for a tick in the corner

@matjaeck

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

infantry still provide vision for a tick in the corner

I think we can live with this.

@pchote pchote force-pushed the pchote:fix-paradrop-logic branch from 16dfe14 to d68bacb May 26, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 26, 2019

Fixed.

@Punsho

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

I often get that ra paradrop doesn't drop all 5 soldiers in a run if rubble is close. It's way worse then in the current release

@pchote pchote force-pushed the pchote:fix-paradrop-logic branch from d68bacb to 6e5783a May 26, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 26, 2019

I've halved the inter-drop delay, which should help with the spacing and missing soldier issues.

@matjaeck
Copy link
Contributor

left a comment

Spacing adjustments helped and LGTM.

@reaperrr reaperrr merged commit e2b2732 into OpenRA:bleed May 31, 2019

2 checks passed

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

@pchote pchote deleted the pchote:fix-paradrop-logic branch Aug 26, 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.