Skip to content

Avoid static global by using user_data to pass state instead. #30

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 1 commit into from
May 20, 2024

Conversation

gw3583
Copy link

@gw3583 gw3583 commented May 17, 2024

No description provided.

@gw3583
Copy link
Author

gw3583 commented May 17, 2024

This only updates the cube example to avoid a global static. If you're generally happy with this change, I can update the other examples to be similar.

@floooh
Copy link
Owner

floooh commented May 17, 2024

Great fix thanks :) I think it makes sense to apply this to all samples, and that way we are at least safe from the deprecation warning becoming an error at some point.

Eventually I'd like to get rid of the unsafe in the samples entirely, maybe by adding some sort of "SokolApp" wrapper/trait/superclass thingie which somehow would provide a "safe-wrapper" for this unsafe user data handling (internally it would probably do the same thing as your fix).

My Rust-foo isn't good enough to know what a proper idiomatic Rust solution would look like though.

@gw3583
Copy link
Author

gw3583 commented May 18, 2024

Here you go, examples and readme updated. The basic idea of a wrapper with a trait sounds good as a future solution.

@floooh
Copy link
Owner

floooh commented May 20, 2024

Giving this a whirl now...

@floooh floooh merged commit fe77299 into floooh:main May 20, 2024
2 checks passed
@floooh
Copy link
Owner

floooh commented May 20, 2024

...and merged. Many thanks, it's nice to see a warning-free build again :)

@gw3583 gw3583 deleted the mut-state branch May 26, 2024 21:06
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