Hi again, (I'm spamming today ;])
@Paul
Well , origin/ paul424-integration right ?
First of all, nice work done there. There are a few things but I can feel the beast growing nicely. An extra point for certain commits fixing indentation and adding headers, I appreciate it.
Now for the requested fixes before merging:
- There are broken indentations in SlopWalk.cpp.
- Please use only spaces, not tabs in the gui files, such as the looknfeel one, to be in sync with what's done in the source code.
- Could you rename the Channel* console commands to something likr Debug* or DbgChannel* since it's more meaningful to me? (What do you think?)
- You've added ${SRC}/ChannelDebug.cpp twice in the CMakeLists.txt, please remove one of them.
- As I told you before:
+ channelInfo[static_cast<int>(ChannelColors::navy)]="Used for SlopeWalk logging";
+ channelInfo[static_cast<int>(ChannelColors::blue)]="Used for Culling Tile mechanism logging";
^ This above is the very evidence the channels shouldn't use colors but more meaningful names. Please consider renaming the used ones with more useful names for the filenames and internal channel names. (The color used can remain, of course.)
I also wondered whether the channel description could be logged as the first thing in the corresponding file, so that when you open the file, what's inside is first described. What do you think?
- Consider renaming ChannelColors to DebugChannelColors.
- While I see it, please don't use ';' at the end of function declarations, only for classes and structs.
- Consider changing the 17 magic number with a more meaningfull const int member, such as const int NUM_DBGCHANNELS = 17;
- Consider adding m in front of class members. I.e: std::array<string, 17> channelInfo; -> std::array<string, 17> mChannelInfo;
- Consider renaming CullingManager::newBashAndSplashTiles(__int128 mode) -> CullingManager::bashAndSplashTiles(__int128 mode) since it's now the only function there.
- Consider using the Allman style, even for headers, please. http://en.wikipedia.org/wiki/Indent_style#Allman_style
Once we have all this settled out, I'll be ok for you to merge.
When merging, have an extra care not to add gui/ files to the code repo, btw, and when merging the gui files in SVN, please have the care not do delete the recent work done on
the gui files.
Could you also remove the paul424-stacktrace/ branch if you have finished with it?
What are you going to do about the image2map/ branch?
Regards,