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 Repairable not accounting for multiple ICallForTransport traits #14837

Merged
merged 1 commit into from May 6, 2018

Conversation

Projects
None yet
5 participants
@abcdefg30
Copy link
Member

abcdefg30 commented Feb 22, 2018

We use TraitsImplementing everywhere else, so it didn't make sense to me to use TraitOrDefault in Repairable (which would also crash with multiple ICallForTransport traits).


transport.RequestTransport(self, targetCell, nextActivity);
t.RequestTransport(self, targetCell, nextActivity);

This comment has been minimized.

@Unit158

Unit158 Feb 22, 2018

Contributor

Is this a possible race condition? I'm not super familiar with the transport code, but there was no possibility of two transports arriving on the same tick before.

@Unit158

This comment has been minimized.

Copy link
Contributor

Unit158 commented Feb 22, 2018

I'm not sure how to test this.

@reaperrr reaperrr added this to the Next + 1 milestone Mar 4, 2018

@@ -142,15 +142,20 @@ public Actor FindRepairBuilding(Actor self)

static void TryCallTransport(Actor self, Target target, Activity nextActivity)
{
var transport = self.TraitOrDefault<ICallForTransport>();

This comment has been minimized.

@pchote

pchote Apr 7, 2018

Member

This can simplify down to something like:

static void TryCallTransport(Actor self, Target target, Activity nextActivity)
{
	var delta = (self.CenterPosition - target.CenterPosition).LengthSquared;
	var targetCell = self.World.Map.CellContaining(target.CenterPosition);
	var transports = self.TraitsImplementing<ICallForTransport>()
		.Where(t => t.MinimumDistance.LengthSquared < delta);

	foreach (var t in transports)
		t.RequestTransport(self, targetCell, nextActivity);
}

@abcdefg30 abcdefg30 force-pushed the abcdefg30:repairRequest branch from d8e0992 to 09ab434 May 4, 2018

@abcdefg30

This comment has been minimized.

Copy link
Member Author

abcdefg30 commented May 4, 2018

Updated.

@GraionDilach
Copy link
Contributor

GraionDilach left a comment

👍

@pchote

pchote approved these changes May 6, 2018

@pchote pchote merged commit 7ae6326 into OpenRA:bleed May 6, 2018

2 checks passed

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

@abcdefg30 abcdefg30 deleted the abcdefg30:repairRequest branch May 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.