search

afritz1 / OpenTESArena

Open-source re-implementation of The Elder Scrolls: Arena.
MIT License
988 stars 68 forks source link

Address some warnings from CPPCheck #67

Closed Allofich closed 7 years ago

Allofich commented 7 years ago

This is just some cleanup, which I enjoy doing, to address warnings that show when checking the code with the CPPCheck code analyzer tool.

One change involved switching the order of arguments in the camera3D constructor from (position, direction) to (direction, position) to match the order the variables are declared. This is only called in one place right now but if you like the current order I could change this back.

Also, I didn't touch these but just in case it is useful for you, there are the following currently unused variables, according to CPPCheck:

Filename Lines Variable names IMGFile.cpp 207, 208, 209, 210 xoff, yoff, width, height CFAFile.cpp 29, 30, 72 unknown1, unknown2, dstOffset DFAFile.cpp 26, 27, 58 unknown1, unknown2, chunkSize ItemParser.cpp 36 text Renderer.cpp 360 screenHeight FontFile.cpp 59 counts

And the unused private function NonPlayer.h 34 NonPlayer::getAnimationType

afritz1 commented 7 years ago

I enjoy making sure code looks neat and tidy, too. I'll review the changes.

Allofich commented 7 years ago

OK, I'll start making these changes. If you want to do it yourself and just use this PR as a reference, that's fine, too, just say so.

afritz1 commented 7 years ago

Go ahead and make the changes. I'm looking at the unused variables and determining which ones to keep. I'll change those afterwards.

afritz1 commented 7 years ago

Some of these changes are different from my usual thinking, but maybe that just says that my style could use some more refining. When I bump into those places in the future, I may or may not tweak them a bit. For now, it's okay.

Allofich commented 7 years ago

Well, I was looking at the player constructor after your latest comment. It looks like just having the constructor declaration in player.hpp have the parameters in the same order as the variables are declared is enough to stop the warning in CPPCheck, even if the way the initialization is written in player.cpp is returned to how it was.

The whole fourth commit is to address the warnings about initialization lists not matching variable declaration order, so if that's not a concern to you, you could revert it if you want. But at any rate, thanks for merging.