Anders429 / brood

A fast and flexible entity component system library.
Apache License 2.0
39 stars 1 forks source link

Require components viewed in `Schedule` to implement `Sync`. #207

Closed Anders429 closed 1 year ago

Anders429 commented 1 year ago

Just requires Task::View to implement Send (which is equivalent to the components themselves implementing Sync, as T implements Send if and only if &T implements Sync). See the added trybuild test for an example that previously (incorrectly) compiled but now does not.

Not sure how many users would actually be affected by this. Most components stored in ECS patterns are fairly straight-forward, but I could see some user trying to store an Rc for some shared state between entities or something.

In the future, there could be a special Task type that specifies which components are non-Sync and can therefore not be borrowed simultaneously between two threads. This would allow all components to be used in schedules again. Not sure what the best API for such a feature is.

Anders429 commented 1 year ago

Code coverage loss is, again, expected, because this PR touched src/system/schedule/mod.rs's tests. I ought to figure out how to fix that.

codecov[bot] commented 1 year ago

Codecov Report

Merging #207 (ddcaebc) into dev (65e1d95) will decrease coverage by 0.05%. The diff coverage is 75.75%.

@@            Coverage Diff             @@
##              dev     #207      +/-   ##
==========================================
- Coverage   95.45%   95.40%   -0.05%     
==========================================
  Files          89       89              
  Lines       14082    14115      +33     
==========================================
+ Hits        13442    13467      +25     
- Misses        640      648       +8     
Impacted Files Coverage Δ
src/system/schedule/task/sealed.rs 100.00% <ø> (ø)
src/system/schedule/mod.rs 74.00% <75.75%> (+0.07%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.