search

trumank / repak

Unreal Engine .pak file library and CLI in rust
Apache License 2.0
174 stars 25 forks source link

Features redo #6

Closed bananaturtlesandwich closed 1 year ago

bananaturtlesandwich commented 1 year ago

redoes #4 so that there are separate methods for each feature if you want to change the function names to something less verbose go ahead not sure how to mark functions as requiring a feature tho also had the idea of making new_any_with_key accept an aes key and not excluding new_any when encryption is enabled but a lot of the time the user will probably be using options so not 100% sure on that

trumank commented 1 year ago

I much preferred the PakEncryption enum that held the key if encryption was enabled as it means significantly fewer #[cfg(feature = "encryption")] instances scattered around. (Quick ctrl+f shows 26 in this PR and 11 with PakEncryption). When encryption is disabled it should compile to a zero-sized type so there shouldn't be any cost associated with it anyway.

bananaturtlesandwich commented 1 year ago

but the pakencryption means that you still have the key clones no? and I don't see the harm in more cfg attributes

bananaturtlesandwich commented 1 year ago

oh no it doesn't actually! That was the main thing reason I didn't want it since I now understand wanting separate functions for each function. I'll put it in

bananaturtlesandwich commented 1 year ago

sorry for being way too stubborn with this

bananaturtlesandwich commented 1 year ago

all this still stands

if you want to change the function names to something less verbose go ahead. not sure how to mark functions as requiring a feature tho. also had the idea of making new_any_with_key accept an aes key and not excluding new_any when encryption is enabled but a lot of the time the user will probably be using options so not 100% sure on that

bananaturtlesandwich commented 1 year ago

oh yeah tests are failing when default stuff is disabled gotcha will fix