-
Notifications
You must be signed in to change notification settings - Fork 72
Add OutlineGizmoPlugin and integrate outline gizmo functionality #195
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the code here: verbose but clear. Toss in a screenshot to the PR description and I'll merge this for you.
I actually can’t get the toggle to display in the pane. Could use some advice on here if possible? |
I generally set up my IDE to format on save to reduce the CI headaches while working on Rust projects :) |
…_gizmo, and lib modules
also add documentation about compiling the editor locally and running with local changes
@alice-i-cecile think this is ready to be merged now - i've also mentioned #207 in this one - not sure if that autolinks the Issue to this PR though (ie, once merged, will it close that Issue?) |
} | ||
} | ||
|
||
// Marker component for selected entities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There already is a SelectedEntity resource defined in bevy_editor_core, you should not define your own component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done - i've added bevy_editor_core
to the cargo.toml
for bevy_3d_viewport
top: Val::Px(20.0), | ||
right: Val::Px(20.0), | ||
width: Val::Px(150.0), | ||
height: Val::Px(15.0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The height of this button is smaller than the text inside which is not very pleasing to look at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, i've fixed thisi by making the font smaller for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | ||
} | ||
|
||
pub fn spawn_gizmo_toggle_ui(mut commands: Commands) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be only a temporary solution, it would be better to have a menu to manage which gizmo types are visible just like other game engines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree with this - but beyond the scope of this PR. I just wanted to add something quick and dirty to show that we can add the outlines
crates/bevy_editor/src/lib.rs
Outdated
project::update_project_info(); | ||
|
||
// project::update_project_info(); | ||
println!("Loading Bevy Editor"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should rather use info!.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
crates/bevy_editor/src/lib.rs
Outdated
}, | ||
Transform::default().looking_to(Vec3::NEG_ONE, Vec3::Y), | ||
)); | ||
fn load_example_scene(mut commands: Commands, asset_server: Res<AssetServer>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will crash the editor when it is not run as the example. This should be somehow integrated into the example code instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, i've removed this for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an invalid scene!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've removed loading this for now
…ctedEntity struct, remove example scene loading, and fix "Show Outlines" button
@Constanze3 Thanks for the constructive review! This was so helpful, and I've made those changes you've recommended. I've also removed the example scene loading for now, until we come up with a more elegant solution. |
crates/bevy_editor/src/lib.rs
Outdated
@@ -46,8 +46,8 @@ pub struct EditorPlugin; | |||
impl Plugin for EditorPlugin { | |||
fn build(&self, bevy_app: &mut BevyApp) { | |||
// Update/register this project to the editor project list | |||
project::update_project_info(); | |||
|
|||
// project::update_project_info(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this commented out intentionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh strange, that must've been an accident on my end - apols
crates/bevy_editor_core/src/lib.rs
Outdated
@@ -14,7 +14,7 @@ impl Plugin for EditorCorePlugin { | |||
} | |||
|
|||
/// The currently selected entity in the scene. | |||
#[derive(Resource, Default, Reflect)] | |||
#[derive(Component, Resource, Default, Reflect)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not how you should do this. This should not be a component.
Instead in your gizmo system you should have a query: Query<&Transform> and also grab selected_entity: Res<SelectedEntity> and then use query.get(selected_entity.0) to get the transform of the selected entity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Thanks for the tip
…te gizmo color logic - also reverts Component derive from SelectedEntity
First cut at getting an Outline Gizmo working - very rough, and highly WIP while learning the ins and outs of bevy and the repo :)
My first thoughts are that it seems incredibly verbose to create a toggle which is updated by some "listener".
It would be nice if this block could be heavily simplified:
into something with a more simplified callback structure.
This is also a first step toward the Milestone Issue we have here: #197
Also adds documentation outlined in #207 to give new devs help when trying compile/run changes locally
Screenshot of toggle button working on the editor locally :)