Moving camera controller into separate crate in examples

[ShadowCurse]

Objective

Fixes #4408

Solution

Moved camera controller into bevy_camera crate.
Updated current 3d examples to use bevy_camera.

[DGriffin91]

This is awesome! I think this will be really useful. I like the defaults for up/down and sensitivity.

Would it be better to have the CameraController inserted on the perspective camera manually rather than automatically adding it with check_camera_controller()?

commands
    .spawn_bundle(PerspectiveCameraBundle { ..default() })
    .insert(CameraController::default());

I don't think this would add much complexity to the examples, and it would both improve flexibility and make it easier for a new user to see how it works.

Also with this configuration check_camera_controller() found_camera will never be true if a user were to manually assign a CameraController. Not sure if there is a meaningful performance impact.

Would it make sense to have a key that causes it to capture the mouse input?

In Unreal Engine, mouse scroll is used to increase/decrease the moment speed. Would that make sense here?

If we were to more fully duplicate Unreal's input it would actually be right click + drag that rotates the camera. Left click + drag moves the camera forward and backward and rotates left and right. And the middle mouse click + drag moves the camera up/down and left/right.

It may be worthwhile to use these control as a lot of people would be used to them. And it is nice to have the middle click panning. But I also think the current control scheme you have is good, and could make sense as well.

[ShadowCurse]

Some updates (better late than never):

• Removed check_camera_controller() and updated all 3d examples to explicitly add the CameraController component. Agree with @DGriffin91 that it better shows how everything works.
• Moved crate from examples into examples_utils in the root because CI wants to see only executable examples in the examples dir.
  About upgrading camera movement and so on, I think it can be done in separate PR.
  Furthermore examples_utils crate can contain other useful plugins, so upgrading what is inside should be done after this refactoring.

[IceSentry]

@DGriffin91 I'm personally not familiar with unreal. Using the scroll wheel to change the speed sounds really counterintuitive to me and I'm not aware of any camera that does that.

As someone more familiar with unity, the scroll wheel simply zooms in or out, right click to change the orientation, WASD to move around and middle click to pan.

Blender and godot also only changes the zoom when using the scroll wheel.

Also, middle click panning is not that nice when using a laptop, so having a control scheme that works without a middle click would be nice, but it's not required.

[DGriffin91]

@IceSentry I should clarify: in Unreal Engine, it's only when you are holding the right mouse button down that the scroll changes the movement speed. Blender also works the same way when in Walk/Fly mode (normal blender navigation does not use WASD movement, so there is no speed component).

I agree regarding the laptop input. I don't think Unreal has a alternate by default. But I do think we should make sure navigation is good with touchpads as well.

[alice-i-cecile]

Some small nits. Overall I'm strongly in favor of this change: I think it adds a lot of functionality to our examples and makes them much cleaner.

[ShadowCurse]

If we were to convert this to bevy_camera then, I think we should introduce some place in the repo where such crates will be. I don’t think placing them in crates is a great idea, because everything in crates is directly engine related, while bevy_camera is more of an addition that is built on top of the engine. But I might be mistaken. In the end, crates is so conveniently named.

[superdump]

Another approach is to not make it into bevy_camera and instead just not publish the crate to crates.io, if that is possible. I feel like it is a good idea to have a reasonably good (this would need more work - smoothing and configurability and such) camera controller as part of the project so people can get going quickly and then implement their own when they want to.

[mockersf]

I don’t think placing them in crates is a great idea, because everything in crates is directly engine related, while bevy_camera is more of an addition that is built on top of the engine.

I think I disagree? How do you see this distinction between "directly engine related" and "addition built on top"? Nothing would depend on bevy_camera? That's already the case of bevy_audio. A camera is a relevant part of the engine.

[alice-i-cecile]

I agree with @mockersf here. I expect we'll accumulate a number of optional leaf node crates over time. cargo is already a great tool to manage that.

[ShadowCurse]

Separation between “engine related” and “additions built on top” is based on my thoughts about Unreal Engine and the idea that when people think about it, they imagine, not the “engine” itself, but the whole ecosystem surrounding it, that is UEEditor, Blueprint editor and so on. So there is a separation between “the thing that runs everything”(that is the engine itself) and “additions”(that is engine ecosystem: editor, camera, ui, …). For example, an engine can not run without ECS, but it can function perfectly without a camera controller.

But I digress. I will convert examples_utils into bevy_camera then. One thing to note though: because bevy_camera is not a real “camera”, but just a controller for it, maybe bevy_camera should be named bevy_camera_controller to make things a little bit more clear?

[alice-i-cecile]

One thing to note though: because bevy_camera is not a real “camera”, but just a controller for it, maybe bevy_camera should be named bevy_camera_controller to make things a little bit more clear?

Down the line, I'd like to move all of our camera-related code into there: there's a lot of scattered duplication across the various rendering crates at the moment. So bevy_camera gets my vote.

[DGriffin91]

@alice-i-cecile If there's going to be a bunch of things in bevy_camera (presumably #4572) then should the file with the controller functionality be named more specifically? Like bevy_camera/camera_controller.rs?

[alice-i-cecile]

@alice-i-cecile If there's going to be a bunch of things in bevy_camera (presumably #4572) then should the file with the controller functionality be named more specifically? Like bevy_camera/camera_controller.rs?

Yes, absolutely :)

[ShadowCurse]

Hm… I really don’t understand why CI fails. It can’t find a new camera crate, but bevy_camera is included in default features and in bevy_internal everything seems correct. Locally it compiles and runs without issues.

[ShadowCurse]

I have checked CI one more time and have found out that bevy is compiled with this command

cargo build --no-default-features --features "bevy_dynamic_plugin,bevy_gilrs,bevy_gltf,bevy_winit,render,png,hdr,x11,bevy_ci_testing,trace,trace_chrome,bevy_audio,vorbis"

Because of that, the bevy_camera feature is not active and examples are failing to compile.
@alice-i-cecile Can you help with this?

[alice-i-cecile]

Because of that, the bevy_camera feature is not active and examples are failing to compile.

So, this PR adds bevy_camera as a feature to the Cargo.toml. The simplest fix would be to just add bevy_camera to the list of enabled features for that example-running shell command.

However, if we're planning to move other camera code into here, I think that the correct solution here is to actually add bevy_camera as a subfeature under render, which will cause it to be enabled with the existing render feature.

[ShadowCurse]

Alright, I will add it as a render subfeature then.

[IceSentry]

Closed since something similar was merged with #11338