The idea here is that rather than placing objects directly into the scene, these building objects are instantiated at run time and placed in the correct locations. This gives me full control over where and how to place the buildings.
The workflow is still a little involved. I have to import the model and texture image, drag it out into the scene, scale it, add a Mesh collider to it, and then add all of that to a prefab object that I also need to create. Then I need to add the name of that prefab to the script below. This must be done for each model that I want to use in the game. This approach is still a whole lot better than having to manually place and position each object in the Unity scene though.
GameModelScript.cs
using UnityEngine;
using System;
using System.Collections;
using System.Collections.Generic;
public class GameModelScript : MonoBehaviour {
public int worldSize;
private System.Random random = new System.Random();
void Start () {
List<string> names = new List<string> ();
names.Add ("arco01_2_prefab");
names.Add ("arco03_prefab");
names.Add ("arco04_prefab");
names.Add ("arco05_prefab");
names.Add ("ballTower01_prefab");
names.Add ("bank01_prefab");
names.Add ("barn01_prefab");
names.Add ("bigApartments05_prefab");
names.Add ("bigFactory06_prefab");
names.Add ("bigMatterCube01_prefab");
names.Add ("busDepot01_prefab");
names.Add ("casino01_prefab");
for (int y = 0; y < worldSize; y+=30) {
for (int x = 0; x < worldSize; x+=30) {
int randomNumber = random.Next(0, names.Count);
GameObject tileObject = (GameObject)GameObject.Instantiate (Resources.Load (names[randomNumber]));
tileObject.transform.position = new Vector3(x, 10, y);
}
}
}
void Update () {
}
}
My plan is to use this foundation to build some kind of superhero city exploration game. Here is a link to a very early playable version: Super Voxel
And here is a little screenshot from up high:
-
\$\begingroup\$ Do you have any plans for this? Like a game, or something? Or is this just a test? Very cool by the way ;-) \$\endgroup\$Ethan Bierlein– Ethan Bierlein2015年08月16日 02:24:13 +00:00Commented Aug 16, 2015 at 2:24
-
1\$\begingroup\$ @EthanBierlein see this question for something a little more game like: codereview.stackexchange.com/q/101324/14608 \$\endgroup\$bazola– bazola2015年08月18日 22:14:09 +00:00Commented Aug 18, 2015 at 22:14
2 Answers 2
Unlike Java List
s, C# lists can be pre-initialized with values, like this:
List<string> names = new List<string>()
{
"arco01_2_prefab",
...
};
This eliminates the need to call .Add
, and improves code readability as well.
In the future as well, if you ever plan to add more buildings, it'll be a pain to edit this script to include the new building names. Instead, you could declare a public variable which allows you to drag and drop the model prefabs instead:
public List<GameObject> buildings;
This also eliminates the need to call Resources.Load
.
In addition, the namespace UnityEngine
provides a class Random
which can be used over the default System.Random
. Unlike System.Random
, UnityEngine.Random
is a static class doesn't need to be instantiated, and you can just call Random.Range
like this:
int randomNumber = (int)Random.Range(0, names.Count);
Other than that, there are just a few things I want to nitpick.
- You don't need to call
Instantiate
from theGameObject
class. It can just be called with out theGameObject.
prefix. In addition, the accepted Unity style for storing an instantiated
GameObject
is like this:GameObject gameObject = Instantiate( ... ) as GameObject;
If you're feeling up to dynamic typing, this also works as well (Thanks @Kroltan):
var gameObject = Instantiate( ... ) as GameObject;
You can also remove the
Update
function. It doesn't do anything, and can decrease performance, due to the fact that it's running many times per frame.- The general style for braces in C# is to put them on the next line, not like Java braces.
- You aren't using
System.Collections
, so you can remove theusing System.Collections;
.
-
1\$\begingroup\$ Or even better,
var gameObject = Instantiate( ... ) as GameObject
. Since there is already a cast, it might look cleaner to infer the type of the variable itself, considering thatvar
is a purely compile-time construct with no overhead. \$\endgroup\$Kroltan– Kroltan2015年08月14日 02:55:57 +00:00Commented Aug 14, 2015 at 2:55
Just as a note ahead of time, I don't know much about C# myself so my review will be fairly short.
Rather than creating a new List
and then creating add
ing all the entries you need afterwards, a cleaner (and possibly faster) solution would be to...
Create an array of all the entries you'd like to add.
Use
ToList
to create a new list.
That would look something like this:
string[] prefabs = new string[] {"arco01_2_prefab", ...};
List<string> names = prefabs.ToList();
You have a few magic numbers in your code:
for (int y = 0; y < worldSize; y+=30) {
and:
tileObject.transform.position = new Vector3(x, 10, y);
Where did the 30 come? And the 10?
I recommend that you create to const
ants to hold these values, and then to use the constants instead of the normal number.
That might look like this (in your class):
private const int BUILDING_SPACING = 30;
private const int BUILDING_HEIGHT = 10;
Note that if you want to edit these in the Unity editor, you will have to make these public
and no longer const
ants. Thanks Ethan Bierlein.
Then, in your code, you would just use BUILDING_SPACING
and BUILDING_HEIGHT
as needed.
Note: You already have a constant worldSize
. This should be created with const
.
-
\$\begingroup\$ The two variables
BUILDING_SPACING
andBUILDING_HEIGHT
should beprivate
, unless @bazola wants them to be modifiable through the Unity Editor, in which case, they shouldn't beconst
. \$\endgroup\$Ethan Bierlein– Ethan Bierlein2015年08月16日 01:12:29 +00:00Commented Aug 16, 2015 at 1:12