Please can someone help me debug this code

Unity Engine
1

Hi all, I’ve been staring at this piece of code for the last 2 hours now trying to debug it, and I cant see the woods for the trees anymore. I’m going out of my mind so please can someone help me!!

Basically, I have two methods to switch weapons:

    IEnumerator NextWeapon()
    {
        if(!changingWeapon)
        {
            changingWeapon = true;

            int weaponToChangeTo = _playerGunManager.currentWeapon += 1;

            if (weaponToChangeTo == _playerGunManager.weapons.Length)
            {
                weaponToChangeTo = 0;
            }

            if (_playerGunManager.weapons[weaponToChangeTo].owned == false)
            {
                while (_playerGunManager.weapons[weaponToChangeTo].owned == false)
                {
                    weaponToChangeTo += 1;

                    if (weaponToChangeTo == _playerGunManager.weapons.Length)
                    {
                        weaponToChangeTo = 0;
                    }
                    yield return null;
                }
            }
            _playerGunManager.ChangeWeapon(weaponToChangeTo);

            yield return new WaitForSeconds(0.2f);
            changingWeapon = false;
        }

        yield return null;
    }

    IEnumerator PreviousWeapon()
    {
        if(!changingWeapon)
        {
            changingWeapon = true;

            int weaponToChangeTo = _playerGunManager.currentWeapon -= 1;

            if (weaponToChangeTo < 0)
            {
                weaponToChangeTo = _playerGunManager.weapons.Length - 1;
            }

            if (_playerGunManager.weapons[weaponToChangeTo].owned == false)
            {
                while (_playerGunManager.weapons[weaponToChangeTo].owned == false)
                {
                    weaponToChangeTo -= 1;
                    if (weaponToChangeTo < 0)
                    {
                        weaponToChangeTo = _playerGunManager.weapons.Length - 1;
                    }
                    yield return null;
                }
            }

            _playerGunManager.ChangeWeapon(weaponToChangeTo);
            yield return new WaitForSeconds(0.2f);
            changingWeapon = false;
        }

        yield return null;
    }

The methods are called using the following:

        if(!_playerGunManager.currentlyReloading)
        {
            if(_player.GetButtonDown("Next_Weapon"))
            {
                StartCoroutine(NextWeapon());
            }

            if (_player.GetButtonDown("Previous_Weapon"))
            {
                StartCoroutine(PreviousWeapon());
            }
        }

So… easy peasy so far.

But, the problem I have is that my methods are basically trying to iterate through an array of weapons, find ones that the player owns and then switch to that weapon, using an integer array value.

The thing is, when I call either function, the variable below is changed immediately

_playerGunManager.currentWeapon

Now at the end of the methods, they are calling another method that sets the current weapon on the playergunmanager:

    public void ChangeWeapon(int weap)
    {
        aSource.playerAudio.PlayOneShot(weaponSwapSound, 0.2f);
        currentWeapon = weap;
        SetWeaponOptions();
        _uiBulletManager.RefreshBullets();
        _weaponIconManager.SetIcons();
    }

Now this is the problem I cant figure out, why the hell is that variable changing immediately when its not changed until the end of the function.

At the game start, the players weapon variable is set at 0. As soon as you press the button for next or previous weapon, it goes to either -1 or 1, until and then changes to the correct value after iterating through the array in the IEnumerator, but nothing is changing it

I am problably being beyond stupid here, but its late and I cant go to bed until this works !

Please help! :slight_smile:

2

err, if the next weapon is owned, there’s no yield between starting NextWeapon and calling ChangeWeapon. You probably want:

_playerGunManager.ChangeWeapon(weaponToChangeTo);
yield return new WaitForSeconds(0.2f);
changingWeapon = false;

to be:

yield return new WaitForSeconds(0.2f);
_playerGunManager.ChangeWeapon(weaponToChangeTo);
changingWeapon = false;

Your code’s doing something strange. Why’s it yielding between each weapon? If you own weapon 1 and 5, do you want there to be 4 frames between pressing next weapon and the weapon switching starting?

You also don’t need to yield return null at the end of your Coroutine. That does nothing.

Finally, note that if this code’s called if the player has no weapon, it’ll loop forever.

3

Thanks @Baste You are indeed right, Ive removed the yields…

I’ve been butchering the code over the last few hours, so It definately needs cleaning up, but its still causing errors by immediately changing the weapon int. Its like ChangeWeapon() is being called immediately and setting the value to the initial value set in the routine.

But, looking at the code, I cant see how thats even remotely possible.

and its not being changed anywhere else.

I have narrowed it down to the while loop that is looking for the next weapon owned, if I remove that, I can change weapons fine without error… I just cannot see what the problem is

4

Ok, so I can see where the change is happening, I have changed the initial value to -5 just to test as seen below:

    IEnumerator PreviousWeapon()
    {
        if(!changingWeapon)
        {
            changingWeapon = true;

            int weaponToChangeTo = _playerGunManager.currentWeapon -= 5;

            if (weaponToChangeTo < 0)
            {
                weaponToChangeTo = _playerGunManager.weapons.Length - 1;
            }

            if (_playerGunManager.weapons[weaponToChangeTo].owned == false)
            {
                while (_playerGunManager.weapons[weaponToChangeTo].owned == false)
                {
                    weaponToChangeTo -= 1;
                    if (weaponToChangeTo < 0)
                    {
                        weaponToChangeTo = _playerGunManager.weapons.Length - 1;
                    }
                }
            }
            yield return new WaitForSeconds(0.2f);
            _playerGunManager.ChangeWeapon(weaponToChangeTo);
            changingWeapon = false;
        }
    }

Now when I press the previous button, I see the value immediately change to -5 on the gun manager, which means that first line int weaponToChangeTo = _playerGunManager.currentWeapon -= 5; is somehow chaning the value on the gun script before the _playerGunManager.ChangeWeapon(weaponToChangeTo); is being called…

I am so confused…

5

Oh, right, didn’t spot that the first time around.

Yeah, += and -= changes the value. These three are equivalent:

//v1:
int weaponToChangeTo = _playerGunManager.currentWeapon -= 1;

//v2:
_playerGunManager.currentWeapon -= 1;
int weaponToChangeTo = _playerGunManager.currentWeapon;

//v3:
_playerGunManager.currentWeapon = _playerGunManager.currentWeapon - 1;
int weaponToChangeTo = _playerGunManager.currentWeapon;

C# returns a value when you set a value. It’s a bit weird, but it allows things like:

a = b = c = d = 5

You want to just do this instead:

int weaponToChangeTo = _playerGunManager.currentWeapon - 1;
6

@Baste oh good god, how did I not spot that!

Man, thanks so much for helping out, I was going out of my mind. I’ve been coding for about 14 hours straight, and on some really complex stuff, so its a real pain when you get bogged down on something so simple!

Thanks again :wink: