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

add error handling when sending score #53

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Vrixyz
Copy link
Contributor

@Vrixyz Vrixyz commented Aug 22, 2022

Adds error handling when sending score.

Rationale

An app might want to retry to send a score if it failed, otherwise the user will be disappointed to see its score not saved :(

I see 2 main reasons for failing to send:

  • in production: bad or no internet connection ; or server down
  • in development: server issue or wrong cli configuration

Pull requests attention points

  • error display in the example whac-a-square doesn't wrap, so it truncated when going offscreen
  • Using a channel for cross thread communication might be better, but I wanted to keep close to existing codebase
  • there are unwraps that might be likely to trigger

fn despawn_menu(mut commands: Commands, root_ui: Query<Entity, (With<Node>, Without<Parent>)>) {
fn despawn_menu(
mut commands: Commands,
root_ui: Query<Entity, (With<Node>, With<MenuUI>, Without<Parent>)>,
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 added more control on what exactly is despawned to be able to keep the debug UI for errors display

@@ -29,6 +30,105 @@ fn main() {
.run();
}

mod debug {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be after GameState and setup()

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

1 participant