idanarye / bevy-yoleck

Your Own Level Editor Creation Kit
Other
172 stars 11 forks source link

Create functionality to allow editors to spawn new objects #15

Closed ZeeQyu closed 1 year ago

ZeeQyu commented 1 year ago

In my game project, I have fences that can be a custom length, and you often want pairs of them. Therefore, I added an yoleck editor button that spawns a clone of that object. This required me to fork yoleck and make a couple fields public, and they reasonably should not be public, since there seems to be some invariants to uphold. My suggestion then is to add functionality to allow the editor to spawn editor objects with an arbitrary payload, since this can be useful for all kinds of use cases.

I'll show you how I did this in my project, with some extra context code. You're free to integrate any of the code I paste in this issue into yoleck, but ask if there's something from the rest of my repo (https://github.com/ZeeQyu/sylt) you want to use.


This is the only change I had to do in yoleck (and I'm hoping this won't be required if you move the function inside since it'd be bad if a user starts editing this without maintaining the invariants).

diff --git a/src/lib.rs b/src/lib.rs
index ea0a503..92d7d96 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -315,8 +315,8 @@ impl YoleckTypeHandlers {
 /// Fields of the Yoleck editor.
 #[derive(Resource)]
 pub struct YoleckState {
-    entity_being_edited: Option<Entity>,
-    level_needs_saving: bool,
+    pub entity_being_edited: Option<Entity>,
+    pub level_needs_saving: bool,
 }

 impl YoleckState {

And in my code:

pub fn create_editor_object(commands: &mut Commands, writer: &mut EventWriter<YoleckEditorEvent>, yoleck: &mut ResMut<YoleckState>, type_name: &str, value: serde_json::Value) {
    let cmd = commands.spawn(YoleckRawEntry {
        header: YoleckEntryHeader {
            type_name: String::from(type_name),
            name: String::from(""),
        },
        data: value,
    });
    writer.send(YoleckEditorEvent::EntitySelected(cmd.id()));
    yoleck.entity_being_edited = Some(cmd.id());
    yoleck.level_needs_saving = true;
}

#[derive(Clone, PartialEq, serde::Serialize, serde::Deserialize)]
struct EditorFence {
    #[serde(default)]
    position: Vec2,
    #[serde(default)]
    orientation: FenceOrientation,
    #[serde(default)]
    section_length: f32,
}

#[derive(Default, Clone, PartialEq, serde::Serialize, serde::Deserialize)]
pub enum FenceOrientation {
    #[default]
    Horizontal,
    Vertical,
}
fn edit_fence(
    mut edit: YoleckEdit<EditorFence>,
    configuration: Res<Configuration>,
    mut commands: Commands,
    mut writer: EventWriter<YoleckEditorEvent>,
    mut yoleck: ResMut<YoleckState>,
) {
    edit.edit(|ctx, data, ui| {
        if ui.add(egui::Button::new("Spawn copy")).clicked() {
            let offset_axis = match data.orientation {
                FenceOrientation::Horizontal => -Vec2::Y,
                FenceOrientation::Vertical => Vec2::X,
            };
            let value = serde_json::to_value(EditorFence { position: data.position + offset_axis * 20.0, ..data.clone() }).unwrap();
            create_editor_object(&mut commands, &mut writer, &mut yoleck, FENCE_NAME, value);
        }
// --snip--
    });
}

I have no clue how to solve this elegantly, without requiring users to add EventWriter and ResMut to their systems, so I'm hoping you find a better way!

idanarye commented 1 year ago

How about something like this?

fn edit_fence(
    mut edit: YoleckEdit<EditorFence>,
    mut writer: EventWriter<YoleckDirective>,
) {
    edit.edit(|ctx, data, ui| {
        if ui.add(egui::Button::new("Spawn copy")).clicked() {
            writer.send(YoleckDirective::spawn(
                FENCE_NAME,
                EditorFence {
                    // set the fields here
                },
                true, // automatically select the new entity
            ));
        }
    });
}

It still requires an EventWriter, but YoleckEditorEventYoleckDirective is supposed to be part of the API (even if it was meant for custom Vpeol implementations and not for handler systems), so it does not feel like an hack.

EDIT: Use YoleckDirective instead of YoleckEditorEvent.

ZeeQyu commented 1 year ago

I like that.

The thing that was bugging me about my solution is that I had to both call a function, and also give the function values I don't use. Just having a value (the eventwriter) that I directly interact with is very in line with bevy's design in general, and sending an event seems like a good way to interact with the system.

Are you supposed to send EntitySelected, EntityDeselected and EditedEntityPopulated though? Cause while the solution you present is fine for my usecase, you can go one step further and make it clearer for new users if you have one event enum which is intended to be created by the user as an input into yoleck, and one event enum that is an output. Like YoleckEditorEvent and YoleckEditorCommand. (Or some naming in line with input/output)

Then, using the same interface, you can add SelectEntity and DeselectEntity to the command enum, and add examples where that's used, and then you can start allowing the user to have knobs or buttons that hop to a paired entity or something. I'm thinking the user sends a SelectEntity to instruct the editor to select something, and can have a hook that listens for EntitySelected to do something when an entity is selected, regardless of the source.

idanarye commented 1 year ago

Wait - that's a good point. I haven't touched that in a while, but I did made the distinction - it's just instead of YoleckEditorCommand I called it YoleckDirective. I'll fix my comment.

ZeeQyu commented 1 year ago

Oh nice, you have constructors and everything!

In your edited comment, you mean YoleckDirective::spawn, not YoleckDirective.spawn, right? Otherwise you're suggesting a comprehensive redesign.

If so, yeah, that looks amazing! Consider adding at least one usage of a directive in the yoleck-examples though, cause I would never for the life of me find this functionality without you pointing it out, in the current version.

idanarye commented 1 year ago

Yes, of course.

idanarye commented 1 year ago

I would prefer if I could do it without constructors, but it uses lots of dynamic data so it'd be very ugly to construct it manually...

ZeeQyu commented 1 year ago

I don't know if you actually have to do serde_json::to_value when you do a real implementation like you're planning to here. I had to in my hacky solution:

            let value = serde_json::to_value(EditorFence { position: data.position + offset_axis * 20.0, ..data.clone() }).unwrap();

but if you need to do either that or some other checking to see if what was passed is serializable, then having a constructor lets you do some error checking before giving the result to the event writer, which also lets you handle it with a result instead of a panic, so that the user could handle it if they wanted to. So there might be an additional point in keeping constructors.

idanarye commented 1 year ago

I'm not sure if this is needed. If the entity type cannot be reliably serialized, there will be enough other problems that break the editor.