Remove use of NotificationCenter #15
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Memory usage seems to have gone up from ~60mb to ~100mb which is interesting. I don't see anything looking like a memory leak... letting the macOS target run for a while to see what happens. |
|
Well something leaks, SpriteKit related, but it doesn't look like memory usage goes much above 108mb anyway. So I don't think it's anything to worry about or anything I did specifically...
|
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I had a theory that the freezing part of this issue #6 was because of my use of
NotificationCenter.default, namely that when firing the 'animationComplete' messages, they were being received by both instances of the screensaver, which made things get wonky. I didn't do anything to prove this theory, beyond the work of this PR.I removed any usage of
NotificationCenterand replaced it instead with completion handlers. It seems like this fixed the issue with multiple instances of the screensaver as well as fixed a millisecond or two delay when progressing through the animation queue, which is a nice benefit. Overall much better to do it this way, though it did mean I had to add a bunch ofcompletionHandlerarguments to stuff in theAnimationfile...