xdanieldzd / Scarlet

Game data conversion/export/import helper libraries - UNMAINTAINED
Other
97 stars 22 forks source link

Add CPK container format, CRILAYLA compression and PSSG texture format #13

Closed darkstar closed 6 years ago

darkstar commented 6 years ago

This commit adds the CPK container and the CRILAYLA compression that is often used in PS3 CPK files, most notably the Neptunia games etc.

Also fixes an encoding issue in the NISPACK container, as the PS3 versions of those files often have ShiftJIS encoded filenames in them

If you think something needs changing, just let me know

xdanieldzd commented 6 years ago

Hey, thanks for the PR, it looks fine at first glance. What I do think would need changing in theory, tho, would be the naming style of various functions and variables, such as with ex. num_bits_produced in CRILAYLA's get_next_bits vs something like numBitsProduced in GetNextBits.

That said, I'm not necessarily always consistent with naming conventions, or, say, comment usage (C-style vs C++ style) and things like that, hence the "in theory", plus there's no formal style guide or anything yet.

I guess I'll look through the existing code and make a short style guide based on that today. Would you be alright with changing your PR's code (function and variable names, comments) to adhere to this guide once posted? Otherwise I'll merge the PR and do the changes myself sometime soon.

darkstar commented 6 years ago

Hey,

no problem I can make the required changes, don't worry. This is mostly a copy/paste of the other code, mostly to see if it actually works at all. I just wanted to do the PR early, in case someone else considers adding CPK containers, so that there's no wasted work

I'll take a closer look at the existing code (but a quick style guide would indeed be helpful) and clean up the PR

Do you prefer a rebased/force-updated PR, or should I just add commits to this one cleaning up the naming conventions? I think I would prefer the rebase/force-update route so that the weird names don't end up in the repo at all

Also, do you want the NISPACK fix separately? It is, technically, not part of the CPK/CRILAYLA stuff. You can also cherry-pick that one and I will redo the other two (CPK/CRILAYLA)

xdanieldzd commented 6 years ago

I posted some style guidelines on Sunday already, but didn't really have time to comment on the PR again.

As for how to continue here, I'm admittedly not very well-versed in Git's functionality yet, especially anything related to PRs - I've had less than 5 of them come from other users since I started using GitHub.

For now, I'd say to continue adding commits to this one for the naming conventions; I'd also just keep the NISPACK fix here, too. In the meantime, I'll get more familiar with Git cherry-picking, etc. for next time.

darkstar commented 6 years ago

I updated the PR and refactored the names a bit. If you spot anything still remaining feel free to comment directly in the commits on the respective line

About the cherry-picking: Basically, what you do is you add my tree as additional remote (e.g. git remote add darkstar https://github.com/darkstar/Scarlet.git), fetch this tree into your local repo (git fetch darkstar fixes) and then pick a particular commit into your master tree (git cherry-pick 47d998b). After that you can delete the remote again (git remote remove darkstar)

darkstar commented 6 years ago

I added support for the PSSG image file format too... This is heavily used in PS3 assets for the Neptunia games, for example.

xdanieldzd commented 6 years ago

Changes merged, tho I could only test CPK and CRILAYLA support myself; Superdimension Neptune on PS Vita uses both as well. Thank you very much!

darkstar commented 6 years ago

If you have a repo or something for sample files I could provide you with a few small cpk files that include pssg images...