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

Simplify Activity class #16354

Merged
merged 1 commit into from Mar 30, 2019

Conversation

@obrakmann
Copy link
Contributor

commented Mar 26, 2019

After the removal of the CompositeActivity class, all the supporting
code that made it work can be removed as well.

Simplify Activity class
After the removal of the CompositeActivity class, all the supporting
code that made it work can be removed as well.
@pchote

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Are we sure that we aren't going to need some of this for the ChildActivity deduplication mentioned in #16348? I was assuming that we would.

@obrakmann

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2019

I doubt it. That shouldn't be more than moving the duplicated parts to the TickOuter method, maybe with a conditional on top to allow for customization.

@reaperrr
Copy link
Contributor

left a comment

Looks good to me

@tovl

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2019

I can confirm that ParentActivity and RootActivity are not needed in the current paradigm. CurrentActivity is always the rootactivity and childactivities should never need to refer to their parent. The refactor mentioned in #16348 doesn't change that paradigm.

@pchote
pchote approved these changes Mar 30, 2019
}
}
get { return childActivity != null && childActivity.State < ActivityState.Done ? childActivity : null; }
set { childActivity = value; }

This comment has been minimized.

Copy link
@pchote

pchote Mar 30, 2019

Member

It might be a good idea to check and throw an exception if Game.Settings.Debug.StrictActivityChecking && childActivity != null && childActivity.State < ActivityState.Done.

Something for a followup PR, perhaps.

This comment has been minimized.

Copy link
@obrakmann

obrakmann Mar 30, 2019

Author Contributor

In theory, this could be removed in the future as well. The ChildActivity = ActivityUtils.RunActivity(...) pattern should see to it that ChildActivity gets set to null eventually, and it shouldn't ever be left in the Done state. I left it in out of caution for now.

@pchote pchote merged commit b4fd733 into OpenRA:bleed Mar 30, 2019

2 checks passed

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

@obrakmann obrakmann deleted the obrakmann:simplify_activities branch Mar 30, 2019

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.