Is this the right way to iterate through list of Items.cs and separate by their quality?

Unity Engine
1 
public void GetWeaponsIntoArrays()
    {
        int lows = 0;
        int commons= 0;
        int rares= 0;
        int mags= 0;
        int heros= 0;
        Weapons_ALL = new string[InventoryManager.Instance.ItemContain.Weapons.Count];
        Debug.Log ("Number Of Items In Array :  " + Weapons_ALL.Length);

        for (int i = 0; i < Weapons_ALL.Length; i++) {
            Weapon tmpWeapon = (Weapon)InventoryManager.Instance.ItemContain.Weapons [i];
            Weapons_ALL [i] = tmpWeapon.ItemName;

            if (tmpWeapon.Quality == Quality.lowQual) {
                lows++;
            }
            if (tmpWeapon.Quality == Quality.common) {
                commons++;
            }
            if (tmpWeapon.Quality == Quality.rare) {
                rares++;
            }
            if (tmpWeapon.Quality == Quality.magnificient) {
                mags++;
            }
            if (tmpWeapon.Quality == Quality.heroic) {
                heros++;
            }

        }
        Weapons_Low_Quality = new string[lows];
        Weapons_Common_Quality = new string[commons];
        Weapons_Rare_Quality = new string[rares];
        Weapons_Mag_Quality = new string[mags];
        Weapons_Heroic_Quality = new string[heros];

        int commonW = 0;
        int lowW = 0;
        int rareW = 0;
        int magW = 0;
        int heroW = 0;
          
        for (int i = 0; i < Weapons_ALL.Length; i++) {
            Weapon tmpWeapon = (Weapon)InventoryManager.Instance.ItemContain.Weapons [i];
            if (tmpWeapon.Quality == Quality.lowQual) {
              
                Weapons_Low_Quality [lowW] = tmpWeapon.ItemName;
                lowW++;
            }
            if (tmpWeapon.Quality == Quality.common) {
                Weapons_Common_Quality [commonW] = tmpWeapon.ItemName;
                commonW++;
            }
            if (tmpWeapon.Quality == Quality.rare) {

                Weapons_Rare_Quality [rareW] = tmpWeapon.ItemName;
                rareW++;
            }
            if (tmpWeapon.Quality == Quality.magnificient) {
                Weapons_Mag_Quality [magW] = tmpWeapon.ItemName;
                magW++;
            }
            if (tmpWeapon.Quality == Quality.heroic) {
                Weapons_Heroic_Quality [heroW] = tmpWeapon.ItemName;
                heroW++;
            }
        }  
    }
2

This migt be achieved in single pass if you replace static sized arrays with dynamic lists. With list, you don’t need to pre-calculate size, just add item to the appopriate list. Also it is better to use switch instead of ifs, and you can get rid of those choices completely with array of lists for names.

3

Linq makes this a lot neater:

Weapons_Low_Quality = tmpWeapon.Where(w => w.Quality == Quality.lowQual).Select(w => w.ItemName).ToArray();

It’s also a really good intro to lambas, so it’s worthwhile to understand what’s going on.

1 Like
4

That might have been better idea right from beggining… I’ll try.

w.Quality and w.ItemName are red , unity says

The type arguments for method `System.Linq.Enumerable.Where(this System.Collections.Generic.IEnumerable, System.Func<TSource,int,bool>)’ cannot be inferred from the usage. Try specifying the type arguments explicitly

ediT : ohh , and the Weapons_Low_Quality is array of strings

5

While with linq things are much easier, it is recommended to avoid it and the lambdas in methods like Update. It generates garbage, wich may leads to unpredictable frame rate drops or even app crashes because of OutOfMemory exceptions.

2 Likes
6

I messed up a bit, should be:

InventoryManager.Instance.ItemContain.Weapons.Where(...)

and you have to:

using System.Linq;

It also looks like you’re casting the Inventory’s .Weapons to Weapon. Are they something like an Item[ ]? In that case, it’d be:

InventoryManager.Instance.ItemContain.Weapons.Cast<Weapon>().Where(...)

Essentially, each of the Linq calls does what they say; Where gives you the elements Where the condition is met, Select takes the item and Selects the property you specify, and so on.

It’s a very neat and concise syntax to filter and manage collections of stuff, with a slight performance cost. If the code runs when the player hit’s a button or opens a menu (ie. not often), that’s not something to worry about.

You’re not going to crash due to garbage. That’s not how any of this works. The performance concern is real, but probably not very relevant at this point for OP.

In general too many questions asked by relatively fresh programmers gets completely derailed into GC lessons which I don’t think helps them much?

2 Likes
7

i’ll try it all out, just to say the garbage talk is fine,. i know whats going on, and no i wont do it in update.
i try to keep a motto,. “the less update, the better” . (not really, more of a second nature)
Thanks Both For A Suggestion , definitely appreciated. :- )

8

ALTHOUGH if i get it right i wont be needing this at all,
i do this for chest random loot selection dependant on player level, chest level, and some modifier lets say.
which then selects random stuff from these arrays,.
but i could just use lambda directly to select Any random weapon with particular quality right from container.weapons.
right? : -)

9

I’d probably do something like this:

public void GetWeaponsIntoArrays()
{
    Dictionary<Quality, List<String>> weapons = new Dictionary<Quality, List<String>>()
    List<String> allWeapons = new List<String>();
   
    foreach(var quality : Enum.GetValues(Quality)
    {
        weapons.Add(quality, new List<String>());       
    }
   
    foreach(Weapon weapon : InventoryManager.Instance.ItemContain.Weapons)
    {
        weapons[weapon.Quality].Add(weapon.ItemName);
        allWeapons.Add(weapon.ItemName);
    }   
}

It probably doesn’t compile, since I’m in notepad, but that’s the general idea. If you really need arrays, List has a ToArray() method.