-2

I learned how to pass an array (of integers) in a function and also know how to do it in a constructor. But when I access the array in the object I just created, the values have been changed. I tried almost everything. I gave the new array a new name, tried using this-> to access the values and stored the array as a private element to be sure it can not be changed somehow. I think I miss something important.

I guess I am not sending the array, but only the address of the first element of the array. But still, when accessing the array at the location where I created the array, the values are correct.

It is better to show the problem with an example I have made. I am simply sending the array "keys" from object "Game" to object "Player" when initialising "Player Joyce". I store this array under the name "keyspressed". After initialising both "Game game" and "Player Joyce" I access the function "render()" continuously from a gameloop. I have printed the values of the array in the different constructors and functions.

In short, I would like that the values of "keyspressed" will always stay {1, 2, 3, 4, 5}.

I have edited my example and made a MCVE. I hope now someone can help me better:p

#include<stdio.h>
#include<vector>

class Player {
    private:

    int *keyspressed;

    public:

    Player(int keys[5]) {
        keyspressed = keys;
        printf("%i - %i - %i - %i - %i from when Player is constructed\n", keyspressed[0], keyspressed[1], keyspressed[2], keyspressed[3], keyspressed[4]);
    }
    void render() {
        printf("%i - %i - %i - %i - %i from render() in Player\n", keyspressed[0], keyspressed[1], keyspressed[2], keyspressed[3], keyspressed[4]);
    }
};
class Game {
    private:

    std::vector<Player> players;

    int keys[5] = {1, 2, 3, 4, 5};

    public:

    Game() {
        players.push_back(Player(keys));        
        printf("%i - %i - %i - %i - %i from when Game is constructed\n", keys[0], keys[1], keys[2], keys[3], keys[4]);                
    }
    void render() {
        //Render all players
        for (int j = 0; j < players.size(); j++) {
            players[0].render();
        }
        printf("%i - %i - %i - %i - %i from render() in Game\n", keys[0], keys[1], keys[2], keys[3], keys[4]);
    }   

};
class Window {
    private:

    std::vector<Game> games;

    public:

    Window() {
    }
    void loadGame() {
        games.push_back(Game());
    }
    void render() {
        //render all games
        for (int i = 0; i < games.size(); i++) {
            games[i].render();
        }
    }
};

class Program {
    private:
    std::vector<Window> windows;

    public:
    Program() {
        //This program uses 1 window
        Window window;
        windows.push_back(window);

        //load game
        windows[0].loadGame();

        run();
    }

    void run() {
        //updates the renderer
        for (int i = 0; i < windows.size(); i++) {
            windows[i].render();
        }
    }
};


int main( int argc, char* args[]) {

    Program program;

    return 0;
};

Output:

1 - 2 - 3 - 4 - 5 from when Player is constructed
1 - 2 - 3 - 4 - 5 from when Game is constructed
237368672 - 32764 - 3 - 4 - 5 from render() in Player
1 - 2 - 3 - 4 - 5 from render() in Game
F.Wessels
  • 179
  • 11
  • How do you use these classes? Can you perhaps try to create a [Minimal, Complete, and Verifiable Example](http://stackoverflow.com/help/mcve) and show us? Also, [an array of arrays is not the same as a pointer to a pointer](http://stackoverflow.com/questions/18440205/casting-void-to-2d-array-of-int-c/18440456#18440456). – Some programmer dude Jan 17 '16 at 10:51
  • keys was a typo, but that was not the problem. I guess I can make a minimal example, it will cost some time. But I just can understand how the private array "keyspressed" can ever be changed, because it is private I do not have any other methods in "Player" – F.Wessels Jan 17 '16 at 10:56
  • @F.Wessels Dangling pointers and out-of-bounds array accesses can cause all kinds of corruption. `private` only protects against deliberate modification. – molbdnilo Jan 17 '16 at 10:59
  • 1
    `ufo_main = new Texture("images/ufo/ufo.png", renderer, ufo_main_location, red, green, blue);` is suspicious, you pass local variables to `texture` constructor, and also `Player` will not behave properly on copy, you would end up with multiple players having the same ufo_main pointer – M.M Jan 17 '16 at 11:02
  • I changed my example to a MCVE. – F.Wessels Jan 17 '16 at 11:46
  • A `std::vector` copies objects when it resizes, potentially destructing old objects. Since you have objects containing pointers to data in other objects, and both are in vectors that are being resized, it is quite possible one of your pointers is pointing at data belonging to an object that no longer exists. Using such a pointer will give undefined behaviour. – Peter Jan 17 '16 at 12:05

2 Answers2

1

Probably you are ending up with dangling pointers, although I can't say for sure since you did not post a MCVE.

To avoid this sort of problem, stop using raw pointers. Instead, use containers with value semantics. You could use std::array<int, 5> keyspressed; instead, for example. You probably want to also stop using raw pointers for states and renderer too.

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
  • I changed my example to a MCVE. – F.Wessels Jan 17 '16 at 11:46
  • @F.Wessels OK. The problem is that you are copying `Game` objects around so you end up with `Player` having dangling pointers to the arrays that were in `Game` objects that have now been destroyed. To fix this, stop using raw pointers. Always use containers and/or smart pointers with ownership semantics. – M.M Jan 17 '16 at 12:59
  • When is the Game object destroyed then? When I make the Game object and put it in the vector? I really don't see it. Beside, why not using pointers, they are nice to work with if you know how to. So for example with the renderer I have 1 renderer for each window, and all subObjects get the address of the renderer. I don't understand why that is a problem. It works like a charm and I simply point to NULL when I do not need them any more. – F.Wessels Jan 17 '16 at 16:39
  • @F.Wessels `games.push_back(Game());` creates a Game, then creates a copy of that game in the vector, then destroys the original Game. This leaves the copy in the vector with dangling pointers. The reason to not use raw pointers is to avoid the risk of creating dangling pointers. Obviously your current use of pointers does not "work like a charm" since you are having these problems you posted about. – M.M Jan 18 '16 at 05:09
1

I was going to answer the question but someone beat me to it :p, however I would like to advise you to completely change how you are sending key presses and the framerates to your player class. In my first game In was doing this as well (passing them in through functions) however it started to soon get very annoying as the only way a class could access a certain variable was through arguments which meant I soon ended up having tonnes of random parameters in every function just so they could have the variables they needed.

I recommend making Game a static class and giving it a bunch of get functions to get things such as the framerate and keypresses, that way you save yourself from keeping a copy of key[5] and State inside player and removes them from your function parameters.

EDD
  • 2,070
  • 1
  • 10
  • 23
  • So what you mean is actually make one large Game object and then let each Player access needed variables from get methods in Game? Or would you consider not making a Player object at all? – F.Wessels Jan 17 '16 at 11:14
  • Yep, the way I have it set up in my android game is I have one static class called 'App' which contains a vector of pointers to 'Scene' classes (which allows different scenes - you change between them using App::gotoScene(id)). Each Scene class has a draw, update, constructor and destructor function and some onTouch, onDrag etc. functions. All theses functions get coordinated by the App class which contains the main loop and calls the functions from the current scene, this way the scenes don't have to worry about how these events are being detected - which is useful for a crossplatform app.. – EDD Jan 17 '16 at 11:53
  • Then each scene class contains the data required to run the scene so your Player class would get instantiated in for example a SceneGame class. The player then gets the frametime with App::getFt() If you want multiple players then a Player class makes sense. – EDD Jan 17 '16 at 11:56