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

Update to bevy v0.11 and kira v0.8. #40

Merged
merged 8 commits into from
Aug 17, 2023
Merged

Conversation

shanecelis
Copy link
Contributor

@shanecelis shanecelis commented Aug 9, 2023

Hi,

I think this is really cool work. I updated bevy_fundsp to work with bevy v0.11 and kira v0.8. I looked at updating bevy_oddio but it doesn't support bevy v0.11 yet.

This patch does make the examples work. However, the bevy_audio backend feels a little strange. DspSources can now be directly added as assets, which is cool; it should make life easier for this project. I took the bevy_audio interactive example and made two versions of it: interactive.rs which is the least intrusively changed but does now have a clone() that feels odd, and interactive_component.rs which relies on DspSources being used as assets and obviates the need for the dsp_manager. Neither feel perfect, but I feel like they'll give you a good means to discern the right direction for the project design-wise.

I haven't touched the docs or outward version numbers of bevy_fundsp itself.

Thanks for this cool project. I hope it continues to be updated.

@harudagondi
Copy link
Owner

Hello! I really appreciate this PR as recently I have been swamped with university stuff. Currently I don't have enough bandwidth to investigate the cloning issue, but I would probably have some free time in the following days. For now, I have updated bevy_oddio to 0.5 which should aid in updating this library. I'll also approve the workflow so it'll help you when you want to update this PR further.

Thank you very much for this!

@harudagondi
Copy link
Owner

harudagondi commented Aug 14, 2023

Hmm, looking at the actual code, it seems that implicitly cloning would be fine, as DspSource is defined as the following:

pub struct DspSource {
    pub(crate) dsp_graph: Arc<dyn DspGraph>,
    pub(crate) sample_rate: f32,
    pub(crate) source_type: SourceType,
}

Second, since this is only done at setup, the act of cloning would be usually only once, so I'd say it's not that bad. I'm thinking of making get_graph and get_graph_by_id return owned DspSource, since I planned it to be cheaply cloned, like how bevy_audio's AudioSource does it:

pub struct AudioSource {
    pub bytes: Arc<[u8]>,
}

@shanecelis
Copy link
Contributor Author

Thank you for the bevy_oddio rev. I updated oddio's examples and hopefully have fixed the doc errors. [Crosses fingers.] It should go green now.

@harudagondi harudagondi merged commit d90dc51 into harudagondi:main Aug 17, 2023
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.

2 participants