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

Get rid of "shutdown" methods because of Apple issues #758

Merged
merged 5 commits into from Oct 26, 2016

Conversation

jmnavarro
Copy link
Contributor

No description provided.

@@ -206,7 +206,7 @@ class DynamicTextureAtlas

/// Clear out the active dynamic textures. Caller deals with the
/// change requests.
void shutdown(ChangeSet &changes);
void teardown(ChangeSet &changes);
Copy link
Contributor Author

@jmnavarro jmnavarro Oct 21, 2016

Choose a reason for hiding this comment

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

@mousebird this seems a bit confusing to me. DynamicTextureAtlas already has a cleanup() method, so now we have both cleanup() and teardown().
I'd say both methods are quite similar, the only different is cleanup() only removes the texture if it has regions. Can we merge both methods together in one single cleanup() one?

Copy link
Collaborator

Choose a reason for hiding this comment

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

cleanup() isn't a shutdown method. It destroys textures we're no longer using and is run regularly. teardown() seems like a good name.

@@ -55,7 +55,7 @@ - (void)cleanup
[self performSelector:@selector(cleanup) withObject:nil afterDelay:kWKParticleSystemCleanupPeriod];
}

- (void)shutdown
- (void)teardown
Copy link
Contributor Author

@jmnavarro jmnavarro Oct 21, 2016

Choose a reason for hiding this comment

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

@mousebird Similar confusion here. Can we merge cleanup() and teardown() methods in a single one?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope! I like the name teardown()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted the latest commit, so everything it teardown() now

@mousebird mousebird merged commit e689d46 into develop_2_4_1 Oct 26, 2016
@jmnavarro jmnavarro deleted the issue/735 branch October 26, 2016 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants