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

Ensure all task exceptions are handled, to prevent crashes. #138

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

Erfa
Copy link
Contributor

@Erfa Erfa commented Jan 2, 2024

task.result.get() crashes for me when calling loadAchievement in one scenario. I don't think this is intentional? Seems like this should be handled by the failure listener which was already present. After this change it is.

I also changed a similar case with leaderboards and saves. After this no completeListeners should assume that data exists.

Disclaimer: I don't really know Kotlin and haven't worked with native Android in years, but it seems pretty straightforward.

@theLee3
Copy link
Collaborator

theLee3 commented Jan 4, 2024

This resolves #133.

We still need to call follow the instructions here to acquire the permission to load friendsOnly leaderboards.

@theLee3
Copy link
Collaborator

theLee3 commented Jan 4, 2024

Out of curiosity @Erfa, what one scenario led to the crash when calling loadAchievement?

@Erfa
Copy link
Contributor Author

Erfa commented Jan 7, 2024

Out of curiosity @Erfa, what one scenario led to the crash when calling loadAchievement?

I was running it on an emulator which I had apparently loaded with an image that didn't have Play Store support. Reproducible every time for me.

@theLee3
Copy link
Collaborator

theLee3 commented Jan 8, 2024

@Abedalkareem I will have the permission request sorted within a couple of days.

@theLee3
Copy link
Collaborator

theLee3 commented Jan 24, 2024

This also closes #149.

Copy link
Collaborator

@theLee3 theLee3 left a comment

Choose a reason for hiding this comment

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

Please remove your changes to Leaderboards.kt from this PR as a more extensive fix had to be merged to account for a permission request.

@Erfa
Copy link
Contributor Author

Erfa commented Jan 28, 2024

Something like that?

@theLee3
Copy link
Collaborator

theLee3 commented Jan 29, 2024

Perfect. Thanks @Erfa.

@theLee3 theLee3 merged commit 431e52d into Abedalkareem:develop Jan 29, 2024
1 check failed
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.

3 participants