0
\$\begingroup\$

I'm using the asset BestHTTP in unity and I'm downloading a few image from amazonaws to use as texture. It used to be one image, but now I have a few.

I was not able to shorten it. When I'm trying to do it in a foreach loop loop it is misbehaving.

Could someone help me to optimize this working code?

for (int i = 0; i < tzPlayInfo.Instance.bc_gametablelist.Count; i++)
{
    dealer_img += tzPlayInfo.Instance.bc_gametablelist[i].dlrimage;
    dealer_img += ",";
}
string[] newLinks = dealer_img.Split(',');

new BestHTTP.HTTPRequest(new System.Uri("**************************.amazonaws.com/resources/" 
    + "dealer/pic/" + newLinks[0]),
    (BestHTTP.HTTPRequest req, BestHTTP.HTTPResponse res)
    =>
    {
        var tex = new Texture2D(20, 20);
        tex.LoadImage(res.Data);
        uitex[0].GetComponent<UITexture>().mainTexture = tex;
    }).Send();

new BestHTTP.HTTPRequest(new System.Uri("**************************.amazonaws.com/resources/"
    + "dealer/pic/" + newLinks[1]),
    (BestHTTP.HTTPRequest req, BestHTTP.HTTPResponse res)
    =>
    {
        var tex = new Texture2D(20, 20);
        tex.LoadImage(res.Data);
        uitex[1].GetComponent<UITexture>().mainTexture = tex;
    }).Send();

new BestHTTP.HTTPRequest(new System.Uri("**************************.amazonaws.com/resources/"
    + "dealer/pic/" + newLinks[2]),
    (BestHTTP.HTTPRequest req, BestHTTP.HTTPResponse res)
    =>
    {
        var tex = new Texture2D(20, 20);
        tex.LoadImage(res.Data);
        uitex[2].GetComponent<UITexture>().mainTexture = tex;
    }).Send();

new BestHTTP.HTTPRequest(new System.Uri("**************************.amazonaws.com/resources/"
    + "dealer/pic/" + newLinks[3]),
    (BestHTTP.HTTPRequest req, BestHTTP.HTTPResponse res)
    =>
    {
        var tex = new Texture2D(20, 20);
        tex.LoadImage(res.Data);
        uitex[3].GetComponent<UITexture>().mainTexture = tex;
    }).Send();
\$\endgroup\$
5
\$\begingroup\$

Instead of using a foreach here, you need to use a for-loop, since you have to know the index of the image to assign it to the correct texture.

// this line replaces the first for loop you have there
var textureLinks = tzPlayInfo.Instance.bcGameTables.Select(t => t.dlrimage).ToArray();
// verify we have enough uitex elements:
if (uitex.Length < textureLinks.Length) 
{
    throw new RuntimeException("too many textures linked");
}
for (int i = 0; i < textureLinks.Length; i++)
{
    new BestHTTP.HTTPRequest(new System.Uri($"**.amazonaws.com/resources/dealer/pic/{textureLinks[i]}"),
        (req, res) => {
            var tex = new Texture2D(20, 20);
            tex.LoadImage(res.Data);
            uitex[i].GetComponent<UITexture>().mainTexture = tex;
        }).Send();
}

There's also some words to be lost on the naming convention you use.

dealer_img should be dealerImg, uitex should be uiTextures, bc_gametablelist should be bcGameTables, dlrimage should be dealerImage...

In general you seem to try to shorten your names and adhere to C++ conventions. That would be fine if you were programming the graphics in C++. But since this is C#, you should be following C# convention (namely camelCase)

In the code above I made use of interpolated strings for the Uri. I also left the variable types of the lambda up for inference by the compiler. It should be smart enough to do so, if BestHTTP properly specified the generics on HTTPRequest.

\$\endgroup\$
  • \$\begingroup\$ How could i seperate this one sir **.amazonaws.com/resources and /dealer/pic i need to seperate them for some purpose \$\endgroup\$ – TheGinxx009 May 7 '18 at 10:34
  • \$\begingroup\$ and i have this error Feature interpolated strings' cannot be used because it is not part of the C# 4.0 language specification` \$\endgroup\$ – TheGinxx009 May 7 '18 at 10:38
  • 2
    \$\begingroup\$ The "Feature interpolates strings cannot be used" is fixed by using an up-to date version of the C# language. Update your project settings to use C#7 or newer. The "out of index" is pretty much impossible with the code I've provided. The only exception to that is if you did not in fact show the actual code you're using. Also "For some purpose" is not sufficent explanation. I can't mindread what your actual code is. You need to show your actual code. \$\endgroup\$ – Vogel612 May 7 '18 at 11:04
  • 1
    \$\begingroup\$ @Vogel612 last time I checked Unity barely supported C#4 let alone C#7. But maybe things have changed since then. :) \$\endgroup\$ – Nikita B May 7 '18 at 12:08
  • 1
    \$\begingroup\$ what do you mean by "it is still out of index" ? you start the incrementation variable at 0 and loop until you loop the number of times equivalent to the number of items in the tzPlayInfo.Instance.bc_gametablelist collection \$\endgroup\$ – Malachi May 7 '18 at 17:06
3
\$\begingroup\$

I noticed that you are using a string variable to list images and then you create a string array and split the string variable into the array

for (int i = 0; i < tzPlayInfo.Instance.bc_gametablelist.Count; i++)
{
    dealer_img += tzPlayInfo.Instance.bc_gametablelist[i].dlrimage;
    dealer_img += ",";
}
string[] newLinks = dealer_img.Split(',');

you can actually skip a variable here and assign directly to the string array, like this

string[] newLinks = new string[tzPlayInfo.Instance.bc_gametablelist.Count];
for (int i = 0; i < tzPlayInfo.Instance.bc_gametablelist.Count; i++)
{
    newLinks[i] = tzPlayInfo.Instance.bc_gametablelist[i].dlrimage;
}

This is probably more efficient than what you were doing, but I don't know what you were doing with dealer_img before the for loop because you haven't given the entire Method or Class.

because we use this value more than once I might add a variable to hold the count to make this code more readable as well.

int gameTableListCount = tzPlayInfo.Instance.bc_gametablelist.Count;
string[] newLinks = new string[gameTableListCount];

for (int i = 0; i < gameTableListCount; i++)
{
    newLinks[i] = tzPlayInfo.Instance.bc_gametablelist[i].dlrimage;
}

now if I am reading the rest of the code correctly, you should be able to move code into the for loop like @Vogel612 suggests. if you need to do it without the interpolation then you could do it like this, keep in mind I did not change much of your original code:

int gameTableListCount = tzPlayInfo.Instance.bc_gametablelist.Count;
for (int i = 0; i < gameTableListCount; i++)
{
    new BestHTTP.HTTPRequest(new System.Uri("**.amazonaws.com/resources/" 
            + "dealer/pic/" + tzPlayInfo.Instance.bc_gametablelist[i].dlrimage),
        (BestHTTP.HTTPRequest req, BestHTTP.HTTPResponse res) =>
        {
            var tex = new Texture2D(20, 20);
            tex.LoadImage(res.Data);
            uitex[i].GetComponent<UITexture>().mainTexture = tex;
        }).Send();
}

I don't think there is anything else to suggest because you haven't provided the entire code.

\$\endgroup\$
2
\$\begingroup\$

It is odd you add "," then split on it. This is inefficient as string is immutable.

StringBuilder sb = new StringBuilder();
for (int i = 0; i < tzPlayInfo.Instance.bc_gametablelist.Count; i++)
{
    sb.Append(tzPlayInfo.Instance.bc_gametablelist[i].dlrimage);
    sb.Append(",");
}
string[] newLinks = sb.ToString().Split(',');
\$\endgroup\$
-4
\$\begingroup\$
for (int i = 0; i < tzPlayInfo.Instance.bc_gametablelist.Count; i++)
    {
        dealer_img += tzPlayInfo.Instance.bc_gametablelist[i].dlrimage;
        dealer_img += ",";
    }
    string[] newLinks = dealer_img.Split(',');

    for (int i = 0; i < newLinks.Length - 1; i++)
    {
        var index = i;  // We need to make a local copy because C# captures variables by reference to lambdas.
        new BestHTTP.HTTPRequest(new System.Uri("********************.amazonaws.com/resources/"
            + "dealer/pic/" + newLinks[index]),
            (BestHTTP.HTTPRequest req, BestHTTP.HTTPResponse res)
            =>
            {
                var tex = new Texture2D(20, 20);
                tex.LoadImage(res.Data);
                uitex[index].GetComponent<UITexture>().mainTexture = tex;
            }).Send();
    }

It's weird but it works.

\$\endgroup\$
  • \$\begingroup\$ note that the capture as reference behaviour is not quite the actual behaviour, at least as of C#5 foreach loops use a different desugaring to provide the expected semantics: stackoverflow.com/q/16264289/1803692 \$\endgroup\$ – Vogel612 May 7 '18 at 11:09
  • \$\begingroup\$ No guarantee uitex is newLinks.Length. \$\endgroup\$ – paparazzo May 7 '18 at 11:35

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.