8
\$\begingroup\$

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 < assetData.Assets.Count; i++){
    GameObject newAsset = null;

    if(((Assets)assetData.Assets[i]).AssetType == "Text"){

        newAsset = (GameObject)Instantiate(asset, new Vector3(15*i, 0, 0), Quaternion.identity);
        newAsset.GetComponent<TextMesh>().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;
        StartCoroutine(DownloadImage(url, newAsset, ((Assets)assetData.Assets[i]).AssetFilename, "IMAGE"));

        newBrick.transform.rotation = Quaternion.Euler(0, 0, 0);

        float posXtag;
        posXtag = (((((Assets)assetData.Assets[i]).AssetLeft * 100f / 1024f)  + (((Assets)assetData.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);
        newAsset.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 == "Video"){

        newAsset = (GameObject)Instantiate(video, new Vector3(15*i, 0, 0), Quaternion.identity);
        newAsset.GetComponent<Playback>().m_path = ((Assets)assetData.Assets[i]).AssetContent;

        string url = ((Assets)newAssetData.Assets[i]).AssetThumbnail;
        StartCoroutine(DownloadImage(url, newAsset, ((Assets)assetData.Assets[i]).AssetFilename, "VIDEO"));


        newAsset.transform.rotation = Quaternion.Euler(0, -180, 0);

        float posXtag;
        posXtag = (((((Assets)assetData.Assets[i]).AssetLeft * 100f / 1024f)  + (((Assets)assetData.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);
        newAsset.transform.localScale = new Vector3(((Assets)assetData.Assets[i]).AssetWidth/102.4f,0.2f,((Assets)assetData.Assets[i]).AssetHeight/102.4f);

    }

    newAsset.transform.tag = "Asset";


    // IF ASSET IS SET TO FADEIN DO THAT HERE:
    if(((Assets)assetData.Assets[i]).AssetFadein == "true"){
        iTween.FadeTo(newBrick, 1f, 1);
    } else {
        Color color = newAsset.renderer.material.color;
        color.a = 1f;
        newAsset.renderer.material.color = color;
    }
    // EOF ASSET FADEIN

    if(((Assets)assetData.Assets[i]).AssetAction != ""){
        newAsset.AddComponent("TouchListener");
        newAsset.GetComponent<TouchListener>().actionUrl = ((Assets)assetData.Assets[i]).AssetAction;
    }

}

My JSON file can contain information of 3 "asset" types (text, image or video), each asset type renders a different prefab (Text = asset, Image = iasset, Video = video). I then set the different parameters for the prefab:

  • Hieght
  • Width
  • position x-axis
  • position y-axis
  • background color (text only)
  • texture (image and video)
  • fadein animation (if set)
  • touch action (if set)

My code works fine, but as it is my first time coding in C# I am sure there is a cleaner way of getting the job done. Any suggestion would be greatly appreciated!

\$\endgroup\$
6
\$\begingroup\$

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<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.

Your main loader then looks like:

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
          }

     }
}

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<AssetData>. 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.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the detailed response @Jeff, I am going to integrate it and test and will post back \$\endgroup\$ – Ronny vdb Jan 28 '14 at 11:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.