C++

Forum

 
Nested for loop doesn't do what I want...

AmbitiousCoder (27)

Hey guys,

So it seems that when I compile my code, 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
 
#include <SDL.h>
using namespace std;

class Cube
{
public:
    Cube(int x,int y,int w,int h) { XPos = x; YPos = y; width = w; height = h; box.x = XPos; box.y = YPos; box.w = width; box.h = height;}
    SDL_Rect box;

private:
    int XPos;
    int YPos;
    int width;
    int height;
};

class Enemy
{
public:
    Enemy(int x,int y) { xPos = x; yPos = y; width = 50; height = 15; box.x = xPos; box.y = yPos; box.w = width; box.h = height;}
    SDL_Rect box;

private:
    int xPos;
    int yPos;
    int width;
    int height;
};

bool collision(SDL_Rect* rect1)
{
    if(rect1->x <= 0)
        return 1;
    if(rect1->x + rect1->w >= 640)
        return 1;
    if(rect1->y <= 0)
        return 1;
    if(rect1->y + rect1->h >= 480)
        return 1;
    return 0;
}

//bool collision(SDL_Rect* rect1, SDL_Rect* rect2)
//{
//    if(rect1->x <= rect2->x + rect2->w && rect1->x + rect1->w >= rect2->x && rect1->y <= rect2->y + rect2->h && rect1->y + rect1->h >= rect2->y)
//        return 1;
//    return 0;
//}

int main(int argc, char* argv[])
{
    SDL_Init(SDL_INIT_EVERYTHING);
    SDL_Surface *screen;
    screen = SDL_SetVideoMode(640,480,32,SDL_SWSURFACE);
    bool running = true;
    bool b[4] = {0,0,0,0};
    Uint32 start;
    int unsigned second = 1000;
    int unsigned fps = 30;
    SDL_Event event;
    Uint32 blue = SDL_MapRGB(screen->format,0,0,255);
    Uint32 black = SDL_MapRGB(screen->format,0,0,0);
    Cube * Box = new Cube(15,450,55,15);
    int unsigned row = 0;
    int unsigned enemyX = 25;
    int unsigned enemyY = 15;
    int unsigned const opponents = 10;
    Enemy * opponent[opponents];
    for( row ; row < 2; row += 1 )
    {
        for(int unsigned i = 0 ;i <= 10; i++ )
        {
           opponent[i] = new Enemy(enemyX,enemyY);
           enemyX = enemyX + 60;
        }
        enemyX = 25;
        enemyY += 15;
    }
    while(running)
    {
            start = SDL_GetTicks();
            while(SDL_PollEvent(&event))
            {
                switch(event.type)
                {
                case SDL_QUIT:
                    running = false;
                    break;
                case SDL_KEYDOWN:
                    switch(event.key.keysym.sym)
                    {
                    case SDLK_ESCAPE:
                        running = false;
                        break;
                    case SDLK_LEFT:
                        b[0] = 1;
                        break;
                    case SDLK_RIGHT:
                        b[1] = 1;
                        break;
                    case SDLK_UP:
                        b[2] = 1;
                        break;
                    case SDLK_DOWN:
                        b[3] = 1;
                        break;
                    }
                break;
                case SDL_KEYUP:
                    switch(event.key.keysym.sym)
                    {
                    case SDLK_LEFT:
                        b[0] = 0;
                        break;
                    case SDLK_RIGHT:
                        b[1] = 0;
                        break;
                    case SDLK_UP:
                        b[2] = 0;
                        break;
                    case SDLK_DOWN:
                        b[3] = 0;
                        break;
                    }
                    break;
                }
            }
            if(b[0] && collision(&Box->box) == 0){
                Box->box.x -= 5;
                if(collision(&Box->box) /*| collision(&Box->box,&Box1->box)*/)
                    Box->box.x += 5;
            }
            if(b[1] && collision(&Box->box) == 0){
                Box->box.x += 5;
                if(collision(&Box->box))
                    Box->box.x -= 5;
            }
            if(b[2] && collision(&Box->box) == 0){
                Box->box.y -= 5;
                if(collision(&Box->box))
                    Box->box.y += 5;
            }
            if(b[3] && collision(&Box->box) == 0){
                Box->box.y += 5;
                if(collision(&Box->box))
                    Box->box.y -= 5;
            }
            SDL_FillRect(screen,&screen->clip_rect,black);
            SDL_FillRect(screen,&Box->box,blue);
            for(int unsigned j = 1; j < opponents; j++)
            {
                SDL_FillRect(screen,&opponent[j]->box,blue);
            }
            SDL_Flip(screen);
            if(second/fps>(SDL_GetTicks()-start))
                SDL_Delay(second/fps-(SDL_GetTicks()-start));
    }
    SDL_QUIT;
    return 0;
}

Between lines 69-78 is a nested for loop. It is supposed to draw the objects in a row horizontally, then move down to the next row and repeat. I've changed minor things such as <= and <, i = 0, i = 1, I've mocked up a test loop in another project...

For some reason, it draws 9 objects despite me defining 10, and once it runs the inner loop, it is supposed to increase the Y coordinate by 15. What it really does, is runs the inner and outer loop, gets to the last row as defined by my outer for loop, takes the coordinate for Y that it should be at the last row, and displays the FIRST row at that Y coordinate. So despite my objects defaulting at Y coordinate 15, they start around 30 in this case... I really don't know what is going on here. Any friendly advice? Thanks..

Btw... I have enabled ALL errors. I don't get a compiler error on this one... :(
Last edited on
Disch (13742)
You are actually creating 22 objects (11 in the inner loop, and the loop is running twice in the outer loop). The first 11 get overridden by the 2nd 11 (MEMORY LEAK!)

What's more, your array is only large enough to hold 10 enemies... so the 11th that your writing to it is MEMORY CORRUPTION.

Memory leaks and memory corruption are very bad things (especially memory corruption). You never, ever ever ever want to step outside of your array boundaries.

Take another look at this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
    int unsigned const opponents = 10;
    Enemy * opponent[opponents];   // an array of 10 pointers.
       // that is, indexes 0-9 are valid.  index 10 is out of bounds.

    for( row ; row < 2; row += 1 )  // looping twice
    {
        for(int unsigned i = 0 ;i <= 10; i++ ) // <- the <= here is trouble.  If i==10, that
              // means you will go out of bounds in the 'opponent' array below.  You want
              //  <, here.  Not <=
        {
           opponent[i] = new Enemy(enemyX,enemyY);  // this creates an enemy in the 
              // first row just fine.  Then when the 2nd row does this, the first row enemy is
              //  lost because you're writing over it.  All you'll see at the end is the 2nd row
              //  of opponents
           enemyX = enemyX + 60;
        }
        enemyX = 25;
        enemyY += 15;
    }



If you have 10 enemies in 2 rows... that is 5 enemies per row... not 10. So do you want 10 enemies in a row (20 total) or do you want 5 enemies per row (10 total)?

Once you answer that question, you can fix your loop appropriately.
AmbitiousCoder (27)
That's the answer I was looking for! Two things I should mention:

Thank you so much for the clarification.

I actually did change the value to 5, before and after I posted this. For some reason, I get an error. Line 69: Statement has no effect.

It doesn't compile. :(
Disch (13742)
line 69 in your code must be different from the line 69 you pasted here.

Can you repost the line that is giving you the error (and the exact error message, in full, would also help).
AmbitiousCoder (27)
Literally only changed the value of 10 to 5 within the inner for loop:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
 
#include <SDL.h>
using namespace std;

class Cube
{
public:
    Cube(int x,int y,int w,int h) { XPos = x; YPos = y; width = w; height = h; box.x = XPos; box.y = YPos; box.w = width; box.h = height;}
    SDL_Rect box;

private:
    int XPos;
    int YPos;
    int width;
    int height;
};

class Enemy
{
public:
    Enemy(int x,int y) { xPos = x; yPos = y; width = 50; height = 15; box.x = xPos; box.y = yPos; box.w = width; box.h = height;}
    SDL_Rect box;

private:
    int xPos;
    int yPos;
    int width;
    int height;
};

bool collision(SDL_Rect* rect1)
{
    if(rect1->x <= 0)
        return 1;
    if(rect1->x + rect1->w >= 640)
        return 1;
    if(rect1->y <= 0)
        return 1;
    if(rect1->y + rect1->h >= 480)
        return 1;
    return 0;
}

//bool collision(SDL_Rect* rect1, SDL_Rect* rect2)
//{
//    if(rect1->x <= rect2->x + rect2->w && rect1->x + rect1->w >= rect2->x && rect1->y <= rect2->y + rect2->h && rect1->y + rect1->h >= rect2->y)
//        return 1;
//    return 0;
//}

int main(int argc, char* argv[])
{
    SDL_Init(SDL_INIT_EVERYTHING);
    SDL_Surface *screen;
    screen = SDL_SetVideoMode(640,480,32,SDL_SWSURFACE);
    bool running = true;
    bool b[4] = {0,0,0,0};
    Uint32 start;
    int unsigned second = 1000;
    int unsigned fps = 30;
    SDL_Event event;
    Uint32 blue = SDL_MapRGB(screen->format,0,0,255);
    Uint32 black = SDL_MapRGB(screen->format,0,0,0);
    Cube * Box = new Cube(15,450,55,15);
    int unsigned row = 0;
    int unsigned enemyX = 25;
    int unsigned enemyY = 15;
    int unsigned const opponents = 10;
    Enemy * opponent[opponents];
    for( row ; row < 2; row += 1 )
    {
        for(int unsigned i = 0 ;i <= 5; i++ )
        {
           opponent[i] = new Enemy(enemyX,enemyY);
           enemyX = enemyX + 60;
        }
        enemyX = 25;
        enemyY += 15;
    }
    while(running)
    {
            start = SDL_GetTicks();
            while(SDL_PollEvent(&event))
            {
                switch(event.type)
                {
                case SDL_QUIT:
                    running = false;
                    break;
                case SDL_KEYDOWN:
                    switch(event.key.keysym.sym)
                    {
                    case SDLK_ESCAPE:
                        running = false;
                        break;
                    case SDLK_LEFT:
                        b[0] = 1;
                        break;
                    case SDLK_RIGHT:
                        b[1] = 1;
                        break;
                    case SDLK_UP:
                        b[2] = 1;
                        break;
                    case SDLK_DOWN:
                        b[3] = 1;
                        break;
                    }
                break;
                case SDL_KEYUP:
                    switch(event.key.keysym.sym)
                    {
                    case SDLK_LEFT:
                        b[0] = 0;
                        break;
                    case SDLK_RIGHT:
                        b[1] = 0;
                        break;
                    case SDLK_UP:
                        b[2] = 0;
                        break;
                    case SDLK_DOWN:
                        b[3] = 0;
                        break;
                    }
                    break;
                }
            }
            if(b[0] && collision(&Box->box) == 0){
                Box->box.x -= 5;
                if(collision(&Box->box) /*| collision(&Box->box,&Box1->box)*/)
                    Box->box.x += 5;
            }
            if(b[1] && collision(&Box->box) == 0){
                Box->box.x += 5;
                if(collision(&Box->box))
                    Box->box.x -= 5;
            }
            if(b[2] && collision(&Box->box) == 0){
                Box->box.y -= 5;
                if(collision(&Box->box))
                    Box->box.y += 5;
            }
            if(b[3] && collision(&Box->box) == 0){
                Box->box.y += 5;
                if(collision(&Box->box))
                    Box->box.y -= 5;
            }
            SDL_FillRect(screen,&screen->clip_rect,black);
            SDL_FillRect(screen,&Box->box,blue);
            for(int unsigned j = 1; j < opponents; j++)
            {
                SDL_FillRect(screen,&opponent[j]->box,blue);
            }
            SDL_Flip(screen);
            if(second/fps>(SDL_GetTicks()-start))
                SDL_Delay(second/fps-(SDL_GetTicks()-start));
    }
    SDL_QUIT;
    return 0;
}


main.cpp|69|warning: statement has no effect [-Wunused-value]|
AmbitiousCoder (27)
Also, noting here that I literally copied my code from this exact post, changed the value, and tried to compile. It compiles, but errors out. The warning message is the one you see.
Disch (13742)
It compiles, but errors out


I'm sorry, I'm a little confused. Does it compile? Or do you get an error?

By error do you mean a runtime bug or a crash? I take "error" to mean a compiler error ... which would mean it isn't compiling. So I'm not quite sure what's really going on.



I guess the compiler might be warning you about this:

1
2
3
4
 
for(
  row ;  // <- this does nothing.  It is pointless to say "row" here.
  row < 2;
  row += 1 )


Solutions for this would be to initialize the row variable in the loop:

for( row = 0; row < 2; row += 1)

Or better yet... declare and initialize the row variable in the loop:
for( int row = 0; row < 2; row += 1)

(then get rid of the row declaration a few lines up)

That'll solve the warning, anyway. But I don't think the warning is related to your problems.



You still have the "overwrite" problem. And you are creating 6 enemies, not 5:

 
 
for(int unsigned i = 0 ;i <= 5; i++ )


This loop iterates six times. Once for each of these values: 0, 1, 2, 3, 4, 5

Again, you do not want <=, here. You want <.


You also are still overwriting the first row opponents.
opponent[i] = new Enemy(enemyX,enemyY);

For the first row... when i==0, you will be assigning opponent[0].
Then for the 2nd row, when i==0 you will be assigning opponent[0] AGAIN. This causes you to lose the first row's opponent[0], and have it replaced with the 2nd row's.


Your other problem is here:

1
2
3
4
 
            for(int unsigned j = 1; j < opponents; j++)
            {
                SDL_FillRect(screen,&opponent[j]->box,blue);
            }


You shouldn't be starting j at 1, because arrays start at 0. (this is probably why one of your items was not drawing before -- you were skipping over the first one)

You also are looping 'opponents' (10) times, but you only put 6 opponents in your 'opponent' array.

So opponent[6], opponent[7], opponent[8] and opponent[9] are all bad pointers. And attempting to read from a bad pointer's 'box' member is likely to cause your program to crash.
AmbitiousCoder (27)
Once again, Thank you for the clarification. With each of those errors happening, and assuming those are the only errors, I understand all of what you have explained. On to your question - It compiled with a runtime error. All said, Thanks!! I'll return shortly with my progress.

AmbitiousCoder (27)
Ok! Wonderful news.

I have made changes using the knowledge you have given me. Here is my final product for this issue:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
 
#include <SDL.h>
using namespace std;

class Cube
{
public:
    Cube(int x,int y,int w,int h) { XPos = x; YPos = y; width = w; height = h; box.x = XPos; box.y = YPos; box.w = width; box.h = height;}
    SDL_Rect box;

private:
    int XPos;
    int YPos;
    int width;
    int height;
};

class Enemy
{
public:
    Enemy(int x,int y) { xPos = x; yPos = y; width = 50; height = 15; box.x = xPos; box.y = yPos; box.w = width; box.h = height;}
    SDL_Rect box;

private:
    int xPos;
    int yPos;
    int width;
    int height;
};

bool collision(SDL_Rect* rect1)
{
    if(rect1->x <= 0)
        return 1;
    if(rect1->x + rect1->w >= 640)
        return 1;
    if(rect1->y <= 0)
        return 1;
    if(rect1->y + rect1->h >= 480)
        return 1;
    return 0;
}

//bool collision(SDL_Rect* rect1, SDL_Rect* rect2)
//{
//    if(rect1->x <= rect2->x + rect2->w && rect1->x + rect1->w >= rect2->x && rect1->y <= rect2->y + rect2->h && rect1->y + rect1->h >= rect2->y)
//        return 1;
//    return 0;
//}

int main(int argc, char* argv[])
{
    SDL_Init(SDL_INIT_EVERYTHING);
    SDL_Surface *screen;
    screen = SDL_SetVideoMode(640,480,32,SDL_SWSURFACE);
    bool running = true;
    bool b[4] = {0,0,0,0};
    Uint32 start;
    int unsigned second = 1000;
    int unsigned fps = 30;
    SDL_Event event;
    Uint32 blue = SDL_MapRGB(screen->format,0,0,255);
    Uint32 black = SDL_MapRGB(screen->format,0,0,0);
    Cube * Box = new Cube(15,450,55,15);
    int unsigned i = 0;
    int unsigned k = 10;
    int unsigned enemyX = 25;
    int unsigned enemyY = 15;
    int unsigned const opponents = 60;
    Enemy * opponent[opponents];
    for(int unsigned row = 0; row < (opponents / 10); row++ )
    {
        for(i;i < k; i++ )
        {
           opponent[i] = new Enemy(enemyX,enemyY);
           enemyX = enemyX + 60;
        }
        enemyX = 25;
        enemyY += 25;
        k += 10;
    }
    while(running)
    {
            start = SDL_GetTicks();
            while(SDL_PollEvent(&event))
            {
                switch(event.type)
                {
                case SDL_QUIT:
                    running = false;
                    break;
                case SDL_KEYDOWN:
                    switch(event.key.keysym.sym)
                    {
                    case SDLK_ESCAPE:
                        running = false;
                        break;
                    case SDLK_LEFT:
                        b[0] = 1;
                        break;
                    case SDLK_RIGHT:
                        b[1] = 1;
                        break;
                    }
                break;
                case SDL_KEYUP:
                    switch(event.key.keysym.sym)
                    {
                    case SDLK_LEFT:
                        b[0] = 0;
                        break;
                    case SDLK_RIGHT:
                        b[1] = 0;
                        break;
                    }
                    break;
                }
            }
            if(b[0] && collision(&Box->box) == 0){
                Box->box.x -= 10;
                if(collision(&Box->box) /*| collision(&Box->box,&Box1->box)*/)
                    Box->box.x += 10;
            }
            if(b[1] && collision(&Box->box) == 0){
                Box->box.x += 10;
                if(collision(&Box->box))
                    Box->box.x -= 10;
            }
            SDL_FillRect(screen,&screen->clip_rect,black);
            SDL_FillRect(screen,&Box->box,blue);
            for(int unsigned j = 0; j < opponents; j++)
            {
                SDL_FillRect(screen,&opponent[j]->box,blue);
            }
            SDL_Flip(screen);
            if(second/fps>(SDL_GetTicks()-start))
                SDL_Delay(second/fps-(SDL_GetTicks()-start));
    }
    SDL_QUIT;
    return 0;
}


This works exactly as intended! I plug in the number of opponents I want, and it automatically builds them in rows of 10. Here I have 60 enemies, so it has built out 6 rows of 10. Perfect!

Once again, I cannot thank you enough for this in-sightly knowledge. I overlooked and did not catch so many problems. There are probably bugs that I have not seen in my code. If you take a look through it and find them, I would appreciate the heads-up. :)

Have a good day!
Disch (13742)
I'm glad to help.

The only other thing I notice is that you are leaking memory. Remember that every new must have matching delete. So those 60 opponents you are creating in that loop need to be deleted when you're done with them.
Topic archived. No new replies allowed.