Random is not working...

Unity Engine
1

I am attempting to instantiate nine rooms, in a square, and each room is chosen at random from a list of prefabs. Instead, this script creates nine of the same room, not even at random.

function Start () {
for (i = 0; i < 3; i++)
	{
	random = Random.Range(1,101);
	random2 = Random.Range(1,101);
	random3 = Random.Range(1,101);
	if (random <= 100) //First line of rooms
		{
		Instantiate(room01, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
	else if (random <= 50)
		{
		Instantiate(room02, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
	else if (random <= 25)
		{
		Instantiate(room03, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
	else if (random <= 5)
		{
		Instantiate(room04, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
		
	if (random2 <= 100) //Second Line
		{
		Instantiate(room01, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
	else if (random2 <= 50)
		{
		Instantiate(room02, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
	else if (random2 <= 25)
		{
		Instantiate(room03, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
	else if (random2 <= 5)
		{
		Instantiate(room04, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
		
	if (random3 <= 100) //Third Line
		{
		Instantiate(room01, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	else if (random3 <= 50)
		{
		Instantiate(room02, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	else if (random3 <= 25)
		{
		Instantiate(room03, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	else if (random3 <= 5)
		{
		Instantiate(room04, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	}
}
2

Look into using arrays (built-in, not UnityScript’s Array class)

They will shorten that script immensely and make it easier to read.

3

Well, if

then

must also be true.

In other words, you need to reverse the order of your if statements.

4

Ok. This works for the random rooms, however, now, for whatever reason, the positions of the rooms are all f*** 'ed up.

function Start () {
for (i = 0; i < 3; i++)
	{
	random = Random.Range(1,101);
	random2 = Random.Range(1,101);
	random3 = Random.Range(1,101);
	if (random >= 100) //First line of rooms
		{
		Instantiate(room01, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
	else if (random >= 50)
		{
		Instantiate(room02, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
	else if (random >= 25)
		{
		Instantiate(room03, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
	else if (random >= 5)
		{
		Instantiate(room04, Vector3((-i * 15) - 7.5, 0, 7.5), transform.rotation);
		}
		
	if (random2 >= 100) //Second Line
		{
		Instantiate(room01, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
	else if (random2 >= 50)
		{
		Instantiate(room02, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
	else if (random2 >= 25)
		{
		Instantiate(room03, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
	else if (random2 >= 5)
		{
		Instantiate(room04, Vector3((-i * 15) - 7.5, 0, 15 + 7.5), transform.rotation);
		}
		
	if (random3 >= 100) //Third Line
		{
		Instantiate(room01, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	else if (random3 >= 50)
		{
		Instantiate(room02, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	else if (random3 >= 25)
		{
		Instantiate(room03, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	else if (random3 >= 5)
		{
		Instantiate(room04, Vector3((-i * 15) - 7.5, 0, 30 + 7.5), transform.rotation);
		}
	}
}
5

With what you’ve got there room1 will never get instantiated, will it? You want the expressions you had, but checked in the reverse order.

Exactly how are the room positions broken? My first suggestion it to check out your prefab and make sure they’re centered there.

6

I never actually realized that room01 shouldn’t ever be instantiated, but it does. I have made sure that the prefabs are centered. The part that confuses me is that it worked fine before, and I never touched the positions they spawn at!

7

if (random <=5)…
if (random <=100)…

Even reversing the workflow as angrypenguin suggested, while working, is bad practice.

if (random >50 random < 101)…

Is the method I would choose. Now that particular “if” would only apply if the number is between 51-100 and there will be no editing mistakes or potential bugs later on down the line if you add/subtract and forget you have to put them in a specific order - now they can be ordered as haphazardly as you want and will not break. I always try to be as specific as possible when working with specific ranges - especially if those ranges are if/else.

Also - while not wrong - “<= 5” is like nails on a chalkboard to me ha! Since you’re technically saying “any number lower than 6” - cleaner to leave out the redundant “=” and just use “<6” - since this will check 5 and below (since you’re looking for a number LESS THAN 6). Now you don’t have to add an extra “=” in your code every time as redundancy with literal numbers - UNLESS - you are working with reals - in which case you keep the “=” in your condition since 5.000000001 is greater than 5.

Also - what Dman suggested is the route I would go - arrays.

Also - room 2 will have the greatest chance of being created since it has the widest split of all the randoms.

Also - the loop you have will instantiate one of 4 rooms - 3 times for each run through, right? You want 9 total, right?

So why bother with 3 randoms? The loop runs 3 times… why not just use 1 random and run the loop 9 times?

for (i = 0; i < 3; i++)

    {

    random = Random.Range(1,101);

    random2 = Random.Range(1,101);

    random3 = Random.Range(1,101);

Would be far easier to maintain as:

for (i = 0; i < 9; i++)

    {

    random = Random.Range(1,101);

Since you’re technically getting a new set of randoms every time the loop is run - why not just stick with 1 random since it is “refreshed” every time the loop is run?

8

If I run the loop 9 times and use 1 random, I will end up with every column of rooms being the same:

= 1, @ = 2, $ = 3

#@ #@
#$@

And that also doesn’t solve the problem that they are being placed at random positions.

9

It shouldn’t since the random will be different every time the loop is run. I’ve used this several times and just tested real quick in my IDE it works flawlessly. Are you making sure to be explicit with your conditions by giving a range of numbers instead of a global?

10

Not to be rude, but I don’t care about shortening the code at the moment. All I need to figure out is why the f*** these rooms are spawning as if they’re handicapped.

11

Well if it worked before you swapped “<” with “>” - I’ve had quite a few odd “happenings” today which required a reboot of Unity to get working right again.

I highly doubt swapping a condition changed the placement of your rooms since that has nothing to do with Vector3. Have you simply tried rebooting Unity? Sounds simple but with the amount of hiccups I’ve seen in 3.5 since it went live I wouldn’t be surprised.

12

I remade all the rooms, and restarted Unity. No luck.

I also tried attaching a script to each room that rounds the position to the nearest(x). I tried x= 15, 7.5, and 0.5. Still, with no luck. This is seriously starting to annoy me considering there is no legitimate reason for this to happen in the first place.

13

When my code suddenly starts acting weird, but it used to work, I usually just reverts the code back to how it was when it worked, and tries again to make whatever I want to happen happen. It’s easier then to use time figuring out why the new code didn’t work.

Here, I think this should work. Less code = less to edit. I have taken into account the difference between random 1 2 and 3, see the s and r variables. And I reversed the order of the if statements, now you shouldn’t have situations where the rooms didn’t change, which you’r second code had.
PS: I haven’t tested the code, just wrote it down, so I cant guarantee there’s no errors.

 function Start () {
for (i = 0; i < 9; i++)
    {
    random = Random.Range(1,101);
	s = Mathf.Ceil((i+1)/3);
	r = (15*(i%3));
    if (random <= 5)
        {
        Instantiate(room04, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
        } 
    else if (random <= 25)
        {
        Instantiate(room03, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
        }
    else if (random <= 50)
        {
        Instantiate(room02, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
        }
    else 
		{
        Instantiate(room01, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
        }
    }
}
14

+! If you don;t know whats going on - first thing to do is try to isolate your problem. Reducing code usually helps this.

However, why all the parenthesis?

 function Start () {
for (i = 0; i < 9; i++)
    {
    random = Random.Range(1,101);
	s = Mathf.Ceil((i+1)/3);
	r = (15*(i%3));

    if (random <= 5)
        Instantiate(room04, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
    else if (random <= 25)
        Instantiate(room03, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
    else if (random <= 50)
        Instantiate(room02, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
    else 
        Instantiate(room01, Vector3((-s * 15) - 7.5, 0, r+7.5), transform.rotation);
    }
}

And I like my code to be readable, concise and to the point. All those Instantiates are not readable, are not concise, and have tons of duplication! Try:

 function Start() {
     for (i = 0; i < 9; i++) {

         random = Random.Range(1, 101);
         var room: GameObject;

         if (random < 5) room = room04;
         else if (random < 25) room = room03;
         else if (random < 50) room = room02;
         else room = room01;

         s = Mathf.Ceil((i + 1) / 3) * -15 - 7.5;
         r = (15 * (i % 3)) + 7.5;

         Instantiate(room, Vector3(s, 0, r), transform.rotation);
     }
 }

There’s probably a little more you can do - but with 2-3mins I’ve massively reduced the complexity of the problem, and made it far easier to insert Log Statements, and made it easier to change key variables [e.g. ‘s’ position now needs to be changed once, instead of in 5 different places as before].

15

Not to sound extremely rude, but…

…is an even bigger problem than the rooms not instantiating correctly. I’m quite sure we’re all trying to help - and even I learned something new consolidation trick in here - and this isn’t even my issue.

16

All I was trying to say is that there are bigger problems than compacting the size of the code at the moment.

17

Did you try the last code NPSF3000 wrote?

The thing is, when the code is shorter like this, it makes it much easier to figure out whats wrong, and to change things to see if that works better. We’re not doing this just to shorten down your code, we’re doing this to figure out whats wrong!

18

Is it only the positions that are messed up, or is it the rotations…

19

So limiting the scope of what can be causing errors isn’t important. Got it. You do realize optimizing the code will help exclude an astronomical amount of lines from being possible culprits, right?

This isn’t just to make it shorter - it limits the range of possible errors by eliminating redundant code - you’re asking for help and we are giving it to you - you’re just refusing to listen. If you don’t understand how optimizing your code will help - perhaps its time to start from “Hello World” all over again.

20

Right…

  • Got a problem with how many loops your code is doing? Put a debug on line 3.
  • Got a problem with which room type is being spawned? Put a debug on line 6 and 11.
  • Got a problem with position of the spawned objected? Line 14 is your friend.

Now try doing the same with the original code.