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

Add support for 32-bit #347

Open
Esption opened this issue Jul 17, 2023 · 6 comments
Open

Add support for 32-bit #347

Esption opened this issue Jul 17, 2023 · 6 comments
Labels
c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library

Comments

@Esption
Copy link

Esption commented Jul 17, 2023

Trying to do cargo build --target i686-pc-windows-msvc fails to build. It is possible to go in and get it to manually compile by editing godt-ffi/src/opaque.rs to have an align(4) instead of 8, and the OpaqueObject in godot-ffi/src/gen/central.rs from 8uszie to 4usize (you need to wait until after you see the fail build for that, because of codegen). Likely there are other things in that need to be updated as well, but at a minimum that will successfully cause it compile. I wasn't able to figure out where that codegen is actually happening. After editing those two things, the creeps example will start and run, but appears to crash on the anim_names.to_vec() in the rust/src/mob.rs

@lilizoey
Copy link
Member

lilizoey commented Jul 17, 2023

i think we need to change this:

    // For float/double inference, see:
    // * https://github.com/godotengine/godot-proposals/issues/892
    // * https://github.com/godotengine/godot-cpp/pull/728
    #[cfg(feature = "double-precision")]
    let build_config = "double_64"; // TODO infer this
    #[cfg(not(feature = "double-precision"))]
    let build_config = "float_64"; // TODO infer this

in here to the respective x_32 build configs for this.

@lilizoey lilizoey added bug c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library and removed bug labels Jul 17, 2023
@Esption
Copy link
Author

Esption commented Jul 18, 2023

So, I'm pretty sure the codegen for the Opaques originates from godot4-prebuilt::load_gdextension_json() called in godot-codegen/src/lib.rs
I guess just need to give that a different target?

@lilizoey
Copy link
Member

yea probably, we can likely use https://doc.rust-lang.org/reference/conditional-compilation.html#target_pointer_width this cfg flag to decide between which representation to use

@Esption
Copy link
Author

Esption commented Nov 19, 2023

Not sure if should mention here or over there, but
https://github.com/godot-rust/godot4-prebuilt
seems to explictly generate for 64-bit targets. It seems fine for the most part with one exception. The built-in tests will fail due to the widths assuming 64-bit. If we added other targets, it would be nice to get win32/linux32/wasm added in there. Example command is to just run cargo test --no-default-features --target i686-pc-windows-msvc

Best I can tell, that's the only reason the itest suite fails as a whole against 32-bit targets, but hard to tell atm.

@Bromeon
Copy link
Member

Bromeon commented Jan 5, 2024

So this would need:

  • a godot4-prebuilt configuration which generates artifacts for 32-bit
  • a CI job in gdext that runs unit tests for it
    Anything else?

Probably it's good enough to combine this with WASM for now...

@Esption
Copy link
Author

Esption commented Jan 5, 2024

AFAIK yes that should be the final thing to tick off 32-bit support.

@Bromeon Bromeon added c: wasm WebAssembly export target and removed c: wasm WebAssembly export target labels Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library
Projects
None yet
Development

No branches or pull requests

3 participants