[SOLVED] Is there anything wrong with what I'm doing? (long post)

Unity Engine
1

I’m making a “condition system” that I use in my targeting system to get an appropriate target, in Ability to specify the conditions under which the ability can be cast, and in my triggering system to check if some conditions are met before executing some actions.

For example this is a condition:

2

        [Serializable]
        public abstract class BaseCondition
        {
            [SerializeField] private readonly Candidate _candidate;

            public abstract bool Check(params DynamicCandidate[] candidates);
        }

        [Serializable]
        public class Condition : BaseCondition
        {
            [SerializeField] private Candidate _candidate;
            [SerializeField] private Game.Condition _condition;

            public override bool Check(params DynamicCandidate[] candidates)
            {
                return _condition.Check(_candidate.GetValue(candidates));
            }
        }

I have a bunch of real conditions (Game.Condition) such as “HasItem”, “IsInState”, “HasStatusEffect”, etc. that derive from condition. I also have some And/Or conditions as well.
And in the _candidate field you can put one of those inside:

1

 public abstract class Candidate
        {
            public abstract Game.Candidate GetValue(params DynamicCandidate[] candidates);
        }

        [Serializable]
        public class DynamicCandidate : Candidate
        {
            [SerializeField] private readonly RoleType _role;
            private readonly GameObject _value;

            public DynamicCandidate(GameObject value, RoleType role)
            {
                _value = value;
                _role = role;
            }

            public override Game.Candidate GetValue(params DynamicCandidate[] candidates)
            {
                for (int i = 0; i < candidates.Length; i++)
                {
                    if (candidates[i]._role == _role) return new ActorCandidate(candidates[i]._value);
                }

                throw new NullReferenceException("RoleType");
            }
        }

        [Serializable]
        public class StaticCandidate : Candidate
        {
            [SerializeField] private readonly GameObject _value;

            public override Game.Candidate GetValue(params DynamicCandidate[] candidates)
            {
                return new ActorCandidate(_value);
            }
        }

This lets you use either a GameObject that already exists in the scene or use an object that was involved (targeter, target, caster, etc.).

This gives great flexibility in how spells select targets: you could make a spell check the caster’s health. If it’s over 100, it heals an ally, if it’s not, it hurts an enemy. You could make an ability only castable when the caster has a certain item. Just some examples. In the future I’d like to make it even more flexible and have conditions that can check stuff about an ability / status effect.
The thing is I don’t know if this is the best way to do this though as I haven’t see any such system before so I’m a bit curious as to what everyone does?

2

Anyone?

3

Does the solution give you the flexibility you need and solve the initial problem you were trying to solve?

Yes:

Then it’s a good way to do it.

No:

Then it’s not.

:).

Without having a full grasp on the scale of the problem you were solving or it’s intent, the question is completely subjective.

4

Why are your BaseCondition and Candidate classes abstract instead of interfaces? You’re overriding the _candidate in BaseCondition anyway, and IMO it would be “neater” to use interfaces.

5

Yea it’s still WIP. Before I go any further I wanted to hear what people thought of such a system.
I tried googling similar systems but I couldn’t find anything so I wanted to know what people do.

Yes, but I’m wondering if it’s the best way to go about it. I could write spaghetti code and it would work but maybe it’s not the best thing to do. :stuck_out_tongue:
Basically, I want to have some spells that automatically target the closest object, some that target the actor at the point clicked but only if it matches certain conditions (like if it’s an ally/enemy).
I also use those “serializable conditions” to choose under what condition an ability can be used. Maybe it can be used when stunned, or maybe it can’t. Maybe you need to have a certain item. But as the system becomes more flexible I’m not sure if it will be the best option. Right now I’m making conditions to test stuff about an object or the game. But then what if I want to test stuff about players or buffs, etc.

6

To me, it is a little hard to read because it is quite generic; it’s not clear what is being checked. That’s normal - generic, theoretical models will always be more difficult to grasp then straight-forward, concrete examples.

However, that doesn’t mean it’s a bad system. Your code adds an additional level of indirection to the problem of “is this a valid target”. In computer science, indirection is very useful because it can reduce code duplication and improve flexibility at the same time. However, indirection is not free; it carries a “weight” of complexity and can make maintenance more difficult as someone needs to read and understand it.

If your game design is complex enough to warrant the weight of such a flexible system then it is a good design. If your game/ability system is not particularly complex, then the design is over-engineered. Unfortunately, what counts as “complex enough” is a judgment call and not everyone will agree.

Personally, I think if you are building up many different targetable spells and want a quick way to prototype from a simple interface, say a spreadsheet describing the rules, this system might come in handy.

1 Like