0.1.1 (older version) Thoroughness: Medium Understanding: Low
by MaulingMonkey on 2019-09-11
This review is from Crev, a distributed system for code reviews. To add your review, set up cargo-crev
.
The current version of Legion is 0.4.0.
0.1.1 (older version) Thoroughness: Medium Understanding: Low
by MaulingMonkey on 2019-09-11
Crates in the crates.io registry are tarball snapshots uploaded by crates' publishers. The registry is not using crates' git repositories. There is absolutely no guarantee that the repository URL declared by the crate belongs to the crate, or that the code in the repository is the code inside the published tarball.
To review the actual code of the crate, it's best to use cargo crev open legion
. Alternatively, you can download the tarball of legion v0.4.0 or view the source online.
A low-boilerplate, high performance archetype based ECS.
Pros:
Cons:
0.1.1
src/borrows.rs
Borrow
beforevalue
lifetime has ended.Borrow
beforevalue
lifetime has ended.Borrow
beforevalue
lifetime has ended.Borrow
beforevalue
lifetime has ended.Borrow
beforevalue
lifetime has ended.src/lib.rs
unsafe { ... }
- I think this might be sound thanks to&mut self
implying we have exclusive access to self.archetypes[?].chunks()[?].entities()unsafe { ... }
- I think this might be sound thanks to&mut self
implying we have exclusive access to self.archetypes[?].chunks()[?].entities()unsafe { ... }
- I think this might be sound thanks to&mut self
implying we have exclusive access to self.archetypes[?].chunks()[?].entities()unsafe { ... }
- I think this might be sound thanks to&mut self
implying we have exclusive access to self.archetypes[?].chunks()[?].entities()unsafe { ... }
- Probably sound - index should be valid - but why not just `.map(unsafe { ... }
- I think this might be sound thanks to&mut Chunk
implying we have exclusive access to chunk.entities()src/query.rs
unsafe { ... }
-0 <= i < j < types.len()
, so both calls to get_unchecked should be sound.unsafe { ... }
- Sketchy, assumes data.len() <= entities.len() and that's not a particularly trivial assertion to prove.unsafe { ... }
- NFI if this is sound or not.src/storage.rs
&mut self
, this forces the caller to enforce borrow checking manually.unsafe { ... }
- I think this might be sound thanks to&mut self
unsafe { ... }
- Unsound? It's not clear what, if anything, ensures len() isn't being mutated by another borrower.&mut self
, this forces the caller to enforce borrow checking manually.unsafe { ... }
- Unsound? "Locks" via borrow types after constructing&T
, which is too late.unsafe { ... }
- Unsound? "Locks" via borrow types after constructing&mut T
, which is too late.unsafe { ... }
- NFI if this is sound or not. Haven't groked why SharedComponentStore is UnsafeCell or what protects it, if anything.unsafe { ... }
- I think this might be sound thanks to&mut self
Miri Example
TIL
TypeId::of::<T>()