Write access violation thrown when trying to call an array-modifying function twice EDIT: Now a read access violation.

[Endersmens]

I have a function that modifies the contents of an array. the function works just fine when I only call it once. If I call it twice, it throws a write access violation exception. I have no idea why this is happening or how to fix it. I tried passing the array by reference but it didn't change the end result.

Yes, this is for a program for a class. However, this kind of error was never covered, and I'm doing everything the way I was taught in the class and it isn't working. Google didn't help me at all, it only talked about things we haven't gone over yet so were useless to me. We haven't gotten to classes or objects yet, the closest we got were structs. Basically we've covered functions, structs, arrays, and loops. (and the basics like variables ect)

Here is my current function. The exception is always thrown at the line with the exclamation points.

void reroll(die (dice)[array_size], bool (rerollarray)[array_size])
{
	cout << "Enter the numbers of the dice you wish to reroll, separated by a space, then enter \"roll\" ";

	intReroll(rerollarray);
	int rerolltemp;
	int len = 0;
	string test;

	for (int i = 0; i < 11; i++) //takes the input stream and processes it
	{
		if (cin.peek() == 'r')
		{
			break;
		}
		else
		{
			cin >> rerolltemp;
!!!			rerollarray[rerolltemp - 1] = 1;        !!!
		}
	}
	for (int i = 0; i < array_size; i++)		//prints the selected dice
	{
		if (rerollarray[i] == true)
		{
			cout << i + 1 << " ";
		}
	}

	for (int i = 0; i < 4; i++)		//rerolls the low dice
	{
		if (rerollarray[i] == true)
			dice[i] = rollDie(1, 6);
	}

	for (int i = 4; i < 10; i++)		//rerolls the high dice
	{
		if (rerollarray[i] == true)
			dice[i] = rollDie(1, 6);
	}

	for (int i = 0; i < 4; i++)		//converts sides 5 and 6 on low dice to a wild face
	{
		if (dice[i].faceValue == 5 || dice[i].faceValue == 6)
		{
			dice[i].wild = true;
		}

	}
	cout << "The dice after the reroll: " << endl;
	displayAll(dice);
}

EDIT: Solved.

[tpb]

What does intReroll() look like? What about rollDie()? What is "die"? What does the input look like?

You should be setting bools to true and false; not 1 and 0 (as on the line marked with bangs).

And why are your parameter names in parentheses? That's just weird. ;-)

[Endersmens]

struct die	//struct for a die
{
	int faceValue;
	COLORS color;
	bool wild;
};

void intReroll(bool (rerollarray)[array_size])
{
	for (int i = 0; i < array_size; i++)
		rerollarray[i] = 0;
}

die rollDie(int min, int max)		//takes the min and max number of the dice, and rolls a random int in that range, also rolls for color, and outputs the rolls as a struct
{

	int rolledFaceValue;
	COLORS color;
	rolledFaceValue = rand() % (max - min + 1) + min;
	color = static_cast<COLORS>(rand() % (3 - 0 + 1) + 0);
	die rolledDice;
	rolledDice.faceValue = rolledFaceValue;
	rolledDice.color = color;
	rolledDice.wild = false;
	return rolledDice;

}

The input for reroll is a set of numbers followed by "roll."

The parameter names in parentheses was a leftover from when I tried passing the arrays by reference, by doing (&array_name)[array_size]

[Endersmens]

https://bpaste.net/show/0bb3f56b8ff6

That's a bpaste of my whole program so far. A lot of what is in main right now is going to be moved into a "turn" function.

The reason I set the bools to 0 and 1 was to do with the error code I was receiving. I was just trying different options. Originally they were being set to true and false, and i still got the exception.

[tpb]

Thanks for posting that. Those are some wacky dice!

You are asking the user to enter some integers followed by the word "roll". You stop reading integers when you detect the 'r', but you leave "roll" in the input buffer. The next time around, the "roll" is the first thing read. You need a cin.ignore(999, '\n') or something like that to clean up the input buffer.

Some little points:
It should be <ctime> instead of <time.h> (basically <xxx.h> for standard C headers gets converted to <cxxx> in C++).

In C++ we don't use NULL. Instead we use 0 or, in modern code (C++11 or better), the shiny new keyword nullptr.

You should use your enumerator identifiers in the switch in displayAll().

"intReroll" should probably be called "initReroll" or "clearReroll".

Remember to assign true and false to bools. And we usually don't say if (whatever == true) but just if (whatever).

[Endersmens]

Wacky indeed.

Clearing the input buffer didn't change anything, still getting a write access violation.

Most of the little points you made are things that I directly copied from my book or my professor, ect.

[tpb]

I gave the above advice just looking through your code. But running it, it still crashes.
You need to change the first loop in reroll to something like:

	for (int i = 0; i < 11; i++)
	{
		cin >> rerolltemp;
		rerollarray[rerolltemp - 1] = true;
	}
	cin.clear();           // reset the stream state
	cin.ignore(999, '\n'); // ignore the rest of the line 

[Endersmens]

Wow, that actually worked. We did learn about cin.clear() and ignore but i never thought to use it here. I did know that my peek strategy wasn't working to the extent I wanted it to, since it was always peeking the space between the last number and the r in roll. But I didn't think that would work. I guess the cin >> rerolltemp ignores the roll since it doesn't match the variable type? and it just loops through repeating the last number until i = 11 correct?

[tpb]

Yeah, that's about it. As soon as it fails to read an integer, every cin>>whatever is basically a no-op until you clear the stream. It can be a tricky one to find. If you suspect a stream is going bad you could put a debugging line in an appropriate place: if (!stream) {cerr << "bad stream!!!\n"; exit(1);}

[Endersmens]

Alright well thank you very much! I was stuck on that for 6 solid hours.... feel pretty silly now. :p

[tpb]

Don't feel silly. It's a tricky one. Just remember it for future reference. :-)

[icy1]

Fun little program you have there. I believe most of the troubles are coming from the previous cin request, namely the one asking for a single-character answer on whether user would like to reroll. The newline after the single character is still in play and causes problems for the next cin (inside reroll()).

Added some other tweaks to help harden things and make it easier to maintain:
- calling the rollAll() from reroll() instead of repeating that logic
- changed rollDie() to roll a die passed in by reference instead of making extra copies and assignments
- was necessary to add another boolean var to each die, "needs_roll", to see if a die needed to be rolled. When a die is first created, I set this var to true.
- no longer need a new array to keep track of reroll indices, since each die now stores this information
- a dynamic vector added just to just push_back and then show the user-input numbers that were parsed successfully
- since you're asking for an unknown number of integers to mark the rerolls, I think it makes sense to use getline() to grab the line, then throw it into an istringstream, the parse on that until it cant find anymore integers
- reduced repetition in main() by running the reroll question logic 2 times under a loop
- might've converted a bunch of tabs to 4 spaces ;D

Can test at https://repl.it/repls/ImpossibleExemplaryResources

#include <iostream> //used for input and output
#include <cstdlib>  //used for random functions
#include <ctime>    //used for setting srand seed based on time
#include <string>   //used for strings
#include <sstream>
#include <limits>
#include <vector>

using namespace std;

enum COLORS { red, blue, green, orange };   //enum type for face color

struct die  //struct for a die
{
    die() : needs_roll(true)
    {
    }
    
    int faceValue;
    COLORS color;
    bool wild;
    bool needs_roll;
};
struct player   //struct for a player
{
    string name;
    int score;
};

const int ARRAY_SIZE = 10;  //size of dice arrays

// Rolls the die d
void rollDie(die& d, int min, int max)       
{
    d.faceValue = rand() % (max - min + 1) + min;
    d.color = static_cast<COLORS>(rand() % 4);
    d.wild = false;
    d.needs_roll = false;
}

// Roll all dice that have "needs_roll" set to true
void rollAll(die* dice)
{
    int LowDiceMin = 1;
    int LowDiceMax = 6;
    int HighDiceMin = 5;
    int HighDiceMax = 10;

    for (int i = 0; i < 4; i++)     //rolls low dice
    {
        if (dice[i].needs_roll)
        {
            rollDie(dice[i], LowDiceMin, LowDiceMax);
            dice[i].wild = dice[i].faceValue == 5 || dice[i].faceValue == 6;
        }
    }
    for (int i = 4; i < 10; i++)        //rolls high dice
    {
        if (dice[i].needs_roll)
            rollDie(dice[i], HighDiceMin, HighDiceMax);
    }
}

void displayAll(die* dice)
{
    char abrcolor;
    for (int i = 0; i < ARRAY_SIZE; i++)
    {
        cout << " ____  ";          //formats the top row of the dice display
    }
    cout << endl;

    for (int i = 0; i < ARRAY_SIZE; i++)
    {
        switch (dice[i].color)          //converts the COLORS enum results to a character variable for output
        {
        case 0:
            abrcolor = 'R';
            break;
        case 1:
            abrcolor = 'G';
            break;
        case 2:
            abrcolor = 'B';
            break;
        case 3:
            abrcolor = 'O';
            break;
        }

        if (i < 4)          //checks for wilds, then outputs low dice
        {
            if (dice[i].wild == true)
                cout << "|" << abrcolor << "  W| ";
            else
                cout << "|" << abrcolor << "  " << dice[i].faceValue << "| ";

        }
        else
            if (dice[i].faceValue == 10)        //checks for a value of ten for spacing, then outputs high dice
                cout << "|" << abrcolor << " " << dice[i].faceValue << "| ";
            else
                cout << "|" << abrcolor << "  " << dice[i].faceValue << "| ";

    }

    cout << endl;

    for (int i = 0; i < ARRAY_SIZE; i++)        //formats the bottom of the dice
    {
        cout << "|____| ";
    }

    cout << endl;

    for (int i = 0; i < ARRAY_SIZE; i++)        //outputs the sequential number of each dice
    {
        cout << "   " << i + 1 << "   ";
    }
    cout << endl;
}

void reroll(die* dice)
{
    cout << "Input space-separated numbers of the dice you wish to reroll and then press <Enter>\n";
    cout << "? ";
    string line;
    getline(cin, line);
    istringstream iss(line);
    
    vector<int> redos;
    int die_num = -1;
    while (iss >> die_num)
    {
        if (1<=die_num && die_num<=ARRAY_SIZE)
        {
            redos.push_back(die_num);
            dice[die_num - 1].needs_roll = true;
        }
        else
        {
            cout << "Skipping input '"<<die_num<<"' -- should be between 1 and "<<ARRAY_SIZE<<endl;
        }
    }
    
    cout << "Parsed rerolls: ";
    for (auto& n : redos)
        cout << n << ' ';
    cout << endl;

    cout << "The dice after the reroll: " << endl;
    rollAll(dice);
    displayAll(dice);
}

int main()
{
    srand(time(NULL));  //sets srand seed

    die dice[ARRAY_SIZE];
    cout << "Press <Enter> to roll the dice.";
    cin.ignore(numeric_limits<int>::max(), '\n');
    rollAll(dice);
    displayAll(dice);
    
    char answer;
    for (int times=1; times<=2; ++times)
    {
        cout << "Would you like to reroll any dice? y/n: ";
        cin >> answer;
        cin.ignore(numeric_limits<int>::max(), '\n');
        if (answer == 'y')
            reroll(dice);
        else
            break;
    }

    cout << endl << endl;
    cout << "Thanks for playing!\n";
    
    return 0;
}

Example output:

Press <Enter> to roll the dice. 
 ____   ____   ____   ____   ____   ____   ____   ____   ____   ____  
|G  1| |O  2| |O  2| |O  1| |B  9| |O 10| |G 10| |R 10| |G  5| |O 10| 
|____| |____| |____| |____| |____| |____| |____| |____| |____| |____| 
   1      2      3      4      5      6      7      8      9      10   
Would you like to reroll any dice? y/n:  y
Input space-separated numbers of the dice you wish to reroll and then press <Enter>
?  1 2 -7 4 5 15 6 7
Skipping input '-7' -- should be between 1 and 10
Skipping input '15' -- should be between 1 and 10
Parsed rerolls: 1 2 4 5 6 7 
The dice after the reroll: 
 ____   ____   ____   ____   ____   ____   ____   ____   ____   ____  
|B  W| |B  4| |O  2| |O  3| |O 10| |B  6| |R  5| |R 10| |G  5| |O 10| 
|____| |____| |____| |____| |____| |____| |____| |____| |____| |____| 
   1      2      3      4      5      6      7      8      9      10   
Would you like to reroll any dice? y/n:  y
Input space-separated numbers of the dice you wish to reroll and then press <Enter>
?  1 2 3 4 5       7 8 9
Parsed rerolls: 1 2 3 4 5 7 8 9 
The dice after the reroll: 
 ____   ____   ____   ____   ____   ____   ____   ____   ____   ____  
|B  W| |G  2| |O  W| |B  1| |O  6| |B  6| |B  6| |B  9| |O  9| |O 10| 
|____| |____| |____| |____| |____| |____| |____| |____| |____| |____| 
   1      2      3      4      5      6      7      8      9      10   


Thanks for playing!

[tpb]

I believe most of the troubles are coming from the previous cin request, namely the one asking for a single-character answer on whether user would like to reroll

That's not right. The problem, as I said above, is that the stream was going bad while reading ints and encoutering a non-int.

[Endersmens]

Now I'm getting a access violation reading <memory location>

The program breaks at like 284, which reads
set[dice[i].faceValue]++;

Everything was working fine until now. I know this code isn't optimized, i just need it working. I can't track down what's causing the read error. Any ideas?

My program in a bpaste: https://bpaste.net/show/0450731d6377

[dhayden]

When I run it, dice[i].faceValue is -2145400288. So it's accessing an array WAY out of bounds.

The problem is that your dice[] array in main() is uninitialized.

[dhayden]

Another comment:
In reroll:

        cin >> rerolltemp;
        if (rerollarray > 0)   // should that be rerolltemp?
            rerollarray[rerolltemp - 1] = true;

[Endersmens]

Thank you, the issue was resolved. Also nice catch in reroll, you are indeed correct. That statement was put there to prevent a 0 from turning into -1 and causing index issues.