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

Fix transport being blocked by its own passengers. #16313

Merged
merged 1 commit into from Mar 26, 2019

Conversation

tovl
Copy link
Contributor

@tovl tovl commented Mar 15, 2019

When determining whether it can unload its cargo, a transport craft will now ignore transient actors. This prevents the UnloadCargo action aborting early due to some of its own passengers blocking the exits. This regression was introduced by #16060.

Fixes #16307 .

@@ -214,7 +214,7 @@ public bool CanUnload()
}

return !IsEmpty(self) && (aircraft == null || aircraft.CanLand(self.Location))
&& CurrentAdjacentCells != null && CurrentAdjacentCells.Any(c => Passengers.Any(p => p.Trait<IPositionable>().CanEnterCell(c)));
&& CurrentAdjacentCells != null && CurrentAdjacentCells.Any(c => Passengers.Any(p => p.Trait<IPositionable>().CanEnterCell(c, null, false)));
Copy link
Member

@pchote pchote Mar 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will return the wrong result for cases like EjectOnDeath: true that need to know if it can unload an actor right now, without waiting for transient actors to move. This will lead to units being stacked on top of eachother in the same (sub)cell.

Adding a checkTransient or immediate or similar flag to CanUnload to distinguish these cases should help.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that pretty much everything else in the EjectOnDeath code is bogus, and so this issue already exists on bleed, and still exists after the changes here - see #16353 for details.

I can confirm that the change here does work after hacking around the other issues, which we shouldn't be trying to fix in this PR.

@obrakmann obrakmann merged commit 872bf73 into OpenRA:bleed Mar 26, 2019
@obrakmann
Copy link
Contributor

Changelog

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

Successfully merging this pull request may close these issues.

None yet

4 participants