- Home /
Unity stops responding when i ask it to execute the same function for the third time
I've built a function that allows me to generate an instance of a class, which is then added to a list and that list generates a series of buttons. At 'midnight' on 'Sunday night', I need it to generate six of these in a row ie. getting me six instances of the class, each of which will generate a button. But it's not working.
It is perfectly fine if I only ask it to generate two instances, but when I put a third instance in, the editor freezes at 'midnight'. I've checked it via generating through a button, and again it will allow the first two but not the third, even when there is a pause between commands. The same thing happens if there's a combination of button and timed generation. Is there any reason that this would happen?
Edit 1: This is the part of my script that I think might be responsible (ie. the whole system works perfectly fine without it but when this is called as part of the class-generating function, the above happens).
Setting the variables at various points earlier in the script:
public static string[] agentstats = {"seduction", "sneakiness", "sniperskills", "deception", "combat"};
initialskillpoints = 70;
Actual function:
void CalculateAgentStats () {
print ("Calculating Stats");
string statname;
for (int i = 0; i < initialskillpoints; i++) {
statname = agentstats [Random.Range (0, agentstats.Length)];
switch (statname) {
case "seduction":
if (seduction == 20) {
i -= 1;
break;
}
else
{
seduction ++;
break;
}
case "sneakiness":
if (sneakiness == 20) {
i-= 1;
break;
}
else
{
sneakiness ++;
break;
}
case "sniperskills":
if (sniperskills == 20) {
i-= 1;
break;
}
else
{
sniperskills ++;
break;
}
case "deception":
if (deception == 20) {
i-= 1;
break;
} else
{
deception ++;
break;
}
case "combat":
if (combat == 20) {
i-= 1;
break;
}
else
{
combat ++;
break;
}
}
}
}
Edit 2: So, a bit more work debugging/crashing Unity found me the problem but I don't know how to fix it. The problem is the "i -= 1" line. I think it's causing a potential infinite loop because if you had epically bad luck with your randomization, you could get caught in a i++, i-=1 circle. But the odds of that (first getting to 20, and then forever randomly choosing that same stat) are so low that I can't see why Unity would object to it.
Can anyone explain this, and tell me how I could get this same functionality to work (ie. if a stat has already reached 20, it can't go any higher and the loop needs to go again).
You should try to debug it, step by step or add Debug.Logs to the code in order to see what gets executed until it freezes. It's hard to guess without having an idea where to start.
Answer by Suddoha · Oct 01, 2015 at 10:23 AM
Good point with the hint to the i-=1, didn't even notice that when i first looked at it.
It's in fact pretty dangerous to rely on random generation when there's the possibilty of non-usable results so that you have to generate a new option again until you find one. A lot of people also do that with pooling systems which may end up having a used-up pool which cannot offer any object anymore and so they'll run the risk to freeze the game or get huge framedrops.
In your case, you could make a local list in that method which stores all the entries again. As soon as one hits 20, remove the entry from the list. The result would be, that you won't generate the one that has already 20 skill points, so you restrict the random generation to the others which are left in the list.
An implementation detail: I'd personally use something else than strings, as you have to do (at least) 70 comparisons per agent with your method. Maybe there's also another way to distribute the skill points.
Thanks Suddoha. I'll have another look at the list idea (although at a quick thought, that's going to involve yet another loop checking for when a stat hits 20,). To be fair, if I take the limitation out, I've never run across a situation where over 20 was generated. But the fact that it's technically possible tells me if I don't fix it, I'm looking at a future bug, right?
I'm not sure what you mean by your last paragraph. Is checking a string vastly more difficult for the system than an int? That would be pretty easy for me to change if it is.
I did look at doing a single random number generation for each stat (so there would be a total of 5 ins$$anonymous$$d of 70), but ran into problems regarding putting a cap on the total stat number (I'm differentiating agent/enemy levels based on total point numbers). Unless I combine the two...run the single number generation once with enough limitations to keep it below the cap and then the loop for any 'remainder'. But that still puts me back in this same problem, just with fewer rounds.
The only other way I've found that might work would be the one mentioned on the unity manual page (http://docs.unity3d.com/$$anonymous$$anual/RandomNumbers.html). ie. /C#
float CurveWeightedRandom(AnimationCurve curve) { return curve.Evaluate(Random.value); }
But although the end result is what I want, I don't really understand how it's supposed to work and so can't implement it. Any chance you could explain that one?
It does not involve another loop to check that, only a short one to add all characteristics to a List once in the beginning of the method.
Anyway, checking strings CAN be inefficient. In your case it might not be extremly inefficient due to how string comparison works. Simply said, It checks things like length and hashcodes first, only if they match, it starts to compare character by character. But that usually happens only when there are many strings to compare to.
However, there are other problems: typos for example etc. Usually they can be avoided by using constants, so you define the string in one place and use a constant so every change to the string applies to every place where the constant is used (very useful for tags).
But there are other options: Integer? Well, not reliable enough. You could pass invalid ints which are anything else than the options you want to have.
Therefore, a nice and handy tools are enumerations. They're typesafe, you cannot pass invalid arguments because they're are kind of "named integer".
For the following example i added:
enum Characteristic { Seduction, Sneakiness, SniperSkills, Deception, Combat }
private const int $$anonymous$$AX_POINTS = 20;
I changed your implementation a bit, using enums for the part of skillpoint distribution:
void CalculateAgentStats()
{
print("Calculating Stats");
// the local list
List<Characteristic> characteristics = new List<Characteristic>();
// add all options dynamically, just in case you add more characteristics later one
// so you won't need to take care to add the new ones manually as well
foreach (Characteristic c in System.Enum.GetValues(typeof(Characteristic)))
characteristics.Add(c);
for (int i = 0; i < initialskillpoints; i++)
{
Characteristic c = characteristics[Random.Range(0, characteristics.Count)];
switch (c)
{
case Characteristic.Seduction:
seduction++;
if (seduction == $$anonymous$$AX_POINTS)
characteristics.Remove(c);
break;
case Characteristic.Sneakiness:
sneakiness++;
if (sneakiness == $$anonymous$$AX_POINTS)
characteristics.Remove(c);
break;
case Characteristic.SniperSkills:
sniperskills++;
if (sniperskills == $$anonymous$$AX_POINTS)
characteristics.Remove(c);
break;
case Characteristic.Deception:
deception++;
if (deception == $$anonymous$$AX_POINTS)
characteristics.Remove(c);
break;
case Characteristic.Combat:
combat++;
if (combat == $$anonymous$$AX_POINTS)
characteristics.Remove(c);
break;
}
}
}
Thanks for that, Suddoha. I've combined the enum method and some more fiddling with the function itself to reduce the number of iterations, which has made the whole process much faster!
Your answer
Follow this Question
Related Questions
Multiple Cars not working 1 Answer
Distribute terrain in zones 3 Answers
A node in a childnode? 1 Answer
how to sort a list of gameobjects? 1 Answer