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

Replace pseudo-childactivities in carryall logic. #16374

Merged
merged 1 commit into from Apr 14, 2019

Conversation

@tovl
Copy link
Contributor

commented Mar 31, 2019

After getting rid of all the self-referential instances of QueueActivities, the next step is to get rid of activities rolling their own pseudo-childactivities instead of using the common ChildActivity logic. To begin with, these are the activities that determine the carryall behaviour.

I've made it dependent on #16372 because it was rather difficult to test otherwise.

@pchote
Copy link
Member

left a comment

Code changes look fine aside from the noted oversights.

I noticed while testing that these activities don't work properly wrt queuing - not sure if you want to address this here, or leave it to a followup.

OpenRA.Mods.Common/Activities/DeliverUnit.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Activities/PickupUnit.cs Outdated Show resolved Hide resolved
@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2019

Are the queueing issues introduced here, or are they also on bleed?

@pchote

This comment has been minimized.

Copy link
Member

commented Mar 31, 2019

Also a problem on bleed.

@obrakmann

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2019

With luck they get solved by the harvester refactor PR

@pchote

This comment has been minimized.

Copy link
Member

commented Mar 31, 2019

17:29 <+pchote> antares79: the queuing issues are with the TS carryall
17:29 <+antares79> ah, sorry, didn't know that.
17:29 <+antares79> what exactly were you trying?
17:30 <+antares79> because the repairable trait causes some weird queuing issues as well, but that's more or less by design atm
17:30 <+pchote> pick up an actor, order a dropoff then queue moves once its on its way
17:30 <+pchote> it will do the dropoff and then stop
17:30 <+antares79> ok
17:30 <+pchote> whereas i'd expect it to either drop off and then move to the queued location or, better, not drop off and treat it as a move waypoint

@tovl tovl force-pushed the tovl:carryall-child branch from cf1a214 to 0d5b0f2 Mar 31, 2019

@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2019

I'll leave the queueing issues for a follow up. That requires a more fundamental rethink of how the carryall order interface works.

@matjaeck
Copy link
Contributor

left a comment

LGTM

@pchote
pchote approved these changes Apr 14, 2019

@pchote pchote merged commit cc7e758 into OpenRA:bleed Apr 14, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.