Timer Crash

[Karmageddon]

I am getting a crash issue from an item that has a timer in it? What the item does is add stat buffs when 2 players wearing linked items are close to each other. What the timer does is actively checks to make sure the players are still within range of each other. This is the code for the timer itself.

private class WeddingRingTimer : Timer
        {
            private readonly WeddingRing _Ring;

            public WeddingRingTimer(WeddingRing ring)
                : base(TimeSpan.Zero, TimeSpan.FromSeconds(30))
            {
                _Ring = ring;
            }

            protected override void OnTick()
            {
                if (_Ring != null)
                {
                   (line 138) if (_Ring.Link.Owner.InRange(_Ring.Link.Owner.Location, 18) && _Ring.Link.Owner.Alive && _Ring.Link.Owner != null && _Ring.Link.Owner.NetState != null && _Ring.Link != null)
                    {
                        if (_Ring.Owner.FindItemOnLayer(Layer.Ring) is WeddingRing)
                        {
                            if (_Ring.Attributes.AttackChance == 0)
                            {
                                _Ring.Attributes.AttackChance = Utility.RandomMinMax(10, 15);
                                _Ring.Attributes.RegenHits = Utility.RandomMinMax(5, 10);
                                _Ring.Attributes.BonusHits = Utility.RandomMinMax(5, 10);
                                _Ring.Attributes.Luck = Utility.RandomMinMax(90, 140);
                                _Ring.InvalidateProperties();
                                _Ring.Owner.SendMessage("Your bond has grown stronger!");
                            }
                        }
                    }
                    else
                    {
                        if (_Ring.Attributes.AttackChance > 8)
                        {
                            _Ring.Owner.SendMessage("Your bond has grown weaker!");
                            _Ring.Attributes.AttackChance = 8;
                            _Ring.Attributes.RegenHits = 4;
                            _Ring.Attributes.BonusHits = 3;
                            _Ring.Attributes.Luck = 50;
                            _Ring.InvalidateProperties();

                        }
                    }
                }
            }
        }

This is the crash report, the line indicated is line 138.

System.NullReferenceException: Object reference not set to an instance of an object.
at Server.Items.WeddingRing.WeddingRingTimer.OnTick() in c:\DarkShadow\Scripts\Custom\WeddingRing.cs:line 138
at Server.Timer.Slice() in C:\ServUO-master (3)\Server\Timer.cs:line 393
at Server.Core.Main(String[] args) in C:\ServUO-master (3)\Server\Main.cs:line 673

 

[PyrO]

the order inside the if is an issue, first you try to access Owner, but later on you check if it is null.

I would also suggest to move the other checks upwards as well, makes it cleaner.

        private class WeddingRingTimer : Timer
        {
            private readonly WeddingRing _Ring;

            public WeddingRingTimer(WeddingRing ring)
                : base(TimeSpan.Zero, TimeSpan.FromSeconds(30))
            {
                _Ring = ring;
            }

            protected override void OnTick()
            {
                if (_Ring == null || _Ring.Link == null || _Ring.Link.Owner == null || _Ring.Link.Owner.NetState == null)
                    return;
                
                if (_Ring.Link.Owner.Alive && _Ring.Link.Owner.InRange(_Ring.Link.Owner.Location, 18))
                {
                    if (_Ring.Owner.FindItemOnLayer(Layer.Ring) is WeddingRing)
                    {
                        if (_Ring.Attributes.AttackChance == 0)
                        {
                            _Ring.Attributes.AttackChance = Utility.RandomMinMax(10, 15);
                            _Ring.Attributes.RegenHits = Utility.RandomMinMax(5, 10);
                            _Ring.Attributes.BonusHits = Utility.RandomMinMax(5, 10);
                            _Ring.Attributes.Luck = Utility.RandomMinMax(90, 140);
                            _Ring.InvalidateProperties();
                            _Ring.Owner.SendMessage("Your bond has grown stronger!");
                        }
                    }
                }
                else
                }
                    if (_Ring.Attributes.AttackChance > 8)
                    {
                        _Ring.Owner.SendMessage("Your bond has grown weaker!");
                        _Ring.Attributes.AttackChance = 8;
                        _Ring.Attributes.RegenHits = 4;
                        _Ring.Attributes.BonusHits = 3;
                        _Ring.Attributes.Luck = 50;
                        _Ring.InvalidateProperties();
                    }
                }
            }
        }

 

[Karmageddon]

Ok didn't realize that the order mattered, but the way you explain it makes sense.

 

[Anon the Felon]

Code is read progressively. A line cannot be read until the previous is.

Sequencing is super important.

 

[Fireball]

I think most of the problems I encounter are null related Like Karmageddon says; always null check first.

Nice idea. Well done.

 

[Fireball]

I think most of the problems I encounter are null related Like Karmageddon says; always null check first.

Nice idea. Well done.

Argh. Can't edit!! Can't edit!!!! I meant PyrO