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

Don't mark useful functions as private #1349

Open
EriKWDev opened this issue Nov 11, 2023 · 2 comments
Open

Don't mark useful functions as private #1349

EriKWDev opened this issue Nov 11, 2023 · 2 comments

Comments

@EriKWDev
Copy link

EriKWDev commented Nov 11, 2023

Hey!
Great bindings and rust library for SDL that I've had extensive use of.

I have a problem though,
I wanted to call SDL_PeepEvents which gives me access to a raw sdl::sys::SDL_Event, but since the rest of my code is structured around this crate's nicer sdl::event::Event, I would preferably like to be able to easily convert it as you are already doing internally.

However, to my great disappointment, the functions Event::to_ll and Event::from_ll are marked as private, giving an unfortunate block to my adventures here. (UPDATE: see comment down below)

While I understand that this is more close to sdl::sys, I still feel like it is poor library design to have useful functions marked as private, especially a library implementing useful stuff ti interfacing with a C-library.

I feel like a public sys-glue module of sorts could be in-place here where such private functions could be marked public, or just be marked public unsafe in the first place.

Feels silly to have to fork this just to mark more functions as public if I need them in the future..

@EriKWDev
Copy link
Author

EriKWDev commented Nov 11, 2023

After looking into it it seems that the version I was using had Event::from_ll function marked as private, but it's now become public since #1210. My point still stands though that all these functions should be marked public in the first place and I see many others still private.

Would a PR changing them all to public be accepted?

@Cobrand
Copy link
Member

Cobrand commented Nov 11, 2023

Yes it would be accepted, but I feel like we should add a warning to all of themwhich are public, along the lines of "only use this is you know what you are doing" or something, because some structs are really dangerous to use if you don't know what you are doing.

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