trumank / repak

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

features #4

Closed bananaturtlesandwich closed 1 year ago

bananaturtlesandwich commented 1 year ago

make encryption and compression capabilities optional, e.g for game-specific tools

trumank commented 1 year ago

I think I like conditionally compiling out whole functions rather than change arguments based on features (at least for the public API). How does this look?

I also fixed some clippy and test issues with particular feature combinations. Turns out cargo test-all-features is a great tool for checking these.

bananaturtlesandwich commented 1 year ago

Sorry but personally I think that conditionally compiling arguments is much cleaner. I don't see the point of 2 different functions for when encryption is enabled (which is by default) when one takes an option anyway. Also, if you have disabled the features that are on by default, then you clearly have the intent of not using the keys so why bother having a wrapper type that is now just a stub internally? It's still functionally the same as before but now it's just been overengineered in my opinion

bananaturtlesandwich commented 1 year ago

Also ooh didn't know about test-all-features that's very useful

trumank commented 1 year ago

The difference is that function signatures remain constant for all combinations of features. I find it annoying when looking up docs and then seeing functions have a different set of args than what I see in my editor.

bananaturtlesandwich commented 1 year ago

Then I can change the docs. Shrimple. Besides, if you're using lsp you would see the version of the function available

bananaturtlesandwich commented 1 year ago

redoing this in new PR since I want to avoid making a merge commit and it's better from fresh anyway