Adds from_static_bytes function to Chunk struct#922
Adds from_static_bytes function to Chunk struct#922mnmaita wants to merge 1 commit intoRust-SDL2:masterfrom
Conversation
|
Oopsie, apparently I un-drafted this branch by mistake. For now it looks good to me, ping me whenever you have tested it! |
SpacingBat3
left a comment
SpacingBat3
left a comment
There was a problem hiding this comment.
(Also commenting since I saw this not to be implemented on upstream but made my own implementation as well, it is a bit different and made based on the code of different from_* methods, but I can't say to be better or anything. At least this is pretty much the reason I made this review, hopefully someone will find that useful. Also I can make a PR as well if anyone wants from me to publish my implementation.)
| if raw.is_null() { | ||
| Err(get_error()) | ||
| } else { | ||
| Ok(Chunk { | ||
| raw: raw, | ||
| owned: true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
I saw in different from_* methods that there was a private method to just do this stuff to avoid duplicates in the code. Any reason it isn't used there?
EDIT: I guess the code base is out-of-date just by looking at the extended diff… or at least different from the latest stable release. I guess this is also one of the things that should be checked after rebasing the fork and before merging it.
| let rw = unsafe { | ||
| sys::SDL_RWFromConstMem(buf.as_ptr() as *const c_void, buf.len() as c_int) | ||
| }; |
There was a problem hiding this comment.
I guess you could use RWops there (exactly RWops::from_bytes(buf)), I've seen other methods to do this and even make it a one-liner in raw declaration (avoiding declaring another var and doing another check I guess). I'm not sure about the benefits with that other than avoiding the use of another unsafe block (I'm not the Rust expert, so I might still be unfamiliar with that language design and quirks, same goes with SDL2).
|
Hi @SpacingBat3 ! This PR is so old that I barely remember I've ever made it. 😅 Feel free to work on top of it or submit your solution as a new PR! I might not be able to rebase and update this soon enough. |
3 participants
My first contribution here 😄.
This PR tries to address issue #743, I will be drafting this one until I can test it properly.
Suggestions and remarks are welcome! Thanks for creating this repo!