Skip to content
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

Externalize behaviors #7

Open
SergioRibera opened this issue Sep 29, 2023 · 21 comments
Open

Externalize behaviors #7

SergioRibera opened this issue Sep 29, 2023 · 21 comments
Labels
enhancement New feature or request hacktoberfest Hacktoberfest Participations help wanted Extra attention is needed
Milestone

Comments

@SergioRibera
Copy link
Owner

This is a feature that I would like to implement, the idea would be to achieve something like this

fn create_scene(mut cmd: Commands) {
    // Spawn Virtual Joystick
    cmd.spawn(
        VirtualJoystickBundle::new(VirtualJoystickNode {
            // ....
            // Feature
            action: ClickColor(Color::WHITE.with_a(0.5)),
        })
    )
    .insert(BackgroundColor(Color::ORANGE_RED.with_a(0.3)))
    .insert(VirtualJoystickInteractionArea);
}

The idea is actually that there are actions already defined by default by the plugin but the user can also create their own actions like this

pub struct ClickColor(pub Color);

impl VirtualJoystickAction for ClickColor {
  pub fn on_click(&self, world: &mut World, entity: Entity) {
    let tint = world.get_mut<TintColor>().unwrap();
    *tint = TintColor(self.0);
  }
}

The trait should be something like this

pub trait VirtualJoystickAction {
  fn on_click(&self, _world: &mut World, _entity: Entity) { }
  fn on_drag(&self, _world: &mut World, _entity: Entity) { }
  fn on_end_drag(&self, _world: &mut World, _entity: Entity) { }
  fn on_up(&self, _world: &mut World, _entity: Entity) { }
}
@SergioRibera SergioRibera added enhancement New feature or request help wanted Extra attention is needed hacktoberfest Hacktoberfest Participations labels Sep 29, 2023
@clinuxrulz
Copy link
Contributor

This to me feels like allowing users to implement their own joystick behaviours. So that means the existing behaviour Fixed, Floating and Dynamic should be able to fit in the same framework. (They can change from enums to singleton structs implementing a trait.) And maybe they can also be composed or pipelined together. E.g. Invisible + Dynamic to make a joystick that is invisible when screen not touch, but becomes visible and moves like a Dynamic when touched.

Existing behaviours then become more of an opt-in rather than default.

Just a thought.

@SergioRibera
Copy link
Owner Author

TOTALLY, with this comment you gave me the idea of abstracting not only the behaviors, I will try to work on that, I really think it is an excellent idea

@clinuxrulz
Copy link
Contributor

clinuxrulz commented Feb 17, 2024

Also horizontal only and vertical only axis constraints would fit inside the same framework. As well as constraining the knob movement in a circular area rather than square.

@clinuxrulz
Copy link
Contributor

I had a quick play at refactoring axis directions and joystick behaviors into singleton structs using traits.

Its just an experiment, the trait methods should probably be modified (minimum refactor to get working):

https://github.com/clinuxrulz/virtual_joystick/tree/externalize-behaviors

It looks like dynamic dispatch for behaviors would have to be used top-level instead of static dispatch. Otherwise the plugin will need to be registered multiple times for each joystick with different behavior. Also handling joystick events get duplicated if behaviors are static-dispatch top level. (Put Behavior in a Box<> in VirtualJoystickNode.)

@clinuxrulz
Copy link
Contributor

clinuxrulz commented Feb 17, 2024

We can change it to your example event trait methods above. VirtualJoystickData would need to be exposed in public api. And maybe a prevent_default callback to overrides default behaviour. E.g. making a acceration slider joystick that does not snap the knob back to the centre when you release it.

Another example. A user might want to make a joystick behave like a manual gear stick for a car. We need to make sure knob movements can be customised.

@clinuxrulz
Copy link
Contributor

clinuxrulz commented Feb 18, 2024

Another option which I think is more simple. Is to make the behaviours just a component that goes in the same entity as the main joystick node. The user can add a system for that component to control the joystick background and knob themselves. This library would still provide components for the current behaviours.

Because for maximum flexibility you ultimately end up with:

pub trait VirtualJoystickBehaviour {
    fn update(world: &mut World, entity: Entity);
}

But that would just be the same as the end user adding their own system.

Will think about it some more.

@clinuxrulz
Copy link
Contributor

clinuxrulz commented Feb 23, 2024

I had a good run on this branch:
https://github.com/clinuxrulz/virtual_joystick/tree/externalize-behaviors-attempt-3

All the behaviors are just ECS components, that can be mixed and matched. JoystickInvisible is supported in the lib too.

Each behavior has its own system, and the user can create their own components and systems to customize behavior (or complete change it)

Invisible from systems.rs:

pub fn update_joystick_visible<S: VirtualJoystickID>(
    mut joysticks: Query<(&mut Visibility, &VirtualJoystickNode<S>), With<JoystickInvisible>>,
) {
    for (mut joystick_visibility, joystick_state) in &mut joysticks {
        if joystick_state.just_released || *joystick_visibility != Visibility::Hidden && joystick_state.touch_state.is_none() {
            *joystick_visibility = Visibility::Hidden;
        }
        if let Some(touch_state) = &joystick_state.touch_state {
            if touch_state.just_pressed {
                *joystick_visibility = Visibility::Inherited;
            }
        }
    }
}

Axis constraint from systems.rs:

pub fn update_horizontal_only<S: VirtualJoystickID>(
    mut joystick: Query<&mut VirtualJoystickNode<S>, With<JoystickHorizontalOnly>>,
) {
    for mut joystick_state in &mut joystick {
        joystick_state.delta.y = 0.0;
    }
}

If the end user wants to implement their own diagonal axis constraint for example, they can just make a component and a system and throw it in just in their code (no need for extra code in lib). Your change Tint color on press should be easy too.

VirtualJoystickNode<S> is in the public API, and its fields are kept to minimum.

For usage, have a look at the examples for that branch.

@SergioRibera
Copy link
Owner Author

Hello, I was carefully reviewing the implementations that you shared and I am afraid that I am looking for a way to implement it more as a variable of the joystick itself than as a component, it does not make much sense to me that it is a component

cmd.spawn(
	  VirtualJoystickBundle::new(VirtualJoystickNode {
	      // Idea
	      action: ClickColor(Color::WHITE.with_a(0.5)),
	  })
)

@clinuxrulz
Copy link
Contributor

clinuxrulz commented Feb 26, 2024

That is fair enough. Thank you for having a look.
I'll refactor it into that form a bit later and see what you think.
All I'd need to do is copy each of the system functions into trait impls.

@clinuxrulz
Copy link
Contributor

Here we go, attempt number 4:
https://github.com/clinuxrulz/virtual_joystick/tree/externalize-behaviors-attempt-4

Behaviors are now a variable that goes into VirtualJoystickNode.
I also added a Tint example. I have action called behavior though, but easy to rename it.

See what you think.

@SergioRibera
Copy link
Owner Author

Oooooh, your implementation is very similar to something I was working on, I'm still seeing things I like, like the autoimplementation macro, today I'll be upgrading to bevy 0.13, there I will share any details or implementations I can think of

@clinuxrulz
Copy link
Contributor

That's all cool. We'll go with yours.
I'm just providing demonstration code just incase your stuck.

I noticed when dealing direct with &mut World things got more tricky borrow checker wise compared to using a system with Querys. I was forced to create temporary child_entity Vecs to let go of my immutable reference to World so I can create a mutable reference to World. Though it should be safe to have a mutual reference to two different parts of World, but not possible for typechecker too see that there is no overlap in the mutable reference unlike in simple structs. E.g. MyStruct { a: f32, b: f32 }, you can have a mutable reference to a and b at the same time.

Anyway. Looking forward to seeing your implementation. I think this externalizing behaviors idea is good. Finally, if a end user needs a missing feature, then they can create a solution themselves without requesting more features of this library.

@SergioRibera
Copy link
Owner Author

@clinuxrulz bro, I was with very little time and trying to integrate it, I have not succeeded, do you dare to make a PR with one of your solutions? I would like to have something like that.

pub trait VirtualJoystickAction<I> {
    fn on_start_drag(
        &self,
        _id: I,
        _axis: VirtualJoystickAxis,
        _data: VirtualJoystickData,
        _world: &mut World,
        _entity: Entity,
    ) {
    }
    fn on_drag(
        &self,
        _id: I,
        _axis: VirtualJoystickAxis,
        _data: VirtualJoystickData,
        _world: &mut World,
        _entity: Entity,
    ) {
    }
    fn on_end_drag(
        &self,
        _id: I,
        _axis: VirtualJoystickAxis,
        _data: VirtualJoystickData,
        _world: &mut World,
        _entity: Entity,
    ) {
    }
}

The idea is that this only abstracts the behaviors related to joystick but with the game, internal joystick behaviors like movements and that, I would like it to be different.

I was already too late with the upgrade to Bevy 0.13

@clinuxrulz
Copy link
Contributor

There is no rush. Lets take time to think about it some more.
We can even just update the library as is for Bevy 0.13 with no changes to the API for now.

@SergioRibera
Copy link
Owner Author

There is no rush. Lets take time to think about it some more. We can even just update the library as is for Bevy 0.13 with no changes to the API for now.

I think it is a very good idea and thank you very much for your comments, even so I would like this feature to be an implementation of yours since I have seen that you have many different implementations for this solution.

@clinuxrulz
Copy link
Contributor

clinuxrulz commented Mar 23, 2024

I made a pull request and got as close as I can to your VirtualJoystickAction<I> trait.
See: #27

It has two types of externalization now, one for joystick behavior (VirtualJoystickBehavior) and one for responding to the drags (VirtualJoystickAction<I>).

I somehow broke my cargo, so I was unable to test it:

error: the 'cargo.exe' binary, normally provided by the 'cargo' component, is not applicable to the 'stable-x86_64-pc-windows-msvc' toolchain

Maybe you can test it for me.

@SergioRibera
Copy link
Owner Author

SergioRibera commented Mar 23, 2024

I made a pull request and got as close as I can to your VirtualJoystickAction<I> trait. See: #27

It has two types of externalization now, one for joystick behavior (VirtualJoystickBehavior) and one for responding to the drags (VirtualJoystickAction<I>).

You're the best, I will look the PR

I somehow broke my cargo, so I was unable to test it:

error: the 'cargo.exe' binary, normally provided by the 'cargo' component, is not applicable to the 'stable-x86_64-pc-windows-msvc' toolchain

Maybe you can test it for me.

No problem, I will test

@SergioRibera SergioRibera added this to the v2.3.0 milestone Mar 23, 2024
@SergioRibera
Copy link
Owner Author

@clinuxrulz about your problem with cargo, please try removing the rust-toolchain.toml file

@clinuxrulz
Copy link
Contributor

It's all good. The problem resolved itself somehow when I closed and opened the terminal window.

@SergioRibera
Copy link
Owner Author

The PR #27 was merged, do you consider that this issue can be closed?

@clinuxrulz
Copy link
Contributor

There will be some fine tuning we can do down the road. But yes we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hacktoberfest Hacktoberfest Participations help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants