BlackPhlox / bevy_config_cam

A Swiss Army knife of a camera plugin that allows for easy setup and configuration of a wide variety of types of moveable cameras and player cameras for a scene.
Other
183 stars 14 forks source link

Split up lib.rs #31

Closed Rushmore75 closed 1 year ago

Rushmore75 commented 1 year ago

Split up lib.rs into multiple files to make it easier to read. Also makes access modifiers relevant, which will make cleanliness easier.

Also changed DriverIndex (now in src/drive/drivers) to be a normal struct instead of a tuple struct. Same thing with Drivers (same file). This will make the code a bit easier to read, so instead of XYZ.0 it would be XYZ.field_name.

BlackPhlox commented 1 year ago

Hi @Rushmore75, thanks for contributing! I'm currently very busy with work, so my response time might be a bit long for the foreseeable future. I think splitting up is a good idea. Though, I think the current naming of the "views" should be different and more granular defined in the readme. And instead of called views, they should be grouped under a drivers folder with both the struct and its impl. Then driver_control.rs and drivers.rs (renamed to e.g. driver_util.rs) and moved to the outer src folder.

What's your opinion on this structure instead?

Rushmore75 commented 1 year ago

Just for making the file structure representative of what someone who is using the crate would see, I like the idea of only having lib.rs in src/ then having everything else back in folders. I don't know if this is actually an optimal way to create the layout tho. It could also make sense, like you said, to take driver_core.rs and driver_resources.rs (as named below) into the src/ directory.

Should the views each be put in their own files then? so have src/(prespectives, views, whatever they are called)/fpv.rs etc.

So would something like this work a bit better? image

And as impls are written for each perspective they would just be added to the respective file

BlackPhlox commented 1 year ago

I like this! Though I think perspectives (still think the name is WIP) should be in their own outer folder (in src) to indicate that users themselves can make drivers. Does that make sense?

Rushmore75 commented 1 year ago

(still think the name is WIP)

Agreed. Idk what to name it really... You'd probably have a better idea for it.

And it does make it more apparent that anyone else can add views/perspectives if we pull that folder out, that makes sense.

BlackPhlox commented 1 year ago

Cool cool! I think lets just rename perspectives to drivers for the time being, so its one-to-one with bevy_dolly

Rushmore75 commented 1 year ago

I think lets just rename perspectives to drivers for the time being, so its one-to-one with bevy_dolly

Yeah, that makes sense

BlackPhlox commented 1 year ago

If you run cargo fmt and cargo clippy locally and push the changes, then the CI should pass

BlackPhlox commented 1 year ago

Add .insert_resource(Drivers::new(vec![Box::new(Pinned), Box::new(Fpv)])) on line 21 in simple.rs to remove the unused function trait new clippy error

Rushmore75 commented 1 year ago

I'll run

If you run cargo fmt and cargo clippy locally and push the changes, then the CI should pass

Once I get back to a computer

BlackPhlox commented 1 year ago

Looks good! I know the test fails but it already did that prior to your pr. I'll fix the pending issues once merged. Thank you very much for your contribution! 🎉