Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

add example for bevy-egui #67 #82

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Tarkin25
Copy link

No description provided.

@Tarkin25
Copy link
Author

Hi there! I just found your awesome library and want to use it to create complex noise graphs for terrain generation in bevy. I have struggled to integrate it with bevy-egui and found an open issue with the request to add an example for it. This PR adds the same graph as the other example, just slightly changed for the bevy integration.

Copy link
Owner

@setzer22 setzer22 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR! 😄 I checked the code and there's some things to take care of before we can merge

Comment on lines 12 to 23
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_plugin(EguiPlugin)
.init_resource::<NodeGraphExample>()
.add_system(draw_graph)
.run();
}

fn draw_graph(mut context: ResMut<EguiContext>, mut graph: ResMut<NodeGraphExample>) {
graph.draw(context.ctx_mut());
}
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure if I'm missing anything, but this seems to be the only required bits to integrate the library with bevy?

If that's the case, I think having the rest of the code be a copy-pasted version of the original example will make maintenance more difficult in the long run, since each change to the example will have to be written twice and kept in sync.

It would be better if we found a way to abstract away what's purely library interaction into a common library, so that we can then have multiple crates that depend on it. Something like:

  • egui_node_graph_example_lib
  • egui_node_graph_example_eframe (depends on *lib)
  • egui_node_graph_example_bevy (depends on *lib)

Copy link
Author

Choose a reason for hiding this comment

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

I agree that it would make sense to have a common library for the pure library interaction part. Might I suggest the following structure for different examples?

  • examples/node_graph.rs // contains the common example code i.e. NodeGraphExample etc.
  • examples/eframe.rs // contains eframe specific code and uses the node_graph module
  • examples/bevy.rs // contains bevy specific code and uses the node_graph module

Copy link
Owner

Choose a reason for hiding this comment

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

Yup! That's exactly how I'd envision this 😄

Copy link
Author

Choose a reason for hiding this comment

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

I have updated the PR to have both the eframe and bevy example in the examples directory like other rust crates do. However I don't know how to handle the parts in the egui_node_graph_example directory with the bat scripts and the wasm configurations. Do you have any ideas on what to do with these?

Copy link
Owner

@setzer22 setzer22 May 8, 2023

Choose a reason for hiding this comment

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

Hey! Sorry it took me so much to get back to you. These files are a copy from the official egui template, but I have never validated them.

Either way, all these scripts would go into the eframe directory, because they're specific to compiling an eframe application for wasm 👍

Another possibility is to remove those scripts. It is beyond the scope of this library to support wasm builds, that's more of a job for egui itself. The library should work on any egui backend, and the examples are just examples.

The only good reason I can think of for keeping the wasm builds is hosting a demo somewhere. But for that, we'd need someone who wants to take on maintenance of this part. I am not experienced enough in distributing wasm applications myself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants