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

Make RenderCoordinator errors recoverable #190

Merged
merged 11 commits into from
May 13, 2024
Merged

Conversation

jandrokt
Copy link
Contributor

Description

This pull request ensures that any errors that may surge in RenderCoordinator#init are propagated back to the UI and so that an error message is shown instead of crashing the entire client.

Fixes #175 (not sure if fully)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation comments
  • My changes generate no new warnings

jandrokt and others added 5 commits May 12, 2024 17:48
This avoids unwrapping, and only adds the localized description when an error is associated with the case.
I also added an unknown so that the catch can handle unexpected errors, if necessary.
Case statements with correlated errors instead of optionals
@mattg42
Copy link
Contributor

mattg42 commented May 13, 2024

Any reason why RendererError is a struct with a separate error kind enum property, instead of just being an enum itself?

Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

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

Looking good so far! You'll be a Swifty in no time hahah, thanks @ninjadev64 and @obvgab for helping out while I was asleep 😴

Just a few requested changes which are mostly about simplifying things that that were artefacts of previous attempts.

Sources/Client/Views/Play/WithRenderCoordinator.swift Outdated Show resolved Hide resolved
Sources/Core/Renderer/RenderCoordinator.swift Outdated Show resolved Hide resolved
Sources/Core/Renderer/RenderCoordinator.swift Outdated Show resolved Hide resolved
Sources/Client/Views/Play/GameView.swift Outdated Show resolved Hide resolved
@stackotter
Copy link
Owner

Any reason why RendererError is a struct with a separate error kind enum property, instead of just being an enum itself?

Hahah didn't see this until after I submitted my review, you beat me to it

Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

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

Awesome, looks much better, thanks! 🎉

@stackotter
Copy link
Owner

I'll wait for the workflows to complete.

@stackotter stackotter merged commit 0943b4e into stackotter:main May 13, 2024
5 checks passed
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.

Make renderer errors recoverable instead of fatal (to help with troubleshooting)
5 participants