dgcor / DGEngine

An implementation of the Diablo 1 game engine
Other
243 stars 30 forks source link

Warning: comparison between signed and unsigned integer expressions #11

Closed mewmew closed 7 years ago

mewmew commented 7 years ago

When compiling with GCC 6.2.1, the following warnings are identified for rev b98b54f87179481e7766e7512402d85379ebf240:

[  6%] Building CXX object CMakeFiles/DGEngine.dir/src/Cel.cpp.o
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp: In function ‘int32_t normalDecode(const std::vector<unsigned char>&, size_t, const Palette&, std::vector<sf::Color>&, bool)’:
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:172:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
    for (j = 1; j < frame[i] + 1 && i + j < frame.size(); j++)
                ~~^~~~~~~~~~~~~~
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:177:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (index > frame.size() - 1) {
         ~~~~~~^~~~~~~~~~~~~~~~~~
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:189:25: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
    for (size_t j = 0; j < 256 - frame[i]; j++) {

/home/u/Desktop/diablo/DGEngine/src/Cel.cpp: In function ‘int32_t cl2Decode(const std::vector<unsigned char>&, const Palette&, std::vector<sf::Color>&)’:
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:254:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     for (j = 1; j < val + 1 && i + j < frame.size(); j++)
                 ~~^~~~~~~~~
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:259:16: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
      if (index > frame.size() - 1) {
          ~~~~~~^~~~~~~~~~~~~~~~~~
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp: In function ‘void fillTransparent(size_t, std::vector<sf::Color>&)’:
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:354:22: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  for (int px = 0; px < pixels; px++) {
                   ~~~^~~~~~~~
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp: In function ‘void decodeGreaterLessThan(const std::vector<unsigned char>&, const Palette&, std::vector<sf::Color>&, bool)’:
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:408:33: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   for (framePos = 256; framePos < frame.size(); framePos++) {
                        ~~~~~~~~~^~~~~~~~~~~~~~
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp: In function ‘size_t decodeRaw32(const std::vector<unsigned char>&, const Palette&, std::vector<sf::Color>&)’:
/home/u/Desktop/diablo/DGEngine/src/Cel.cpp:426:20: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  for (int i = 0; i < frame.size(); i++)
                  ~~^~~~~~~~~~~~~~
...
[ 50%] Building CXX object CMakeFiles/DGEngine.dir/src/Game/PathFinder.cpp.o
/home/u/Desktop/diablo/DGEngine/src/Game/PathFinder.cpp: In member function ‘bool MapSearchNode::IsPassableIgnoreObject()’:
/home/u/Desktop/diablo/DGEngine/src/Game/PathFinder.cpp:7:8: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (x < map->Width() && y < map->Height())
      ~~^~~~~~~~~~~~~~
/home/u/Desktop/diablo/DGEngine/src/Game/PathFinder.cpp:7:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (x < map->Width() && y < map->Height())
                          ~~^~~~~~~~~~~~~~~

The fix should be easy. Would you prefer changing the return type of frame.size(), map->Width() and map->Height() to signed integers, change the type of i, x, and y to unsigned integers, or cast the return value from frame.size(), map->Width() and map->Height() to signed integers?

ghost commented 7 years ago

These warnings are known and will be left alone for now as both those files are in for a refactor sooner or later. The level rendering code is in for a major refactor that I've been delaying for months as I need a couple of uninterrupted coding days to do which I don't have. I have been working on other features that are easier and can be done with more sporadic coding sprints.

Edit: Commit 17cbbda has the level refactor (WIP). This will make it easier to evolve players/items, etc now.

By the way, did you manage to get the pentagram defect fixed with what I suggested?

mewmew commented 7 years ago

These warnings are known and will be left alone for now as both those files are in for a refactor sooner or later. The level rendering code is in for a major refactor that I've been delaying for months as I need a couple of uninterrupted coding days to do which I don't have. I have been working on other features that are easier and can be done with more sporadic coding sprints.

Makes sense. I'll close this issue for now.

By the way, did you manage to get the pentagram defect fixed with what I suggested?

Indeed! Thanks for the help. Replied at https://github.com/dgengin/DGEngine/issues/10#issuecomment-252280129