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

StaticSoundData and other types do not implement Debug #31

Open
attackgoat opened this issue Aug 30, 2022 · 3 comments
Open

StaticSoundData and other types do not implement Debug #31

attackgoat opened this issue Aug 30, 2022 · 3 comments

Comments

@attackgoat
Copy link

This makes it hard to do things like Arc::try_unwrap(...) on a StaticSoundData. The guidelines say all types should implement Debug (except for rare exceptions) but it looks like the ringbuf dependency does not, which makes it hard to fix in kira.

Is there any opinion between these options?

  • Manually implement Debug, formatting the obvious fields (and optionally providing something nice for things like consumer)
  • Upgrade the ringbuf dependency (it is behind) and open an issue in their repo; wait for a fix and then auto-derive Debug
  • Use the new-type pattern to wrap ringbuf types and provide a generic Debug implementation inside kira
@tesselode
Copy link
Owner

The main reason I haven't derived Debug for StaticSoundData is because it contains a Vec which will typically contain a very large number of Frames - you probably don't want all that in your terminal. I was thinking of adding a manual Debug impl that displays something like frames: [2392 frames]. I'm not sure if there's a standard for how to display very long Vecs.

As for the other types, I'd probably use a manual Debug impl for anything containing a ringbuf type that doesn't implement Debug. That being said, you should open an issue in the ringbuf repo is you feel strongly about it.

On a side note, why do you need to wrap a StaticSoundData in an Arc? The frames of a StaticSoundData are already in an Arc.

@attackgoat
Copy link
Author

The frames: [2392 frames] idea sounds clear and avoids the issue you're predicting, yes a terminal full of stuff would be 😔.

I was playing around with Arc<Mutex<HashMap<String, StaticSoundData>>> with sounds being loaded on multiple threads; at the end I wanted the HashMap out of that mess.

@tesselode
Copy link
Owner

Added some Debug implementations in v0.7.1, but there's still some types that could implement Debug but don't, so I'll leave this issue open for now.

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

No branches or pull requests

2 participants