leudz / shipyard

Entity Component System focused on usability and flexibility.
Other
738 stars 44 forks source link

AtomicU64 is not available on 32-bit platforms #181

Closed parasyte closed 1 year ago

parasyte commented 1 year ago

I'm building shipyard with --target thumbv7em-none-eabihf and getting a build error:

error[E0432]: unresolved import `core::sync::atomic::AtomicU64`
 --> /Users/parasyte/.cargo/registry/src/index.crates.io-6f17d22bba15001f/shipyard-0.6.2/src/scheduler/into_workload.rs:9:26
  |
9 | use core::sync::atomic::{AtomicU64, Ordering};
  |                          ^^^^^^^^^
  |                          |
  |                          no `AtomicU64` in `sync::atomic`
  |                          help: a similar name exists in the module: `AtomicU32`

For more information about this error, try `rustc --explain E0432`.
error: could not compile `shipyard` (lib) due to previous error

This specific target is a 32-bit ARM CPU, and is currently a tier 2 platform.

Switching to AtomicUsize might be a decent compromise to support 32-bit platforms. Or even:

#[cfg(target_has_atomic = "64")]
use core::sync::atomic::AtomicU64 as AtomicUInt;

#[cfg(not(target_has_atomic = "64"))]
use core::sync::atomic::AtomicU32 as AtomicUInt;

If either sounds like the way forward, I'll gladly submit a PR...

leudz commented 1 year ago

32bits seems plenty enough on all platforms. I expected to find more errors but it's the only one I could find. Don't hesitate to open other issues or reopen this one if you run into more errors.

parasyte commented 1 year ago

I'm able to build my project with that change! I haven't had a chance to test it (my device is being RMA'd) but as far as I know, that was the only issue.

It's very cool to see a powerful ECS that's usable on this tiny ARM Cortex-M7! The only other hiccup I had was trying to use !Send + !Sync types as uniques (this usually requires the thread_local feature which depends on the standard library). Since I don't have a standard library, I had to use World::run_with_data() instead.

The one limitation that I know of with this workaround so far is that these systems "with data" cannot be bundled into workloads. I don't know if that's a big deal, and it seems to work.

leudz commented 1 year ago

I should be able to remove the std dependency for thread_local. It comes from AtomicRefCell (the lock around components) needing a way to check ThreadId for !Send components. Currently it uses std::thread::current().id() but as long as there is a function to provide a unique id for threads, that would work. It shouldn't be a big change, I'll try to do it this weekend.


That's a creative way of using run_with_data. The main drawback as you already noticed is that it can't be used in workloads. Some people prefer not to use them but you should at least have the choice.

leudz commented 1 year ago

I haven't tested it but it should work in theory.

The new World::builder() accepts a function that returns the current thread ThreadId. I hope you have a function like that 😅

parasyte commented 1 year ago

Just getting back to you on the non-send-sync stuff. It works!

I had to do a little reverse engineering with:

cargo doc --no-deps --document-private-items --no-default-features --features 'thread_local'

... to find out how to use the "private-public" WorldBuilder, but otherwise no issues.

Thanks for this, BTW! shipyard has generally been a pleasure to use for a number of years, and these changes make it more accessible for small game platforms.


This fixes the docs for WorldBuilder and World::builder() linking to it, but the struct could use some more of its own documentation:

diff --git a/src/lib.rs b/src/lib.rs
index 4c6eab6..7f65c50 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -128,7 +128,7 @@ pub use views::{
     AllStoragesView, AllStoragesViewMut, EntitiesView, EntitiesViewMut, UniqueView, UniqueViewMut,
     View, ViewMut,
 };
-pub use world::World;
+pub use world::{World, WorldBuilder};

 #[cfg(not(feature = "std"))]
 type ShipHashMap<K, V> =

I hope you have a function like that 😅

Yeah, I guess I do. I don't have threads on this platform at all, so it's || 1!

leudz commented 1 year ago

It works!

Awesome!

I had to do a little reverse engineering [...] to find out how to use the "private-public" WorldBuilder

oops, it's fixed. And yeah I'll improve the docs, the methods could do with an example each and build should mention that the error message will point to what is missing.

Yeah, I guess I do. I don't have threads on this platform at all, so it's || 1!

I'll also add that to the docs, most platforms without std probably don't have threads.