betajaen / gorilla

Minimal HUD and Overlay replacement system for Ogre 1.x only
60 stars 11 forks source link

Made OgreConsoleForGorilla -Wall friendly. #5

Closed HeatherSoron closed 12 years ago

HeatherSoron commented 12 years ago

Most of the fixes I applied here are pretty straightforward. Reordering member variable initialisation, making sure that we don't compare signed and unsigned ints, getting rid of two unused variables, that sort of thing.

There is one line that may deserve more scrutiny: the old legalchars[c]==arg.text line. I'm pretty sure that casting arg.text to a char is an appropriate way to resolve the signed vs. unsigned warning, and I'm pretty sure that a static cast is appropriate, but that one wasn't a COMPLETE no-brainer (i.e., if this were Wikipedia, I'd hesitate before marking that as a minor edit).

Also, note: I still haven't looked at the example code, and have no idea whether or not THAT issues warnings with -Wall.

merlinblack commented 12 years ago

Just the console code? I did the whole lot! ;-P Yeah that legal chars bit had me wondering exactly what to do about it. I made the array unsigned, as well as the other vars rather than put casts everywhere.

Have a look at the new temporary branch 'warnings_tidy_up'. I'm guessing we made mostly similar changes.

HeatherSoron commented 12 years ago

Heh, good on ya for doing the whole lot - I was just focused on the bits that I needed for my app, for the moment ;).

And yeah, sticking unsigned all over the place seemed appropriate, although that one char comparison seemed a bit less obvious. And yeah, I'll diff the two branches, I imagine that the main difference would be formatting and such.

HeatherSoron commented 12 years ago

Yep, those two files had all the same changes in both versions, aside from the legal chars array. Yours was cleaner on that one, too ;). I hadn't thought of declaring it an unsigned char array.

Feel free and close this issue, then.

merlinblack commented 12 years ago

Ok will do. I'm pretty sure it hasn't broken anything, I'm going to merge the new branch into master.