tiby312 / broccoli-project

MIT License
78 stars 2 forks source link

Owning version of Tree #168

Open WaDelma opened 2 years ago

WaDelma commented 2 years ago

I have physics system using specs that was previously using self-made broad phase. In addition of using the sorted spans in that system I also stash them to a Resource so that other systems can do queries against it.

I have now switched broad phase to broccoli, but it leaves question how to stash the tree I create to that resource. Here is my current very hacky solution:

// My resource
pub struct CollisionDetector {
    tree: Option<broccoli::Tree<'static, BBox<f32, Entity>>>,
    broccoli_contents_no_touchi: Vec<BBox<f32, specs::Entity>>,
}

// In the physics system
drop(detector.tree.take());
detector.broccoli_contents_no_touchi = // Get relevant data from specs
let mut tree = broccoli::new_par(RayonJoin, &mut detector.broccoli_contents_no_touchi);
// Do collision detection stuff
detector.tree = Some(unsafe {
    mem::transmute::<Tree<'_, BBox<f32, Entity>>, Tree<'static, BBox<f32, Entity>>>(broc)
});

I looked at some self-referential struct libraries such as self_cell (doesn't support mutable references), escher (specs wants Send and there is some raw pointer usage so it didn't work), but no avail. Maybe there is some library that would work, but it would be nicer if there was owned variant of the Tree.

WaDelma commented 2 years ago

I have certain system that does some background processing that takes more than one tick I have realized I could speed it up using broccoli. However, because it borrows mutably it cannot be cloned and sent to another thread. So for now I would have to redo the construction again if I wanted to use Tree there. (Possibly I could remove the background processing in this case as it'll probably be fast enough after using broccoli, but that is not the case in general)

tiby312 commented 2 years ago

@WaDelma if I am understanding you this luckily is already in there. check out broccoli::container::TreeOwned and broccoli::container::TreeIndOwned

WaDelma commented 2 years ago

Ah didn't notice those. Have to check them out.

romamik commented 7 months ago

It looks that TreeOwned was removed. There are tree.get_tree_data() and Tree::from_tree_data though.

tiby312 commented 7 months ago

Thank you for pointing that out. I have indeed removed TreeOwned and replaced it with what you mentioned. This was in an effort to remove all use of unsafe in the crate.

indubitablement2 commented 3 months ago

Handling lifetime is a bit of a headache right now. A simple Tree<Rect<T>, V> where the rects and values are kept into Tree would be really useful.

tiby312 commented 3 months ago

@indubitablement2 I agree. I think there should be like a simpler wrapper around using these Tree and TreeData structs, that makes some reasonable defaults.

In some cases, I think the user has their own container, and they want a tree to just reference that. In other cases, the user has their own container, but wants the tree to reference pointers to their container. That means that there needs to be a separate container of just the pointers. Right now you have to make that separate container yourself basically. In other cases, they don't need their own container and the tree could just be both the container and the tree. I struggled to find a simple api that could support these different usecases that also didnt result in using unsafe.