Simulation for rolling dice in X-Wing

Question

I've recently started playing the X-Wing Miniature game, and there is quite some math behind some of the dice rolls, as they are affected by many abilities and so on. Manually calculating these is quite burdensome, so I decided to write a simple C++ program to approximate the expected damage by running many simulations.

#include "targetver.h"
#include <stdio.h>
#include <tchar.h>
#include <iostream>
#include <random>
#include <string>
#include <ctime>
#include <algorithm>
int main()
{
std::random_device rd;
std::mt19937 mt(rd());
std::uniform_int_distribution<int> dist(1, 8);
const int nSim = 100000;
const int NAttackDice = 2;
const bool AttackFocus = true;
int AttackDiceResult [NAttackDice];
int HowlRunnerResult;
const bool Howlrunner = true;
const bool CrackShot = true;
bool CrackShotUsed = false;
int AttackHitCounter = 0;
int AttackFocusCounter = 0;
int CritCounter = 0;
int HowlRunnerHitCheck;
int HowlHitCounter = 0;
int HowlFocusCounter = 0;
int HowlCritCounter = 0;
const int NEvadeDice = 3;
const bool EvadeFocus = true;
int EvadeDiceResult [NEvadeDice];
int EvadeCounter = 0;
int DefenseFocusCounter = 0;
double Damage[nSim];
double DamageSum = 0;
std::cout << "Rolling " << NAttackDice << " attack dice and " << EvadeDice << " evade dice" << std::endl;
std::cout << "Attack Focus: " << AttackFocus << " and Evade Focus: " << EvadeFocus << " Howlrunner: " << Howlrunner << " Crack Shot: " << CrackShot << std::endl;
for (int n = 0; n < nSim; ++n)
{
 CrackShotUsed = false;
 for (int i = 0; i < NAttackDice; i++)
 {
 AttackDiceResult[i] = dist(rd);
 if (1 <= AttackDiceResult[i] && AttackDiceResult[i] <= 3)
 {
 AttackHitCounter = AttackHitCounter + 1;
 }
 else if (4 <= AttackDiceResult[i] && AttackDiceResult[i] <= 5 && AttackFocus == true)
 {
 AttackFocusCounter = AttackFocusCounter + 1;
 }
 else if (6 == AttackDiceResult[i])
 {
 CritCounter = CritCounter + 1;
 }
 }
 HowlRunnerHitCheck = AttackHitCounter + AttackFocusCounter + CritCounter;
 if (Howlrunner == true && HowlRunnerHitCheck < NAttackDice)
 {
 HowlRunnerResult = dist(rd);
 if (1 <= HowlRunnerResult && HowlRunnerResult <= 3)
 {
 HowlHitCounter = HowlHitCounter + 1;
 }
 else if (4 <= HowlRunnerResult && HowlRunnerResult <= 5 && AttackFocus == true)
 {
 HowlFocusCounter = HowlFocusCounter + 1;
 }
 else if (6 == HowlRunnerResult)
 {
 HowlCritCounter = HowlCritCounter + 1;
 }
 }
 for (int i = 0; i < NEvadeDice; i++)
 {
 EvadeDiceResult[i] = dist(rd);
 if (1 <= EvadeDiceResult[i] && EvadeDiceResult[i] <= 3)
 {
 EvadeCounter = EvadeCounter + 1;
 }
 else if (4 <= EvadeDiceResult[i] && EvadeDiceResult[i] <= 5 && EvadeFocus == true)
 {
 DefenseFocusCounter = DefenseFocusCounter + 1;
 }
 }
 int CrackShotHitCheck = AttackHitCounter + AttackFocusCounter + CritCounter + HowlHitCounter + HowlFocusCounter + HowlCritCounter;
 if (CrackShot == true && CrackShotUsed == false && EvadeCounter >= 1 && CrackShotHitCheck > 0)
 {
 EvadeCounter = EvadeCounter - 1;
 CrackShotUsed = true;
 }
 if (CrackShot == true && CrackShotUsed == false && DefenseFocusCounter >= 1 && CrackShotHitCheck > 0)
 {
 DefenseFocusCounter = DefenseFocusCounter - 1;
 CrackShotUsed = true;
 }
 Damage[n] = std::max(0, (AttackHitCounter + CritCounter + AttackFocusCounter + HowlHitCounter + HowlFocusCounter + HowlCritCounter) - EvadeCounter - DefenseFocusCounter);
 DamageSum = DamageSum + Damage[n];
 AttackHitCounter = 0;
 AttackFocusCounter = 0;
 CritCounter = 0;
 HowlHitCounter = 0;
 HowlFocusCounter = 0;
 HowlCritCounter = 0;
 EvadeCounter = 0;
 DefenseFocusCounter = 0;
}
std::cout << "Number of simulations was " << nSim << std::endl;;
std::cout << "Average damage was " << DamageSum / nSim << std::endl;
return 0;
}

The code is a little messy, but the basic idea is pretty simple: Run n simulations, in each simulation roll a specific number of attack dice, a specific number of defense dice, and then add the results. I've already added several special actions from the game (Focus for the attacker/defender, Howlrunner and CrackShot) since these are the ones my squadron will be using.

Since I have very limited experience with programming, I'm not sure how to improve the procedure, but in my mind, there are several issues with my code:

1. The Damage array has size nSim, so if I increase the number of simulations, the program crashes due to overflow problems.
2. I'm pretty sure that a for-loop is not the most efficient way here.
3. The way the dice results are counted seems inefficient to me as well (using the different if conditions)

Edit: The code can also be found on Github, if you prefer to read the code over there.

Edit 2: I have now implemented some of the changes in the answers - they can be found in the github link, as I think posting the new code here a seems a little redundant to me. However, the first problem still persists, i.e. the program crashes if the number if simulations is too high (because the array is too large?).

• Яois: It should be NEvadeDice in the first cout. The "N" is missing.
• Яois: don't use C-style arrays like in double Damage[nSim]. Use the list or vector templates in the C++ standard library and allocate memory dynamically. This should solve #1
• Olorun: @KeillЯandor I've changed my code to use an array as some of the below answers suggested (since I wasn't sure how to use list or vector) and the problem still persists.

Answer 1

This code would greatly benefit from being split into functions, the most important one for your logic being a function to run a single instance of the simulation.

// Runs one simulation of an X-Wing attack and returns the actual damage result.
int SimulateDamage(int attackDice, bool attackFocus,
 int evadeDice, bool evadeFocus,
 bool howlRunner, bool crackShot)
{
 ... // calculations
 int totalDamage = AttackHitCounter + CritCounter + AttackFocusCounter 
 + HowlHitCounter + HowlFocusCounter + HowlCritCounter
 - EvadeCounter - DefenseFocusCounter;
 return std::max(0, totalDamage);
}

This clearly defines what values and options can affect the result, and you can keep all the variables and calculations needed for doing one simulation local to this function. I would consider breaking this function down further, but just pulling out this one function will make things a lot neater and clearer.

Other things in the code that aren't so serious and reflect my personal style preferences:

• Don't use C-style arrays like int AttackDiceResult[NAttackDice] - use a std::vector (unknown size) or std::array (fixed size) instead.
• Don't use abbreviations in names unless they're really common; NAttackDice could rather be numAttackDice or just attackDice; nSim could be numSimulations or just simulations; dist could be die or d8.
• Use '\n' rather than std::endl unless you specifically need the flush from the latter (which is almost never).
• AttackHitCounter = AttackHitCounter + 1; should be just ++AttackHitCounter; (or AttackHitCounter += N; for values greater than 1). This is idiomatic C++ and makes it obvious you're actually modifying a variable.
• if (CrackShot == true) should be just if (Crackshot) and if (CrackShotUsed == false) should be just if (!CrackShotUsed) - again idiomatic C++, and well-chosen variable names will actually make these read more like natural language sentences and so easier for a reader to reason about (reading ! as "not").
• No need for a return 0; - main will automatically return a 0 result on reaching the end.

One final thing: the github version has replaced all the includes with a #include "stdafx.h" - that's a bad use of Visual C++ pre-compiled headers. Since pre-compiled headers are non-standard, I'd recommend just not using them, but if you have to, the pre-compiled header should be duplicating your (common) includes, not replacing them.

• Olorun: I've implemented all of your suggestions, and even split up the damage simulation into two more functions (attack & defense simulation). However, my first problem still persists: If I increase the number of simulations, my compiled file still crashes and the debugging shows a overflow error.
• theosza: The reason for the crash is that your program is trying to use too much stack memory. Visual C++ by default allows a 1 MB stack. Your damage array uses 100,000 * 8 bytes = approx 0.8 MB. Increasing the number of simulations by more than 25% will run you out of stack memory - a stack overflow. To get around this you could use a std::vector which is allocated on the heap, or if you don't need all the values in the array, just get rid of it entirely - your calculations only needs to keep track of the sum of damage.
• Olorun: I've heard somewhere that preallocating an array is saving time in the calculation. If I were to use a vector which is allocated on the heap, the program would run slower? I will probably just remove the array as you mentioned, since I just need the sum.
• theosza: The problem with a vector is that whenever it needs to grow, it has to reallocate the memory and copy the whole contents of the vector. You can avoid that by specifying the size in the constructor or by using std::vector::reserve.

Answer 2

To add to @theosza's excellent list (also in a pull request https://github.com/Olorun/x-wing-simulation/pull/1)

• @theosza's recommendation of std::array is a C++ Core Guideline ES.27

• Prefer algorithms C++ Core Guideline T2

• anonymous enum can make nice names for constants enum { NAttackDice = 2 };
• DamageSum isn't used until the end of the function so don't declare it until it is needed C++ Core Guideline ES.5
• Rewriting the simulation for loop as a generate() statement lets you move the sum to an algorithm too const auto DamageSum = std::accumulate(std::begin(Damage), std::end(Damage), 0.0);
• you are using the random distribution incorrectly. You construct a Mersenne Twister using the random_device as a seed, this is correct. But later you use the random_device to provide entropy for the uniform distribution. Instead use the Mersenne Twister as your entropy source. The random_device could be very very slow.

Change: HowlRunnerResult = dist(rd); To: HowlRunnerResult = dist(mt);

Prefer algorithms since it clarifies your intent. Once things are moved into algorithms and functions it becomes very clear how things are interacting. It also appears that a visitor pattern would be helpful for you to apply each of the hits and damages Demo Example, Code Example.

• Olorun: Thanks, I've implemented all of your suggestions - especially the last part of using the Mersenne Twister as my entropy source. The visitor pattern looks interesting, maybe I'll implement it later when I understand it better.

Answer 3

This is a late answer which already applies some of the previous answers suggestions. However, it will concentrate more on architectural aspects, rather than technical ones (there is a lot of dust on my C++ skills).

1) use classes - since you are working in C++, it is better to encapsulate anything in a class, that does a specific thing (Separation of Concerns). So, your main function should be very slim and delegate the actual work to an object:

int main()
{
 const int nSim = 10;
 Simulation sim(nSim);
 sim.PrintSimulationStartData();
 sim.Simulate();
 sim.PrintSimulationEndData();
 return 0;
}

2) avoiding repetation - see DRY principle - as you have already notices, some things are repeating. One way to model your characters actions is to use abilities. E.g. normal attack and howl attack are using the same logic, so they can be unified:

class Ability
{
 private:
 int HitCounter = 0;
 int FocusCounter = 0;
 int CritCounter = 0;
 public:
 static const int HitMin = 1;
 static const int FocusMin = 4;
 static const int CritMin = 6;
 int GetHitCounter() { return HitCounter; }
 int GetFocusCounter() { return FocusCounter; }
 int GetCritCounter() { return CritCounter; }
 void ResetCounters()
 {
 HitCounter = 0;
 FocusCounter = 0;
 CritCounter = 0;
 }
 void ApplyAbility(int value, bool focus)
 {
 // std::cout << "Ability value" << value << std::endl;
 // use constants
 if (HitMin <= value && value < FocusMin)
 HitCounter++;
 else if (FocusMin <= value && value < CritMin && focus)
 FocusCounter++;
 else if (value >= CritMin)
 CritCounter++;
 }
 int GetCounterSum()
 {
 return HitCounter + FocusCounter + CritCounter;
 }
};

This allows for future types of attacks. E.g. magic attacks

Here, the are already some refactoring done, as explained in the steps:

3) Avoid hardcoding values, especially when they meaning is hard to guess. So your comparisons should be made against meaningful constants (I tried to guess their meaning):

static const int HitMin = 1;
static const int FocusMin = 4;
static const int CritMin = 6;

4) Defensive programming - means to compensate for some possible future changes.

else if (6 == AttackDiceResult[i])
{
 CritCounter = CritCounter + 1;
}

What happens if your dice changes into a larger polyhedral one? Your ifs statements will not handle the value at all. So, a reasonable version is to use something like AttackDiceResult[i] >= 6

5) Encapsulate your damage simulation in a class

Since main is much slimmer now, we expect that everything is handled by a special designed class:

// created new class to separate things

class Simulation
{
 private:
 // all context data are declared as members
 const int MinDistributionValue = 1;
 const int MaxDistributionValue = 8;
 Ability AttackAbility;
 Ability HowlAbility;
 const int NAttackDice = 2;
 const bool AttackFocus = true;
 const bool Howlrunner = true;
 const bool CrackShot = true;
 int* AttackDiceResult;
 int HowlRunnerResult;
 bool CrackShotUsed = false;
 const int NEvadeDice = 3;
 const bool EvadeFocus = true;
 // allocated everything on stack -> check stack limit in C++
 int* EvadeDiceResult;
 int EvadeCounter = 0;
 int DefenseFocusCounter = 0;
 double* Damage;
 double DamageSum = 0;
 int nSim = 0;
 // std::random_device rd;
 std::mt19937 mt;
 std::uniform_int_distribution<int> dist;
 // other code comes here
}

6) Clear separation of initialization and destruction activities

 // fixed custom initialization 
 void init()
 {
 AttackDiceResult = new int[NAttackDice];
 EvadeDiceResult = new int[NEvadeDice];
 Damage = new double[nSim];
 }
 // generates next 
 int spin() 
 { 
 int ret = dist(mt);
 // std::cout << "Random = " << ret << std::endl;;
 return ret; 
 }
public:
 // constructor
 Simulation(int simCount) :
 mt(std::random_device()()),
 dist(MinDistributionValue, MaxDistributionValue)
 {
 nSim = simCount;
 init();
 }
 ~Simulation()
 {
 free(AttackDiceResult);
 free(EvadeDiceResult);
 free(Damage);
 }

7) Merging counter reset in one place. Also, all the printing will be clearly separated (maybe you want to write the data to a file, database etc., so its easy to see where to change in the future).

 // this is done after each step, so it makes sense to have one function
 void ResetCounters()
 {
 AttackAbility.ResetCounters();
 HowlAbility.ResetCounters();
 }
 void PrintSimulationStartData()
 {
 std::cout << "Rolling " << NAttackDice << " attack dice and " << NEvadeDice << " evade dice" << std::endl;
 std::cout << "Attack Focus: " << AttackFocus << " and Evade Focus: " << EvadeFocus << " Howlrunner: " << Howlrunner << " Crack Shot: " << CrackShot << std::endl;
 }
 void PrintSimulationEndData()
 {
 std::cout << "Number of simulations was " << nSim << std::endl;;
 std::cout << "Average damage was " << DamageSum / nSim << std::endl;
 }

8) Avoid doing lots of stuff in a single loop:

 void Simulate()
 {
 for (int n = 0; n < nSim; ++n)
 {
 SimulateStep(n);
 ResetCounters();
 }
 }

Generally, it is easier to think your program logic top-down than bottom up (doing little things and merging usually requires more skill):

• your main function creates the Simulation, prints start data, simulate and prints the results (nothing is created yet)
• you define variables and constants for Simulation, create printing functions (they are the smallest), simulate does nothing for now.
• simulation is done in several steps. Just define it as above etc.

9) Step functionality is now simplified:

 void SimulateStep(int n)
 {
 CrackShotUsed = false;
 for (int i = 0; i < NAttackDice; i++)
 {
 AttackDiceResult[i] = spin();
 AttackAbility.ApplyAbility(AttackDiceResult[i], AttackFocus);
 }
 // not needed outside
 int HowlRunnerHitCheck = AttackAbility.GetCounterSum();
 if (Howlrunner && HowlRunnerHitCheck < NAttackDice)
 {
 HowlRunnerResult = spin();
 HowlAbility.ApplyAbility(HowlRunnerResult, AttackFocus);
 }
 for (int i = 0; i < NEvadeDice; i++)
 {
 EvadeDiceResult[i] = spin();
 if (Ability::HitMin <= EvadeDiceResult[i] && EvadeDiceResult[i] < Ability::FocusMin)
 EvadeCounter++;
 else if (EvadeDiceResult[i] >= Ability::FocusMin && EvadeDiceResult[i] <= Ability::CritMin && EvadeFocus)
 DefenseFocusCounter++;
 // else?
 }
 int CrackShotHitCheck = AttackAbility.GetCounterSum() + HowlAbility.GetCounterSum();
 if (CrackShot && !CrackShotUsed && EvadeCounter >= 1 && CrackShotHitCheck > 0)
 {
 EvadeCounter--;
 CrackShotUsed = true;
 }
 if (CrackShot && !CrackShotUsed && DefenseFocusCounter >= 1 && CrackShotHitCheck > 0)
 {
 DefenseFocusCounter--;
 CrackShotUsed = true;
 }
 Damage[n] = std::max(0, CrackShotHitCheck - EvadeCounter - DefenseFocusCounter);
 // std::cout << "Damage " << Damage[n];
 // contracted
 DamageSum += Damage[n];
 }

Defense and CrackShot logic can be separated, to have separate Defense concern from attack concern.

Also, all assignments have been contracted (++, --, +=) and reduced number of accolades. It is a matter of taste, but I think that accolades can be missed when if statement is very short and of course, the logic is a single instruction.

All the code (not really tested):

#include <stdio.h>
#include <iostream>
#include <random>
#include <string>
#include <ctime>
#include <algorithm>
// separated ability (improper name) properties and actions
class Ability
{
 private:
 int HitCounter = 0;
 int FocusCounter = 0;
 int CritCounter = 0;
 public:
 static const int HitMin = 1;
 static const int FocusMin = 4;
 static const int CritMin = 6;
 int GetHitCounter() { return HitCounter; }
 int GetFocusCounter() { return FocusCounter; }
 int GetCritCounter() { return CritCounter; }
 void ResetCounters()
 {
 HitCounter = 0;
 FocusCounter = 0;
 CritCounter = 0;
 }
 void ApplyAbility(int value, bool focus)
 {
 // std::cout << "Ability value" << value << std::endl;
 // use constants
 if (HitMin <= value && value < FocusMin)
 HitCounter++;
 else if (FocusMin <= value && value < CritMin && focus)
 FocusCounter++;
 // else? 
 else if (value >= CritMin)
 CritCounter++;
 }
 int GetCounterSum()
 {
 return HitCounter + FocusCounter + CritCounter;
 }
};
// created new class to separate things
class Simulation
{
 private:
 // all context data are declared as members
 const int MinDistributionValue = 1;
 const int MaxDistributionValue = 8;
 Ability AttackAbility;
 Ability HowlAbility;
 const int NAttackDice = 2;
 const bool AttackFocus = true;
 const bool Howlrunner = true;
 const bool CrackShot = true;
 int* AttackDiceResult;
 int HowlRunnerResult;
 bool CrackShotUsed = false;
 const int NEvadeDice = 3;
 const bool EvadeFocus = true;
 // allocated everything on stack -> check stack limit in C++
 int* EvadeDiceResult;
 int EvadeCounter = 0;
 int DefenseFocusCounter = 0;
 double* Damage;
 double DamageSum = 0;
 int nSim = 0;
 // std::random_device rd;
 std::mt19937 mt;
 std::uniform_int_distribution<int> dist;
 // fixed custom initialization 
 void init()
 {
 AttackDiceResult = new int[NAttackDice];
 EvadeDiceResult = new int[NEvadeDice];
 Damage = new double[nSim];
 }
 // generates next 
 int spin() 
 { 
 int ret = dist(mt);
 // std::cout << "Random = " << ret << std::endl;;
 return ret; 
 }
 public:
 // constructor
 Simulation(int simCount) :
 mt(std::random_device()()),
 dist(MinDistributionValue, MaxDistributionValue)
 {
 nSim = simCount;
 init();
 }
 ~Simulation()
 {
 free(AttackDiceResult);
 free(EvadeDiceResult);
 free(Damage);
 }
 // this is done after each step, so it makes sense to have one function
 void ResetCounters()
 {
 AttackAbility.ResetCounters();
 HowlAbility.ResetCounters();
 }
 void PrintSimulationStartData()
 {
 std::cout << "Rolling " << NAttackDice << " attack dice and " << NEvadeDice << " evade dice" << std::endl;
 std::cout << "Attack Focus: " << AttackFocus << " and Evade Focus: " << EvadeFocus << " Howlrunner: " << Howlrunner << " Crack Shot: " << CrackShot << std::endl;
 }
 void PrintSimulationEndData()
 {
 std::cout << "Number of simulations was " << nSim << std::endl;;
 std::cout << "Average damage was " << DamageSum / nSim << std::endl;
 }
 void SimulateStep(int n)
 {
 CrackShotUsed = false;
 for (int i = 0; i < NAttackDice; i++)
 {
 AttackDiceResult[i] = spin();
 AttackAbility.ApplyAbility(AttackDiceResult[i], AttackFocus);
 }
 // not needed outside
 int HowlRunnerHitCheck = AttackAbility.GetCounterSum();
 if (Howlrunner && HowlRunnerHitCheck < NAttackDice)
 {
 HowlRunnerResult = spin();
 HowlAbility.ApplyAbility(HowlRunnerResult, AttackFocus);
 }
 for (int i = 0; i < NEvadeDice; i++)
 {
 EvadeDiceResult[i] = spin();
 if (Ability::HitMin <= EvadeDiceResult[i] && EvadeDiceResult[i] < Ability::FocusMin)
 EvadeCounter++;
 else if (EvadeDiceResult[i] >= Ability::FocusMin && EvadeDiceResult[i] <= Ability::CritMin && EvadeFocus)
 DefenseFocusCounter++;
 // else?
 }
 int CrackShotHitCheck = AttackAbility.GetCounterSum() + HowlAbility.GetCounterSum();
 if (CrackShot && !CrackShotUsed && EvadeCounter >= 1 && CrackShotHitCheck > 0)
 {
 EvadeCounter--;
 CrackShotUsed = true;
 }
 if (CrackShot && !CrackShotUsed && DefenseFocusCounter >= 1 && CrackShotHitCheck > 0)
 {
 DefenseFocusCounter--;
 CrackShotUsed = true;
 }
 Damage[n] = std::max(0, CrackShotHitCheck - EvadeCounter - DefenseFocusCounter);
 std::cout << "Damage " << Damage[n];
 // contracted
 DamageSum += Damage[n];
 }
 void Simulate()
 {
 for (int n = 0; n < nSim; ++n)
 {
 SimulateStep(n);
 ResetCounters();
 }
 }
};
int main()
{
 const int nSim = 10;
 Simulation sim(nSim);
 sim.PrintSimulationStartData();
 sim.Simulate();
 sim.PrintSimulationEndData();
 return 0;
}

• Olorun: The attack dice has a 3/8 probability to return a hit, 2/8 for focus, 1/8 for a critical hit and the remaining 2/8 are misses. So I just "translated" that into 1-3 being hits, 4-5 focus, 6 a crit and 7/8 change nothing. That's the way the game works, I'm just implementing a simulation for that.
• Olorun: Also, this looks a little daunting to me - what's the advantage of using classes instead of functions, as the other answer suggested?
• Alexei: @Olurun - for your particular simulation, it works. However, as a programmer it is important to make good habits (which eventually end up in best practices becoming second nature). This means to not have hardcoded values floating around, as other programmers will not understand their meaning (even you will forget after some time). Also, classes allow a clear separation of what you are doing. There is a lot to tell here and [mindview.net/Books/TICPP/ThinkingInCPP2e.html] (Thinking in C++) is a very good book to learn about them.