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

BindActorTriggers for APC passengers #16963

Closed
wants to merge 1 commit into from

Conversation

abcdefg30
Copy link
Member

Fixes a part of #16960.

pchote
pchote previously approved these changes Aug 18, 2019
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Awful, but 👍

@pchote pchote added this to the Next Release milestone Aug 18, 2019
@tovl
Copy link
Contributor

tovl commented Aug 18, 2019

Why not make UnloadPassengers return the actor list? Like so:

 		[ScriptActorPropertyActivity]
 		[Desc("Command transport to unload passengers.")]
-		public void UnloadPassengers(CPos? cell = null, int unloadRange = 5)
+		public Actor[] UnloadPassengers(CPos? cell = null, int unloadRange = 5)
 		{
 			if (cell.HasValue)
 			{
 				var destination = Target.FromCell(Self.World, cell.Value);
 				Self.QueueActivity(new UnloadCargo(Self, destination, WDist.FromCells(unloadRange)));
 			}
 			else
 				Self.QueueActivity(new UnloadCargo(Self, WDist.FromCells(unloadRange)));
+
+			return cargo.Passengers.ToArray();
 		}

@pchote
Copy link
Member

pchote commented Aug 18, 2019

UnloadCargo isn't guaranteed to unload everything - there may not be enough space. IMO the expectation for this API should be for it to return only the actors that are actually unloaded, which isn't possible to know ahead of time.

@pchote
Copy link
Member

pchote commented Aug 18, 2019

I just noticed that we have a Trigger.OnPassengerExited. We can/should use this to call BindActorTriggers on the new actors for both the cargo and paradrop case.

IMO we should deprecate the actor return value from SendParatroopers for the same reason as above - we can't guarantee that all of them will be dropped, and OnPassengerExited provides a more convenient solution.

@pchote
Copy link
Member

pchote commented Aug 19, 2019

Superseded by #16964 - I wanted to make sure we had a fix ready for the new test build I will be spinning today.

@pchote pchote closed this Aug 19, 2019
@abcdefg30 abcdefg30 deleted the unloadShell branch August 19, 2019 17:05
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

3 participants