search

Palm-Studios / sh3redux

SILENT HILL 3 Engine Remake in OpenGL and C++
GNU General Public License v3.0
162 stars 16 forks source link

sh3_arc* cleanup #109

Closed z33ky closed 7 years ago

z33ky commented 7 years ago

I decided the first step towards #95 is some cleanup in the arc* structs. The rest should follow in the next days so I'd suggest you leave this PR unmerged until they are in. But you can start reviewing each commit (I'm gonna change each struct in a separate commit), hence me opening the PR already.

Quaker762 commented 7 years ago

@z33ky Have you made the changes to this?? If so, I'll merge it :)

z33ky commented 7 years ago

Sorry, I've been distracted last weekend by a SBC I got and have been busy otherwise this week. I should be able to push two more commits today though; the changes are in, I just need to split them up a bit. After this there's only vfile left.

z33ky commented 7 years ago

There we go. I'll squash the fixup! after review and now only sh3_arc_vfile is missing the refactorization treatment.

Quaker762 commented 7 years ago

Looks really good, much cleaner than what we originally had. I'm curious though, how does this line std::string &&subarcName work, with the double ampersand??

Apart from that, squash when you're ready :)

z33ky commented 7 years ago

It's an rvalue-reference. Basically it's a reference where the caller promises that the value will not get used any further, which means you can "move" stuff out of it. So for the case of the std::string, the copy-constructor (assuming no CoW (Copy-on-Write) semantics) needs to allocate new memory and copy the characters over, while the move-constructor just takes the existing memory from the other string and changes that string to be empty, so its destruction will not delete the memory.

z33ky commented 7 years ago

Rebased onto master and squashed the fixup. There are two new commits on top for vfile with which I now consider this PR mergeable after you approve.