Skip to content

fix bindgen #116

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

Merged
merged 10 commits into from
Mar 14, 2025
Merged

fix bindgen #116

merged 10 commits into from
Mar 14, 2025

Conversation

khanghugo
Copy link
Contributor

Bindgen 0.71 and latest rust nightly version.

Bindgen does not right away so there are some changes to bindgen such as c_uint to c_int or *mut T to *const T. With that, bxt-rs code can remain unchanged. Bindgen also makes function type to be Option<unsafe extern ...> instead of the usual unsafe extern .... So, for some parts, I add .unwrap() wherever applicable. There are a lot of changes needed doing after generating bindings so I write all instructions along with the bindgen command inside each binding files.

Some files are deleted because the new bindgen conveniently generates bunch of related structs. There are some re-declared structs such as hull_s where its members are defined in some files and not defined in some files. Doesn't seem like a problem.

I am able to load a TAS and interact with the editor just fine. If anything to worry about, it is the tas_server_time_fix because there is a small change to the code that does pointer comparison. Though the changed code is from rust_analyzer suggestion so I guess it is all okay.

@YaLTeR
Copy link
Owner

YaLTeR commented Mar 2, 2025

Oh right, I forgot I did a bunch of manual cleanup on the generated bindings. Thanks for documenting the steps.

Bindgen also makes function type to be Option<unsafe extern ...> instead of the usual unsafe extern .... So, for some parts, I add .unwrap() wherever applicable.

I got rid of all Option<> manually in the generated bindings, I think that's a better move here rather than adding unwrap(), since we know that those pointers are always present. (Only leave Option<> when the pointer can actually be NULL.)

I also replaced all ::std::os::raw:: and such with use manually, it's easy enough, just do a search and replace for ::std::os::raw:: to nothing, and same for ::std::mem:: (add the new offset_of import). ::std::option:: can also be removed.

For types like vec3_t I replaced them with [f32; 3] manually to avoid these spammy type aliases. Also doable with a search and replace. Same with edict_s -> edict_t etc.

Remove Copy and Clone too since the types are not semantically copyable.

Also if you don't mind, move structs like usercmd_s back to their separate files.

With these cleanups it should be easy to see if anything changed in the bindings unexpectedly. Actually I don't think much should change in the actual types, it's the layout tests that changed.

I don't think we need to regen this often, so it's fine that more manual work is involved. Actually I doubt we'll have to regen this for a while if at all because the major problem with bindgen layout tests had been fixed now with offset_of! and I don't think there are any other big problems.

@khanghugo
Copy link
Contributor Author

Take a look again and see. Doesn't seem to have any problem loading and interacting with a TAS

Copy link
Owner

@YaLTeR YaLTeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks much better. Left some suggestions and comments

@YaLTeR YaLTeR merged commit 9dfe3e3 into YaLTeR:master Mar 14, 2025
13 checks passed
@YaLTeR
Copy link
Owner

YaLTeR commented Mar 14, 2025

Thanks, I hope I didn't break anything with those last changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants