Jump to content

  • Log In with Google      Sign In   
  • Create Account


#ActualServant of the Lord

Posted 12 November 2012 - 10:45 PM

A few stylistic notes:
  • Add whitespace to your code, it makes it more legible.
  • Don't construct a variable with null, then initialize it in the very next line. Construct and initialize it on the same line. Similarly, you should construct and initialize 'mapFile' on the same line, thusly: ifstream mapFile("map1.map"); instead of needlessly spreading it over two lines.
  • You don't need to explicitely call close() on mapFile, since it's destructor will take care of that. RAII is very important to understand.
  • I get that 'i' is usually used for iterating, but when iterating over 2D, use 'x' and 'y', not 'i' and 'j' (alternatively, 'column' and 'row' are also acceptable), Variables names should describe their purpose.
  • Be consistent with your bracketing. Your first set of for() loops are bracketed on the same line as the 'for' keyword. The second set of for() loops are bracketed at the start of the next line. Use one or the other, but be consistent. (Personally, I prefer the second method - having the brackets on a new line has several minor benefits - but it's mostly a matter of personal preference).

A few problem notes:
  • I think your second set of for() loops should have 'mapFile >> currentTIleID' before the call to 'MCSFApplySurface()', and that you should remove the 'mapFile >> currentTileID' right after currentTileID is constructed. Otherwise, you'd have (iMax * jMax) + 1 calls to mapFile >> currentTIleID instead of just (iMax * jMax).
  • On the line "MCSFApplySurface(0, 0, spriteSheet, tempSurface, ¤tTile);" you have a funky symbol in the final parameter.

Your actual issue:

But the biggest issue is... *dramatic drumroll*
You never constructed 'tempSurface', and 'tempSurface' is thus invalid. And you are drawing to an invalid surface that doesn't exist.
Worst still, 'tempSurface' is ironically the only surface you didn't initialize the NULL (and it was the only one you actually needed to).

The fact that it is all white is lucky - since you never initialized the surface to NULL, it can randomly crash your program (or worse *shudders* - it can randomly write to parts of your program's RAM in unexpected and very hard to track ways).

So, how do you actually create an empty SDL surface? I'll re-direct you to this thread, where another user was having the same problem.

The rule of thumb with pointer initialization to NULL:
  • Immediately initialize your pointers to real data whenever possible. (You *don't* need to initialize them to NULL if you can immediantly initialize them to something else instead).
  • If you need to construct your pointers but can't yet initialize them to real data, then you *must* initialize them to NULL, or face dire programming pitfalls that drive even experienced programmers crazy.

One final parting shot: I see you are using uniform initialization. Good for you! But that comes from C++11. Since you are using C++11, you actually shouldn't use NULL at all, and instead should use nullptr.

Hope that helped! If you have any questions, just ask. Posted Image

#5Servant of the Lord

Posted 12 November 2012 - 10:43 PM

A few stylistic notes:
  • Add whitespace to your code, it makes it more legible.
  • Don't construct a variable with null, then initialize it in the very next line. Construct and initialize it on the same line. Similarly, you should construct and initialize 'mapFile' on the same line, thusly: ifstream mapFile("map1.map"); instead of needlessly spreading it over two lines.
  • You don't need to explicitely call close() on mapFile, since it's destructor will take care of that. RAII is very important to understand.
  • I get that 'i' is usually used for iterating, but when iterating over 2D, use 'x' and 'y', not 'i' and 'j' (alternatively, 'column' and 'row' are also acceptable), Variables names should describe their purpose.
  • Be consistent with your bracketing. Your first set of for() loops are bracketed on the same line as the 'for' keyword. The second set of for() loops are bracketed at the start of the next line. Use one or the other, but be consistent. (Personally, I prefer the second method - having the brackets on a new line has several minor benefits - but it's mostly a matter of personal preference).

A few problem notes:
  • I think your second set of for() loops should have 'mapFile >> currentTIleID' before the call to 'MCSFApplySurface()', and that you should remove the 'mapFile >> currentTileID' right after currentTileID is constructed. Otherwise, you'd have (iMax * jMax) + 1 calls to mapFile >> currentTIleID instead of just (iMax * jMax).
  • On the line "MCSFApplySurface(0, 0, spriteSheet, tempSurface, ¤tTile);" you have a funky symbol in the final parameter.

Your actual issue:

But the biggest issue is... *dramatic drumroll*
You never constructed 'tempSurface', and 'tempSurface' is thus invalid. And you are drawing to an invalid surface that doesn't exist.
Worst still, 'tempSurface' is ironically the only surface you didn't initialize the NULL (and it was the only one you actually needed to).

The fact that it is all white is lucky - since you never initialized the surface to NULL, it can randomly crash your program (or worse *shudders* - it can randomly write to parts of your program's RAM in unexpected and very hard to track ways).

So, how do you actually create an empty SDL surface? I'll re-direct you to this thread, where another user was having the same problem.

The rule of thumb with pointer initialization to NULL:
  • Immediately initialize your pointers to real data whenever possible. (You *don't* need to initialize them to NULL if you can immediantly initialize them to something else instead).
  • If you need to construct your pointers but can't yet initialize them to real data, then you *must* initialize them to NULL, or face dire programming pitfalls that drive even experienced programmers crazy.
One final parting shot: I see you are using uniform initialization. Good for you! But that comes from C++11. Since you are using C++11, you actually shouldn't use NULL at all, and instead should use nullptr.

Hope that helped! If you have any questions, just ask. Posted Image

#4Servant of the Lord

Posted 12 November 2012 - 10:43 PM

A few stylistic notes:
  • Add whitespace to your code, it makes it more legible.
  • Don't construct a variable with null, then initialize it in the very next line. Construct and initialize it on the same line. Similarly, you should construct and initialize 'mapFile' on the same line, thusly: ifstream mapFile("map1.map"); instead of needlessly spreading it over two lines.
  • You don't need to explicitely call close() on mapFile, since it's destructor will take care of that. RAII is very important to understand.
  • I get that 'i' is usually used for iterating, but when iterating over 2D, use 'x' and 'y', not 'i' and 'j' (alternatively, 'column' and 'row' are also acceptable), Variables names should describe their purpose.
  • Be consistent with your bracketing. Your first set of for() loops are bracketed on the same line as the 'for' keyword. The second set of for() loops are bracketed at the start of the next line. Use one or the other, but be consistent. (Personally, I prefer the second method - having the brackets on a new line has several minor benefits - but it's mostly a matter of personal preference).
A few problem notes:
  • I think your second set of for() loops should have 'mapFile >> currentTIleID' before the call to 'MCSFApplySurface()', and that you should remove the 'mapFile >> currentTileID' right after currentTileID is constructed. Otherwise, you'd have (iMax * jMax) + 1 calls to mapFile >> currentTIleID instead of just (iMax * jMax).
  • On the line "MCSFApplySurface(0, 0, spriteSheet, tempSurface, ¤tTile);" you have a funky symbol in the final parameter.
Your actual issue:

But the biggest issue is... *dramatic drumroll*
You never constructed 'tempSurface', and 'tempSurface' is thus invalid. And you are drawing to an invalid surface that doesn't exist.
Worst still, 'tempSurface' is ironically the only surface you didn't initialize the NULL (and it was the only one you actually needed to).

The fact that it is all white is lucky - since you never initialized the surface to NULL, it can randomly crash your program (or worse *shudders* - it can randomly write to parts of your program's RAM in unexpected and very hard to track ways).

So, how do you actually create an empty SDL surface? I'll re-direct you to this thread, where another user was having the same problem.


The rule of thumb with pointer initialization to NULL:
  • Immediately initialize your pointers to real data whenever possible. (You *don't* need to initialize them to NULL if you can immediantly initialize them to something else instead).
  • If you need to construct your pointers but can't yet initialize them to real data, then you *must* initialize them to NULL, or face dire programming pitfalls that drive even experienced programmers crazy.
One final parting shot: I see you are using uniform initialization. Good for you! But that comes from C++11. Since you are using C++11, you actually shouldn't use NULL at all, and instead should use nullptr.

Hope that helped! If you have any questions, just ask. Posted Image

#3Servant of the Lord

Posted 12 November 2012 - 10:40 PM

A few stylistic notes:
  • Add whitespace to your code, it makes it more legible.
  • Don't construct a variable with null, then initialize it in the very next line. Construct and initialize it on the same line. Similarly, you should construct and initialize 'mapFile' on the same line, thusly: ifstream mapFile("map1.map"); instead of needlessly spreading it over two lines.
  • I get that 'i' is usually used for iterating, but when iterating over 2D, use 'x' and 'y', not 'i' and 'j' (alternatively, 'column' and 'row' are also acceptable), Variables names should describe their purpose.
  • Be consistent with your bracketing. Your first set of for() loops are bracketed on the same line as the 'for' keyword. The second set of for() loops are bracketed at the start of the next line. Use one or the other, but be consistent. (Personally, I prefer the second method - having the brackets on a new line has several minor benefits - but it's mostly a matter of personal preference).

A few problem notes:
  • I think your second set of for() loops should have 'mapFile >> currentTIleID' before the call to 'MCSFApplySurface()', and that you should remove the 'mapFile >> currentTileID' right after currentTileID is constructed. Otherwise, you'd have (iMax * jMax) + 1 calls to mapFile >> currentTIleID instead of just (iMax * jMax).
  • On the line "MCSFApplySurface(0, 0, spriteSheet, tempSurface, ¤tTile);" you have a funky symbol in the final parameter.

Your actual issue:

But the biggest issue is... *dramatic drumroll*
You never constructed 'tempSurface', and 'tempSurface' is thus invalid. And you are drawing to an invalid surface that doesn't exist.
Worst still, 'tempSurface' is ironically the only surface you didn't initialize the NULL (and it was the only one you actually needed to).

The fact that it is all white is lucky - since you never initialized the surface to NULL, it can randomly crash your program (or worse *shudders* - it can randomly write to parts of your program's RAM in unexpected and very hard to track ways).

So, how do you actually create an empty SDL surface? I'll re-direct you to this thread, where another user was having the same problem.


The rule of thumb with pointer initialization to NULL:
  • Immediately initialize your pointers to real data whenever possible. (You *don't* need to initialize them to NULL if you can immediantly initialize them to something else instead).
  • If you need to construct your pointers but can't yet initialize them to real data, then you *must* initialize them to NULL, or face dire programming pitfalls that drive even experienced programmers crazy.
One final parting shot: I see you are using uniform initialization. Good for you! But that comes from C++11. Since you are using C++11, you actually shouldn't use NULL at all, and instead should use nullptr.

Hope that helped! If you have any questions, just ask. Posted Image

#2Servant of the Lord

Posted 12 November 2012 - 10:40 PM

A few stylistic notes:
  • Add whitespace to your code, it makes it more legible.
  • Don't construct a variable with null, then initialize it in the very next line. Construct and initialize it on the same line. Similarly, you should construct and initialize 'mapFile' on the same line, thusly: ifstream mapFile("map1.map"); instead of needlessly spreading it over two lines.
  • I get that 'i' is usually used for iterating, but when iterating over 2D, use 'x' and 'y', not 'i' and 'j' (alternatively, 'column' and 'row' are also acceptable), Variables names should describe their purpose.
  • Be consistent with your bracketing. Your first set of for() loops are bracketed on the same line as the 'for' keyword. The second set of for() loops are bracketed at the start of the next line. Use one or the other, but be consistent. (Personally, I prefer the second method - having the brackets on a new line has several minor benefits - but it's mostly a matter of personal preference).
A few problem notes:
  • I think your second set of for() loops should have 'mapFile >> currentTIleID' before the call to 'MCSFApplySurface()', and that you should remove the 'mapFile >> currentTileID' right after currentTileID is constructed. Otherwise, you'd have (iMax * jMax) + 1 calls to mapFile >> currentTIleID instead of just (iMax * jMax).
  • On the line "MCSFApplySurface(0, 0, spriteSheet, tempSurface, ¤tTile);" you have a funky symbol in the final parameter.
Your actual issue:

But the biggest issue is... *dramatic drumroll*
You never constructed 'tempSurface', and 'tempSurface' is thus invalid. And you are drawing to an invalid surface that doesn't exist.
Worst still, 'tempSurface' is ironically the only surface you didn't initialize the NULL (and it was the only one you actually needed to).

The fact that it is all white is lucky - since you never initialized the surface to NULL, it can randomly crash your program (or worse *shudders* - it can randomly write to parts of your program's RAM in unexpected and very hard to track ways).

So, how do you actually create an empty SDL surface? I'll re-direct you to this thread, where another user was having the same problem.

The rule of thumb with pointer initialization to NULL:
  • Immediately initialize your pointers to real data whenever possible. (You *don't* need to initialize them to NULL if you can immediantly initialize them to something else instead).
  • If you need to construct your pointers but can't yet initialize them to real data, then you *must* initialize them to NULL, or face dire programming pitfalls that drive even experienced programmers crazy.
One final parting shot: I see you are using uniform initialization. Good for you! But that comes from C++11. Since you are using C++11, you actually shouldn't use NULL at all, and instead should use nullptr.

Hope that helped! If you have any questions, just ask. Posted Image

#1Servant of the Lord

Posted 12 November 2012 - 10:40 PM

A few stylistic notes:
  • Add whitespace to your code, it makes it more legible.
  • Don't construct a variable with null, then initialize it in the very next line. Construct and initialize it on the same line. Similarly, you should construct and initialize 'mapFile' on the same line, thusly: ifstream mapFile("map1.map"); instead of needlessly spreading it over two lines.
  • I get that 'i' is usually used for iterating, but when iterating over 2D, use 'x' and 'y', not 'i' and 'j' (alternatively, 'column' and 'row' are also acceptable), Variables names should describe their purpose.
  • Be consistent with your bracketing. Your first set of for() loops are bracketed on the same line as the 'for' keyword. The second set of for() loops are bracketed at the start of the next line. Use one or the other, but be consistent. (Personally, I prefer the second method - having the brackets on a new line has several minor benefits - but it's mostly a matter of personal preference).
A few problem notes:
  • I think your second set of for() loops should have 'mapFile >> currentTIleID' before the call to 'MCSFApplySurface()', and that you should remove the 'mapFile >> currentTileID' right after currentTileID is constructed. Otherwise, you'd have (iMax * jMax) + 1 calls to mapFile >> currentTIleID instead of just (iMax * jMax).
  • On the line "MCSFApplySurface(0, 0, spriteSheet, tempSurface, ¤tTile);" you have a funky symbol in the final parameter.

Your actual issue:

But the biggest issue is... *dramatic drumroll*
You never constructed 'tempSurface', and 'tempSurface' is thus invalid. And you are drawing to an invalid surface that doesn't exist.
Worst still, 'tempSurface' is ironically the only surface you didn't initialize the NULL (and it was the only one you actually needed to).

The fact that it is all white is lucky - since you never initialized the surface to NULL, it can randomly crash your program (or worse *shudders* - it can randomly write to parts of your program's RAM in unexpected and very hard to track ways).

So, how do you actually create an empty SDL surface? I'll re-direct you to this thread, where another user was having the same problem.

The rule of thumb with pointer initialization to NULL:
  • Immediately initialize your pointers to real data whenever possible. (You *don't* need to initialize them to NULL if you can immediantly initialize them to something else instead).
  • If you need to construct your pointers but can't yet initialize them to real data, then you *must* initialize them to NULL, or face dire programming pitfalls that drive even experienced programmers crazy.
One final parting shot: I see you are using uniform initialization. Good for you! But that comes from C++11. Since you are using C++11, you actually shouldn't use NULL at all, and instead should use nullptr.

Hope that helped! If you have any questions, just ask. Posted Image

PARTNERS