Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bevy_ecs: Allow using i32 for entity ID cursor on platforms without AtomicI64 support #4451

Closed
ian-h-chamberlain opened this issue Apr 10, 2022 · 0 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible

Comments

@ian-h-chamberlain
Copy link
Contributor

What problem does this solve or what need does it fill?

I have been trying to get bits and pieces of Bevy building for an unusual target platform (armv6k-nintendo-3ds), and to my surprise bevy_ecs almost compiles already!

The only issue I ran into was in https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/entity/mod.rs#L35 – since the platform does not have atomic load/stores for i64, there is no AtomicI64 available for import. I imagine that some other 32-bit platforms may also run into this problem, but have probably not been the focus of the project thus far.

What solution would you like?

With Rust 1.60.0 the #[cfg(target_has_atomic = "64")] flag has just been stabilized: rust-lang/rust#93824. I propose using this to conditionally compile using AtomicI32 on platforms that do not support 64-bit atomics.

I'm not sure if there might be any further-reaching consequences of such a change, but this platform is already not supported, so it seems like adding support in this way would still be "experimental" or possibly unstable, but still an improvement on the status quo. Existing platforms' implementation would not be impacted.

What alternative(s) have you considered?

No action; effectively require using a fork for platforms without atomic i64 support.

Additional context

I already have a local change with which I have successfully built for armv6k-nintendo-3ds – I'm happy to open a PR if this proposed solution is accepted.

@ian-h-chamberlain ian-h-chamberlain added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Apr 10, 2022
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Apr 11, 2022
@bors bors bot closed this as completed in cde5ae8 Aug 21, 2022
maccesch pushed a commit to Synphonyte/bevy that referenced this issue Sep 28, 2022
…evyengine#4452)

# Objective
- Fixes bevyengine#4451

## Solution
- Conditionally compile entity ID cursor as `AtomicI32` when compiling on a platform that does not support 64-bit atomics.

- This effectively raises the MSRV to 1.60 as it uses a `#[cfg]` that was only just stabilized there. (should this be noted in changelog?)

---

## Changelog
- Added `bevy_ecs` support for platforms without 64-bit atomic ints


## Migration Guide
N/A
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
…evyengine#4452)

# Objective
- Fixes bevyengine#4451

## Solution
- Conditionally compile entity ID cursor as `AtomicI32` when compiling on a platform that does not support 64-bit atomics.

- This effectively raises the MSRV to 1.60 as it uses a `#[cfg]` that was only just stabilized there. (should this be noted in changelog?)

---

## Changelog
- Added `bevy_ecs` support for platforms without 64-bit atomic ints


## Migration Guide
N/A
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
…evyengine#4452)

# Objective
- Fixes bevyengine#4451

## Solution
- Conditionally compile entity ID cursor as `AtomicI32` when compiling on a platform that does not support 64-bit atomics.

- This effectively raises the MSRV to 1.60 as it uses a `#[cfg]` that was only just stabilized there. (should this be noted in changelog?)

---

## Changelog
- Added `bevy_ecs` support for platforms without 64-bit atomic ints


## Migration Guide
N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Projects
None yet
2 participants