0

I want to make a simple game of 3 players, each player moves in a block depending of the random function from 1 to 6 blocks each time, when first player has been moved the second player start and then then the third player. To do that I increase the index of an array rach time a player finish its move.

My problem is that the indexer seems no to been increased, and it stacks in the player 1 even if I increase it. I have exactly the same code in C# and it works well!

Here is the code in C++.

int main ()
{
        string namesofplayers[] = {"one","two","three"};

        int movementofplayers[] = {0,0,0}; // start position of players is 
        int gamesize = 32; //32 blocks-steps of game
        int random;
        int y = 0;

        a:

        y++;
        if (y >= 3) 
        {
            y = 0;
        }
        cout << "it's" << namesofplayers[y] << "turn to play";
        int R = (rand() % 6 + 1);
        cout << "player " << namesofplayers[y] << " moves to block" << R << endl;
        movementofplayers[y] += random;
        cout << movementofplayers[y];


        if (movementofplayers[y] < gamesize)
        {
             goto a;
        }
        else 
        { 
              cout << "Player " << namesofplayers[y] << " wins the game" << endl;
        }
}
Andrea
  • 6,032
  • 2
  • 28
  • 55
Antho
  • 181
  • 11
  • Where is `random` set? – chris Dec 19 '13 at 09:54
  • 4
    You assign the random value to `R` but add the uninitialized `random` to the position. – Retired Ninja Dec 19 '13 at 09:54
  • http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – juanchopanza Dec 19 '13 at 09:54
  • 8
    Using `goto` can be okay in certain situations. Your use of it is not one of those situations. Use a loop instead. – Some programmer dude Dec 19 '13 at 09:56
  • Note that uncontrolled usage of `goto` usually results in [spaghetti code](http://en.wikipedia.org/wiki/Spaghetti_code) – amit Dec 19 '13 at 10:01
  • URRRGGGHHH C style arrays and "goto" good luck to you sir! – AngryDuck Dec 19 '13 at 10:03
  • Can't believe I finally found `goto` in an actual program and not in some example! – exAres Dec 19 '13 at 10:04
  • @AngryDuck: What's wrong with a fixed-size array? Admittedly, I'd use `std::array` or some custom fixed-sized vector-replacement but C-arrays aren't bad per se. – thokra Dec 19 '13 at 10:04
  • Dijkstra would be very unhappy now! – Eutherpy Dec 19 '13 at 10:06
  • movementofplayers[y] += R; edit – Antho Dec 19 '13 at 10:06
  • i need an example why this program in ton increasing the indexer ? i know that especially the goto is wrong technique. but same code in C# runs smoothly – Antho Dec 19 '13 at 10:08
  • Dijkstra was always unhappy. – noelicus Dec 19 '13 at 10:14
  • What do you mean by indexer. I ran your code. It seems to work. – luk32 Dec 19 '13 at 10:15
  • @thorka whats not wrong with them! its 2013 just use a vector – AngryDuck Dec 19 '13 at 10:15
  • @AngryDuck: Ehm, no, not in general. A vector allocated memory dynamically and that's a hell of a lot slower than a stack allocated, fixed-size array - even in 2013. :) – thokra Dec 19 '13 at 10:16
  • no it does not work as i expected. i just want in every round both 3 players have a turn in the game .. this is my C# code – Antho Dec 19 '13 at 10:17
  • @AngryDuck Reallocation, and overhead, that's what wrong. You think `c++` standard committee put it into `c++11` for laughs, or to please the mob? I think there was a valid reason. – luk32 Dec 19 '13 at 10:17
  • At a glance, the code does not seem to match your written description of how it behaves (or how you think it behaves). Try running it with debugger. And stop using the uninitialized `random`. – hyde Dec 19 '13 at 10:19
  • 1
    @Antho Then, put the actual and expected output in the question. It's hard to figure out what you want, when code actually runs. – luk32 Dec 19 '13 at 10:19
  • @luk32: "There is a new feature in C++, therefore I _must_ use it" Is that **really** how you live?! – Lightness Races in Orbit Dec 19 '13 at 10:25
  • `I have exactly the same code in C# and it works well!` That seems unlikely. – Lightness Races in Orbit Dec 19 '13 at 10:26
  • yeah it runs but not as i expected. i have 3 players in a string array. each player is moved depending the R, just forget random my mistake.. the second int array movementofplayers[] saves the new position of each player by just adding the previous value with the R... . my problem is that when [namesofplayers][1] finishes its turn and the movementofplayers[1]+=R ; the y is increasing.. new value [namesofplayers][2], but it does not !! this is my guestion. the programm makes all the movement just for the first player. it seems to ignore the y++ and move to the second index of the array – Antho Dec 19 '13 at 10:28
  • @Antho is this actually your code copy and pasted? - looking at the edits I'm not sure it is. – noelicus Dec 19 '13 at 10:34
  • @LightnessRacesinOrbit **What in the world made you think I think like that!?** "Arrays are bad, use vectors. c++ standard committee does not know nothing!" is that how you live? Please ... don't try read other people's brain. I said, there is probably a good reason for it to exists, since it was added **after** a container with similar functionality already existed. Who said you have to use it? Please, source, cite me. – luk32 Dec 19 '13 at 10:39
  • `"Arrays are bad, use vectors. c++ standard committee does not know nothing!" is that how you live?` Huh? I didn't say that. I advocate _using the right tool for the job_. But your previous argument hinged on asking "you think c++ standard committee put it into c++11 for laughs, or to please the mob?" which is not relevant to the feature's merits. – Lightness Races in Orbit Dec 19 '13 at 10:40
  • @LightnessRacesinOrbit I never said you did, I just applied your logic, to your words. And you went with the same "Huh?" I did. I said my words in response, to AngryDuck's comment, that "it is 2013, and use vectors" in a matter that one should use them anywhere because "What is bad in them?". I thought it's is obvious. I also said **there is a reason for it to exists**, id est there are use-cases that it's better to use it over `vector`. Not you have to use it, because it exists. Please don't overinterpret. – luk32 Dec 19 '13 at 10:48
  • also i think ther is a reason to exists goto in 2013 ...hehe – Antho Dec 19 '13 at 10:49
  • With regards to `std::vector` vs. C style arrays: C style arrays are tricky, because they don't have normal C++ value semantics. The OP is clearly just starting, so `std::vector` would seem the most appropriate solution. On the other hand, he's dimensioning the arrays according to the number of initializers, which can only be done with C style arrays, and is one of the major reasons for using them, even with C++11. (Of course, `movementofplayers` has the constraint that it must be the same size as `namesofplayers`, so should probably be defined in a way that reflects this.) – James Kanze Dec 19 '13 at 10:57
  • @Antho: The main reason is C backwards compatibility. There is simply no reason to express an algorithm with `goto` - if you find a good one, please share. It's not the only one. `this` absolutely needs not be a pointer - it could simply be a [`const`] ref, but because it was initially a pointer and a lot of code already existed when people realized that it could be a ref, so it survived for backwards compatibility. There are a lot more – thokra Dec 19 '13 at 11:21
  • 1
    @luk32: I'm not overinterpretting anything. The argument "it is 2013, and use vectors" is also void; the year is irrelevant. _Use the correct tool for the job_. If that's a vector, fine, but not because of some number on a calendar. – Lightness Races in Orbit Dec 19 '13 at 11:21
  • @LightnessRacesinOrbit: Agreed. – thokra Dec 19 '13 at 11:23
  • @LightnessRacesinOrbit We are on the very same point on this matter, sir. I just disliked your over-interpretation of my words. You did imply that I was saying or living by "There is a new feature in C++, therefore I must use it", while and I never made such point, nor said anything to support it. I made a point, that since there is such as `array`, it most probably must be a right tool for something, and using `vector` because "What's wrong with it, it's 2013." Is bad approach. Of course year does not matter, but I was not the one saying that. – luk32 Dec 19 '13 at 12:44

2 Answers2

1

On the off chance of doing your work, I took the liberty to write up an alternative implementation which fixes some of the problems your former code had and also produces more readable output. I also threw out the one-liners because they drive me crazy, but that's personal preference. Also, I tend to explicitly qualify symbols from the standard library using the appropriate scope.

  1. Get rid of goto. You can browse SO and the web for multiple reasons why not to use an explicit jump like that. Just use a loop

  2. Fix the missing initial seed for the pseudo-random number generator. If you set a varying seed, i.e. by invoking it with some variable value (e.g. time(nullptr) ), you'll always get the same succession of "random" values - with each program invocation.

  3. Fix the use of the variable random. You tried to add some garbage-initialized value random to movementofplayers[y]. Interestingly, g++-4.7 seems to ensure that the variable is set to 1 before being used in the arithmetic op. However, the correct variable you need is R.

  4. Return a well defined value from main().

I hope the code still does what you intended it to do:

#include <string>
#include <iostream>

int main ()
{
  srand(time(NULL));
  std::string namesofplayers[] = {"one","two","three"};

  int movementofplayers[] = {0,0,0}; // start position of players is
  int gamesize = 32;                 //32 blocks-steps of game
  int y = 1;

  while(movementofplayers[y] < gamesize)
  {
    if (y >= 3)
    {
      y = 0;
    }

    std::cout << "it's " << namesofplayers[y] << " turn to play" << std::endl;
    int R = (rand() % 6 + 1);
    std::cout << "player " << namesofplayers[y] << " moves to block " << R << std::endl;
    movementofplayers[y] += R;
    std::cout << "movements of player " << namesofplayers[y] <<": " << movementofplayers[y] << std::endl;

    y++;
  }

  std::cout << "Player " << namesofplayers[y] << " wins the game" << std::endl;

  return 0;
}
thokra
  • 2,874
  • 18
  • 25
  • The question is compiler-agnostic, so you could add (I think you should add :) ) a version using C++11 random libraries. – Manu343726 Dec 19 '13 at 10:35
  • i have problems with your code. it runs forever. just please take a look how pefect my C# code runs this logic https://www.dropbox.com/s/ifo2wur79o1mnsg/ConsoleApplication2.rar – Antho Dec 19 '13 at 10:41
  • @Manu343726: Well, I think the OP has different problems on their hands right now. C++11 RNGs probably aren't one of them. – thokra Dec 19 '13 at 10:41
  • @Antho: It runs fine here. No termination issues whatsoever, which is not surprising since the termination criterion is based on the number of moves the currently "selected" player has made. – thokra Dec 19 '13 at 10:43
  • @thokra. srandom and time are not defined. do you miss something in the code ? i am using codeblocks for debuging – Antho Dec 19 '13 at 10:51
  • If your compiler doesn't pull them in automatically, include `` for `srand()` and `` for `time()`. Also, you can easily look it up [here](http://en.cppreference.com/w/cpp/numeric/random/srand). – thokra Dec 19 '13 at 11:13
0

Here is how I would do it.

Added seeding the random number generator so you don't get the same game every time.

Added a constant for number of players to get rid of the magic number and also make it easier to expand the number of players if desired.

Got rid of the goto. Although it is possible to use goto in a reasonable way it is prone to accidental misuse, makes the code harder to follow, and makes people angry. :)

I tweaked the output and names a bit just to make it a little easier for me to test. In doing so I corrected an issue where it said the player moved to block R which was their roll for that turn, not their actual position in the game.

#include <string>
#include <iostream>
#include <cstdlib>
#include <ctime>

int main()
{
    std::srand(static_cast<unsigned int>(std::time(0)));

    const int gamesize = 32;
    const int num_players = 3;
    const std::string namesofplayers[num_players] = {"1", "2", "3"};
    int movementofplayers[num_players] = {0, 0, 0};

    int current_player = 0;
    for(;;) //Loop forever, the game logic will exit the loop when a winner is found
    {
        const int roll = rand() % 6 + 1;
        movementofplayers[current_player] += roll;

        std::cout << "Player " << namesofplayers[current_player] << " rolls a " << roll << " and moves to block " << movementofplayers[current_player] << std::endl;
        //Check if they won and if so, end the game
        if(movementofplayers[current_player] >= gamesize)
        {
            std::cout << "Player " << namesofplayers[current_player] << " wins the game!" << std::endl;
            break;
        }
        current_player = (current_player + 1) % num_players;
    }
    return 0;
}
Retired Ninja
  • 4,785
  • 3
  • 25
  • 35
  • It seems tha too many people are getting angry when the see the go to loop. i am using also a litle bit assembly and an assembly code is full o JMP commands. yes goto is really bad idea but i did it more for fun than for serious reasons. mainly i am a C# coder. i created a programm about this game which workos perfectly in C# code but when i decided to try it in C++ i faced too many problems – Antho Dec 20 '13 at 08:30