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

Allow additional actor IDs to be send with orders. #16999

Merged
merged 2 commits into from Nov 26, 2019

Conversation

@tovl
Copy link
Contributor

tovl commented Aug 27, 2019

split from and prerequisite for #16883

Updates the order protocol to allow sending a list of actors.

minLength += 4 + 1 + 13 + (TargetString != null ? TargetString.Length + 1 : 0) + 4 + 4 + 4;

if (ExtraActors != null)
minLength += ExtraActors.Count() * 4;

This comment has been minimized.

Copy link
@RoosterDragon

RoosterDragon Aug 28, 2019

Member

Plain old Count should work here, right?

This comment has been minimized.

Copy link
@tovl

tovl Aug 29, 2019

Author Contributor

Fixed.

This comment has been minimized.

Copy link
@pchote

pchote Nov 26, 2019

Member

This is still using the Enumerable method, so hasn't been resolved?

We'll want .Length now that its an array instead of a list.

This comment has been minimized.

Copy link
@tovl

tovl Nov 26, 2019

Author Contributor

Weird. Don't know how that happened.

Fixed.

@@ -329,6 +351,13 @@ public byte[] Serialize()
if (fields.HasField(OrderFields.TargetString))
w.Write(TargetString);

if (fields.HasField(OrderFields.ExtraActors))
{
w.Write(ExtraActors.Count());

This comment has been minimized.

Copy link
@RoosterDragon

RoosterDragon Aug 28, 2019

Member

Count should work here, right?

This comment has been minimized.

Copy link
@tovl

tovl Aug 29, 2019

Author Contributor

Fixed.

This comment has been minimized.

Copy link
@pchote

pchote Nov 26, 2019

Member

Likewise here.

This comment has been minimized.

Copy link
@tovl

tovl Nov 26, 2019

Author Contributor

Fixed.

@tovl tovl force-pushed the tovl:order-actorlist branch from c42172c to effdf86 Aug 29, 2019
@tovl tovl force-pushed the tovl:order-actorlist branch from effdf86 to bd3d429 Aug 31, 2019
@tovl tovl force-pushed the tovl:order-actorlist branch from bd3d429 to dc5924f Sep 16, 2019
@reaperrr reaperrr mentioned this pull request Nov 17, 2019
@tovl tovl requested review from RoosterDragon and teinarss Nov 19, 2019
Copy link
Contributor

teinarss left a comment

Otherwise it looks good!

OpenRA.Game/Network/Order.cs Outdated Show resolved Hide resolved
@tovl tovl force-pushed the tovl:order-actorlist branch from dc5924f to 4149833 Nov 22, 2019
OpenRA.Game/Network/Order.cs Outdated Show resolved Hide resolved
@tovl tovl force-pushed the tovl:order-actorlist branch from 4149833 to 92a03ab Nov 22, 2019
@pchote

This comment has been minimized.

Copy link
Member

pchote commented Nov 26, 2019

Looks reasonable otherwise, but not tested.

@tovl tovl force-pushed the tovl:order-actorlist branch from 92a03ab to 3f089ab Nov 26, 2019
@abcdefg30 abcdefg30 merged commit 6fb3dc0 into OpenRA:bleed Nov 26, 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
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Nov 26, 2019

@tovl tovl deleted the tovl:order-actorlist branch Dec 15, 2019
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

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