leudz / shipyard

Entity Component System focused on usability and flexibility.
Other
755 stars 46 forks source link

Local<T> paramters in systems? #209

Closed BlackNinja745 closed 2 weeks ago

BlackNinja745 commented 3 weeks ago

Hey, love how easy shipyard makes ECS! I'm not sure if this would be possible as I'm not super familiar with how the shipyard world handles storing systems, but could shipyard get a Local feature like bevy?

Here's bevy docs on the feature: https://docs.rs/bevy/latest/bevy/ecs/system/struct.Local.html

A local may only be accessed by the system itself and is therefore not visible to other systems. If two or more systems specify the same local type each will have their own unique local. If multiple SystemParams within the same system each specify the same local type each will get their own distinct data storage.

The addition of this feature would not only make it so you could have a different instance of unique like data per system, but it would also make having data that needs to persist between multiple calls to the system easier, as you don't need to wrap it in any other type.

I'm currently working on a project that heavily uses shipyard and many of my Uniques are just data that is only needed in one system to persist state between multiple calls.

If this is added, I don't think the for Local, T should be restricted by any trait the same way Unique and component is, since that would defeat the purpose of not needed to wrap your data in another type for data just needed to persist between different calls to a system. Additionally, T isn't Default, maybe a method like .get_or_init_with(f: impl FnOnce() -> T)/get_or_init(T) would be useful?

Here's a really quick attempt at getting it to work, but this is not ideal at all since you still have to initialize it somewhere at the start.

use shipyard::{Unique, UniqueViewMut, World};

#[derive(Unique)]
pub struct Local<T: Sync + Send + 'static, const TAG: usize>(pub T);

fn main() {
    let world = World::new();

    world.add_unique(Local::<_, 1>(5.0f32));
    world.add_unique(Local::<_, 2>(3.0f32));

    for _ in 0..3 {
        world.run(a);
        world.run(b);
    }
}

fn a(mut local_f: UniqueViewMut<Local<f32, 1>>) {
    local_f.0 += 1.0;
    println!("a: {}", local_f.0);
}

fn b(mut local_f: UniqueViewMut<Local<f32, 2>>) {
    local_f.0 += 1.0;
    println!("b: {}", local_f.0);
}

Thanks for your consideration!

leudz commented 2 weeks ago

Hi, there is a very old issue about this https://github.com/leudz/shipyard/issues/35 (just for context). This is so old, shipyard was still using a system proc macro. I closed it because of custom storages but you'd have to make a type per system, it wouldn't be a single type like Local.


I'm not opposed to add a version close to Bevy's Local but it would not be easy.

I'm not super familiar with how the shipyard world handles storing systems

Shipyard only stores systems in workloads. For example World::run doesn't store anything. Because of that, the type is very generous. For example the closure can be not 'static. Which makes storing the Local hard (impossible?).

For systems in workloads Local could work but the implementation would be more complicated than Bevy's. Shipyard stores systems as Fn as opposed to FnMut. This means the trick in Bevy's doc doesn't work and implementing it similarly wouldn't work either. The Local would have to be stored somewhere else.

BlackNinja745 commented 2 weeks ago

Oh okay, that makes a lot of sense, thank you!

To mimic a Local functionality, do you think it would be possible to have something similar to one of these?

fn system(unique: Default<UniqueView<T>>) {
    // auto add's T's Default implmentation if it hasn't been added yet
}

fn system(unique: GetOrInit<UniqueView<T>>) {
    let view: UniqueView<T> = unique.get_or_init(|| /* initialize here */);
}

This might allow for local-like behavior, since you could define a wrapper type and the system in the module and only publicly expose the function, without having to worry about initializing the unique somewhere, if that makes sense.

My main thing is I don't want to have to initialize the unique for data that I'm only using in one system, it would be much nicer to have a way to initialize it in the system itself, if that makes sense.

leudz commented 2 weeks ago

These are possible. Here's GetOrDefault:

GetOrDefault ```rust use crate::get_or_default::GetOrDefault; use shipyard::{Unique, World}; fn main() { let world = World::new(); for _ in 0..3 { world.run(a); world.run(b); } } fn a(mut local_f: GetOrDefault) { local_f.0 += 1.0; println!("a: {}", local_f.0); } fn b(mut local_f: GetOrDefault) { local_f.0 += 1.0; println!("a: {}", local_f.0); } #[derive(Unique)] struct LocalF1(f32); impl Default for LocalF1 { fn default() -> Self { Self(5.0) } } #[derive(Unique)] struct LocalF2(f32); impl Default for LocalF2 { fn default() -> Self { Self(3.0) } } mod get_or_default { use shipyard::{ error::GetStorage, ARef, Borrow, BorrowInfo, Unique, UniqueViewMut, World, WorldBorrow, }; use std::ops::{Deref, DerefMut}; pub struct GetOrDefault<'v, T: Unique + Default>(UniqueViewMut<'v, T>); impl<'v, T: Unique + Default> Deref for GetOrDefault<'v, T> { type Target = T; fn deref(&self) -> &Self::Target { &self.0 } } impl<'v, T: Unique + Default> DerefMut for GetOrDefault<'v, T> { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } impl<'v, T: Unique + Default> WorldBorrow for GetOrDefault<'v, T> { type WorldView<'a> = GetOrDefault<'a, T>; fn world_borrow( world: &World, last_run: Option, current: shipyard::TrackingTimestamp, ) -> Result, shipyard::error::GetStorage> { let all_storages = world .all_storages() .map_err(|err| shipyard::error::GetStorage::AllStoragesBorrow(err))?; match all_storages.borrow::>() { Ok(_) => {} Err(err) => match err { GetStorage::StorageBorrow { .. } => return Err(err), GetStorage::MissingStorage { .. } => all_storages.add_unique(T::default()), _ => unreachable!(), }, }; let (all_storages, all_borrow) = unsafe { ARef::destructure(all_storages) }; let view = UniqueViewMut::borrow(all_storages, Some(all_borrow), last_run, current)?; Ok(GetOrDefault(view)) } } unsafe impl<'v, T: Unique + Default> BorrowInfo for GetOrDefault<'v, T> { fn borrow_info(info: &mut Vec) { UniqueViewMut::::borrow_info(info); } fn enable_tracking( _enable_tracking_fn: &mut Vec< fn(&shipyard::AllStorages) -> Result<(), shipyard::error::GetStorage>, >, ) { } } } ```

GetOrInit would be just a bit more complicated. It would have to hold the AllStorages reference and maybe an Option<UniqueViewMut<T>> or maybe a LazyLock.

BlackNinja745 commented 2 weeks ago

Thanks, that makes a lot of sense! Do you think it's possible to include the GetOrDefault in the next version of shipyard?

leudz commented 2 weeks ago

Yeah sure, I'll release a new one soon because of https://github.com/leudz/shipyard/issues/210.

BlackNinja745 commented 2 weeks ago

Thank you so much!

leudz commented 2 weeks ago

New version is up with a few new view types. UniqueOrDefaultViewMut is likely the one you want.