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

Cool project! Some questions/comments #3

Closed
BrettHemes opened this issue May 10, 2022 · 1 comment
Closed

Cool project! Some questions/comments #3

BrettHemes opened this issue May 10, 2022 · 1 comment

Comments

@BrettHemes
Copy link

BrettHemes commented May 10, 2022

Awesome work porting this! I currently have a C++ application using Box2d and was contemplating the merit of switching to native rust. After a quick search I came across your library and was thrilled to see a native port that is in working condition. I have started porting my code into rust and, so far, I have the following (very minor) notes:

  • Crate name has _rs suffix. From the naming guidelines: "Crate names should not use -rs or -rust as a suffix or prefix. Every crate is Rust! It serves no purpose to remind users of this constantly." Is this because of the other box2d crate that existed already?
  • CamelCase type inconsistency. As an example, B2vec vs B2Vec. This seems to happen throughout the library with some types assuming the first style and some the latter.
  • In my application, I had some issues using the testbed as a template with the latest glium (v0.31.0) + imgui/winit (e.g., WinitPlatform::attach_window). Is there a reason you aren't using the latest glium? I ask in case I should be on v0.30.0 as well.

Some other questions:

  • What is your current release/support plan?

Hopefully I am not being to picky; I am just excited to use this library and would like to help / see it mature. In general, how would you prefer me reporting stuff I find as I dig deeper?

@HumMan
Copy link
Owner

HumMan commented May 20, 2022

Awesome work porting this! I currently have a C++ application using Box2d and was contemplating the merit of switching to native rust. After a quick search I came across your library and was thrilled to see a native port that is in working condition. I have started porting my code into rust and, so far, I have the following (very minor) notes:

  • Crate name has _rs suffix. From the naming guidelines: "Crate names should not use -rs or -rust as a suffix or prefix. Every crate is Rust! It serves no purpose to remind users of this constantly." Is this because of the other box2d crate that existed already?
  • CamelCase type inconsistency. As an example, B2vec vs B2Vec. This seems to happen throughout the library with some types assuming the first style and some the latter.
  • In my application, I had some issues using the testbed as a template with the latest glium (v0.31.0) + imgui/winit (e.g., WinitPlatform::attach_window). Is there a reason you aren't using the latest glium? I ask in case I should be on v0.30.0 as well.

Some other questions:

  • What is your current release/support plan?

Hopefully I am not being to picky; I am just excited to use this library and would like to help / see it mature. In general, how would you prefer me reporting stuff I find as I dig deeper?

Hi, I'm glad the project was helpful to you.
You are right about all 3 notes

  • Crate name has _rs suffix - because of existing old box2d crate
  • CamelCase type inconsistency - I completely relied on rust-analyzer's camelcase warnings, both B2vec vs B2Vec seems valid to it, so inconsistency exists
  • Issues with glium (v0.31.0) - when i try to use glium (v0.31.0) i get errors, i didn't dig deeper in trying to fix it,
mismatched types
expected struct `winit::window::Window`, found `&glium::glutin::window::Window`
note: expected reference `&winit::window::Window`
         found reference `&&glium::glutin::window::Window`rustc(E0308)
main_loop.rs(72, 48): expected struct `winit::window::Window`, found `&glium::glutin::window::Window`

image

and others relaited to prepare_frame, prepare_render, render, handle_event.
At this moment i have no fix to this.

  • What is your current release/support plan? - release plan depends of original Box2D repository, when erincatto create new release i will start porting all new things.
    All other fixes not related to original repository we can release as needed.

Feel free to create any issues, pull requests

@HumMan HumMan closed this as completed Jan 28, 2023
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

No branches or pull requests

2 participants