HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Loop through JSON and create prefab in Unity3D

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unity3dcreateloopandthroughjsonprefab

Problem

I am creating an app in Unity3D and it is my first time coding in C#.
I use the following code to parse a JSON file hosted on my server and use the information to render prefabs within my scene:

```
// Parses the json Object
JSONParser parser = new JSONParser();

AssetData assetData = parser.ParseString(mJsonAssetInfo.text);
mAssetData = assetData;

// Updates state variables
mIsLoadingAssetData = false;

for(var i = 0; i ().text = ((Assets)assetData.Assets[i]).AssetContent;
newAsset.transform.rotation = Quaternion.Euler(90, -180, 0);

string[] rgba = Regex.Split(((Assets)assetData.Assets[i]).AssetBgcolor, ", ");
float red = float.Parse(rgba[0]);
float green = float.Parse(rgba[1]);
float blue = float.Parse(rgba[2]);
float alpha = float.Parse(rgba[3]);

var child = newAsset.renderer.transform.transform.Find("background");
child.renderer.material.color = new Color(red/255, green/255, blue/255, alpha);

float posXtag;
posXtag = (((((Assets)assetData.Assets[i]).AssetLeft 100f / 1024f) + (((Assets)bookData.Assets[i]).AssetWidth 100f / 1024f) / 2f))-50f;

float posYtag;
posYtag = -1((Assets)assetData.Assets[i]).AssetTop 50f / 512f - ((Assets)assetData.Assets[i]).AssetHeight * 50f / 512f / 2f +25f;

newAsset.transform.localPosition = new Vector3(posXtag,0,posYtag);
child.renderer.transform.localScale = new Vector3(((Assets)assetData.Assets[i]).AssetWidth/102.4f,0.2f,((Assets)assetData.Assets[i]).AssetHeight/102.4f);

} else if (((Assets)assetData.Assets[i]).AssetType == "Image"){

newAsset = (GameObject)Instantiate(iasset, new Vector3(15*i, 0, 0), Quaternion.identity);
newAsset.transform.parent = AugmentationObject.transform;
Color color = newAsset.renderer.material.color;
color.a = 0f;
newAsset.renderer.material.color = color;
string url = ((Assets)bookData.Assets[i]).AssetContent;
StartCorout

Solution

I would split the asset loading into 3 different classes: TextAsssetLoader, ImageAssetLoader, and VideoAssetLoader. This will allow you to separate the loading of each type. Additionally, I would create an interface so you can eliminate the if statement in your current code.

public interface IAssetLoader
{
    GameObject Create(AssetData assetData, int index);
}


Then each class would be:

public class TextAssetLoader : IAssetLoader
{
    public GameObject Create(AssetData assetData, int index)
    {
        var newAsset = (GameObject)Instantiate(asset, new Vector3(15*index, 0, 0), Quaternion.identity);
        newAsset.GetComponent().text = assetData.AssetContent;
        newAsset.transform.rotation = Quaternion.Euler(90, -180, 0);

        string[] rgba = Regex.Split(assetData.AssetBgcolor, ", ");
        float red = float.Parse(rgba[0]);
        float green = float.Parse(rgba[1]);
        float blue = float.Parse(rgba[2]);
        float alpha = float.Parse(rgba[3]);

        var child =  newAsset.renderer.transform.transform.Find("background");
        child.renderer.material.color = new Color(red/255, green/255, blue/255, alpha);

        float posXtag;
        posXtag = (((((Assets)assetData.Assets[i]).AssetLeft * 100f / 1024f)  + assetData.AssetWidth * 100f / 1024f) / 2f))-50f;

        float posYtag;
        posYtag = -1*assetData.AssetTop * 50f / 512f - (assetData.AssetHeight * 50f / 512f / 2f +25f;

        newAsset.transform.localPosition = new Vector3(posXtag,0,posYtag);
        child.renderer.transform.localScale = new Vector3(assetData.AssetWidth/102.4f,0.2f,assetData.AssetHeight/102.4f);
    }
}

// Next two clases are pretty much the same.


Your main loader then looks like:

public class AssetLoader
{
     private static readonly IDictionary Loaders = new Dictionary
                   {
                       { "Text", new TextAssetLoader() },
                       { "Image", new ImageAssetLoader() },
                       { "Video", new VideoAssetLoader() },
                   };

     public void LoadAssets()
     {

          for(var i = 0; i < assetData.Assets.Count; i++)
          {
              var asset = assetData.Assets[i];

              GameObject gameObject = Loaders[asset..AssetType].Create(asset, i);

              // Rest of the code to deal with gameObject
          }

     }
}


On top of this change, I would suggest getting rid of all the casts in your code; ((Assets)assetData.Assets[i]) gets a little confusing. If you have to cast it do it once as in my code, a better alternative is to have an AssetData class, and have the Assets and IEnumerable. This way you don't have to ever cast it in your code; it is done on deserialization.

Standard C# guidelines suggest having opening and closing braces on separate lines:

if (something)
{
}


Other than making it easier to read, I don't see too much else that needs attention.

Code Snippets

public interface IAssetLoader
{
    GameObject Create(AssetData assetData, int index);
}
public class TextAssetLoader : IAssetLoader
{
    public GameObject Create(AssetData assetData, int index)
    {
        var newAsset = (GameObject)Instantiate(asset, new Vector3(15*index, 0, 0), Quaternion.identity);
        newAsset.GetComponent<TextMesh>().text = assetData.AssetContent;
        newAsset.transform.rotation = Quaternion.Euler(90, -180, 0);

        string[] rgba = Regex.Split(assetData.AssetBgcolor, ", ");
        float red = float.Parse(rgba[0]);
        float green = float.Parse(rgba[1]);
        float blue = float.Parse(rgba[2]);
        float alpha = float.Parse(rgba[3]);

        var child =  newAsset.renderer.transform.transform.Find("background");
        child.renderer.material.color = new Color(red/255, green/255, blue/255, alpha);

        float posXtag;
        posXtag = (((((Assets)assetData.Assets[i]).AssetLeft * 100f / 1024f)  + assetData.AssetWidth * 100f / 1024f) / 2f))-50f;

        float posYtag;
        posYtag = -1*assetData.AssetTop * 50f / 512f - (assetData.AssetHeight * 50f / 512f / 2f +25f;

        newAsset.transform.localPosition = new Vector3(posXtag,0,posYtag);
        child.renderer.transform.localScale = new Vector3(assetData.AssetWidth/102.4f,0.2f,assetData.AssetHeight/102.4f);
    }
}

// Next two clases are pretty much the same.
public class AssetLoader
{
     private static readonly IDictionary<string IAssetLoader> Loaders = new Dictionary<string, IAssetLoader>
                   {
                       { "Text", new TextAssetLoader() },
                       { "Image", new ImageAssetLoader() },
                       { "Video", new VideoAssetLoader() },
                   };

     public void LoadAssets()
     {

          for(var i = 0; i < assetData.Assets.Count; i++)
          {
              var asset = assetData.Assets[i];

              GameObject gameObject = Loaders[asset..AssetType].Create(asset, i);

              // Rest of the code to deal with gameObject
          }

     }
}
if (something)
{
}

Context

StackExchange Code Review Q#40221, answer score: 6

Revisions (0)

No revisions yet.