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

Fix sometimes player cannot be determined and yield nil unwrap #2231

Merged
merged 1 commit into from Jan 19, 2019

Conversation

3 participants
@alejx
Copy link
Member

alejx commented Jan 18, 2019

Fix the issue metioned in #2207

@saagarjha

This comment has been minimized.

Copy link
Member

saagarjha commented Jan 18, 2019

Do you know the underlying reason why this happens? Otherwise, this seems like a bit of a kludge…

@alejx

This comment has been minimized.

Copy link
Member Author

alejx commented Jan 18, 2019

I've no idea, but directly setting player in the view controller when the playlist view is loaded is better than searching the player reversely when it's used IMO.

@lhc70000

This comment has been minimized.

Copy link
Member

lhc70000 commented Jan 18, 2019

While it's not easy to reproduce this issue, I think the crash occurred because playlistView.window was nil, i.e. the view is not attached to a window in the mean time (such as it was in the middle of switching to music mode).

Therefore IMO this approach is at least better than the original one since it set up the player at the beginning, so it won't need to look up for it in the window hierarchy each time.

@saagarjha

This comment has been minimized.

Copy link
Member

saagarjha commented Jan 18, 2019

With that in mind, I'm satisfied with these changes.

@alejx alejx merged commit fbac25f into develop Jan 19, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alejx alejx deleted the subpopover-nil-unwrap branch Jan 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment