Wondering what's the best way to structure the code for items with effects?

Another aspect is also if you instance the SO for mutating its data. Alot of references will create cache misses, allocation etc.

Disagree with the guy. Polymorphism is about substitution. If you can swap a derived type out with another and not have to care about its implementation, that’s polymorphism done correctly.

He doesn’t even explain his point properly. He just adds another layer of indirection, which doesn’t really prove his point either. Doing what he suggests just ends with bigger inheritance hierarchies, which is what you don’t want in game dev.

Abstract as much as you need to get it to work, and give you the degree of scalability.

Then you havent understood what he is saying. Using polymorhism when you only have variance in data is wrong. Variance in data is handled by the information sent to the implementation.

Then if you go by his definition we shouldn’t have MonoBehaviour, as that’s both variance in data and behaviour.

It’s a stupid dogmatic stance that doesn’t make game dev any easier.

We’re here to make games and not pass some stuffy professor’s tests.

Thats not what he is saying. You should use polymorhism when you have variance in behaviour.

I think this is beside the point of OP’s thread.

My method was variance in behaviour anyway. Aka the strategy pattern. Aka basic C#.

What you said here intrigued me. I think the discussion between polymophism of subtypes might be a bit too advance for me, but how would you “replace” the use of enum with SerializeReference. Just want to understand a bit more as it seems useful for me in the future.

Right now the Scriptable Object that holds the IPerkEffect:

[CreateAssetMenu(fileName = "Perk", menuName = "New Perk")]
public class Perk : ScriptableObject
{
    [SerializeField] internal TRIGGERFREQUENCY Frequency;
    [SerializeReference, SubclassSelector]
    private IPerkEffect _perkEffect;

    public float ApplyEffect(Body body, float damage, int index)
    {
        return _perkEffect.ApplyEffect(body, damage, index);
    }
}

Now I use the Frequency to distinguish between when to call the ApplyEffect function in a different script.

public enum TRIGGERFREQUENCY
{
    StartOfBattle,
    BeforeAttack,
    AfterAttack,
    BeforeDamage,
    AfterDamage,
    EndOfBattle,
    Constant
}

i.e.

        for (int i = 0; i < _body.NormalPerks.Count; i++)
        {
            if (_body.NormalPerks[i].Frequency == frequency)
            {
                alteredDamage += _body.NormalPerks[i].ApplyEffect(_body, alteredDamage, i);
            }
        }

How would you replace this function with SerializeReference, or is this use of enum valid? Thanks in advance.

Thinking about it, scriptable objects would probably be better.

You make Frequency a scriptable object class, and simply compare the equality like you would with the enum. Upside is, you have more room to add/remove/reorder your frequencies without necessarily having to change any code. The serialisation is more stable, too.

I’ve been thinking about enums a lot lately and it LOOKS like you’re using them here and I don’t think I saw anyone bring this up, and if you already know this appologies, but if you plan on adding to an enum list in the future you have to be very careful, because it will garble all your existing data you punched into your fields in your scriptable game objects and prefabs.

Here’s a quick video i made the other day showing this issue (sliding enum values).

You can avoid this if when creating your enums you assign your strings a number. So long as you never create a duplicate index #, and you don’t ever change the value of what each enum is assigned, you will never lose your reference to the correct string or index:

so for example:

public enum Bonus
{
    None = 0,
    AfterShocks = 1,
    Anger = 2,
    Angry = 99,  //Though in alphabetical order in the IDE, it will be displayed at the bottom of the list
    Behemoth = 3,
}

But this can cause problems if you later want to insert a new value between these as I show above with “angry” (to keep it alphabetical). So it might make sense to do increments of 10 or even 1000 so you have gaps if you want to add new values later in-between values so you don’t garble the arrangement of things.

public enum Bonus
{
    None = 0,
    AfterGlow = 5,
    AfterGrow = 7,   //Here is an example index you may want to add later to keep things alphabetical
    AfterShocks = 10,
    Anger = 20,
    Behemoth = 30,
}

It’s not a perfect solution, but it’s better than ignoring the problem altogether. Lets say you worked and added and added, or maybe just make a human error, you could run out of room or throw off your values simply by inputting the wrong value somewhere. Unless you catch this right away this can become a nightmare and all your internal game logic will break from a single # in the wrong place.

public enum Bonus
{
    None = 0,
    AfterGlow = 5,
    AfterGlowness =7,
    AfterGlownesss =8,
    AfterGlownessss = 9,
// NOW YOU ARE OUT OF ROOM
    AfterShocks = 10,
    Anger = 20,
    Behemoth = 30,
}

Hope this is relevant to the thread, but I’ve been in the thick of it with enums lately and wanted to be sure you were aware of their kinks so these issues don’t jump up later and kick your butt.

It’s gotten to the point I don’t rely on enums with drop down lists any more. I name the Scriptable object the correct string that is mirrored in the enum and then use the replace string function to cull the " (Clone)" or other syntax unity adds to get it down to its clean value and use this string to check if it’s the correct enum from the name of the scriptable object.

Essentially at this point all i’m using the enum for is a rigid data structure that keeps all the potential names of skills, and makes sure the naming conventions are enforced. Nowhere in my project do i rely on the enum dropdown because over time it just becomes a massive headache, I always derive from the object name which will never be garbled or moved around or put out of alphabetical order. I can keep the enum list alphabetical and I don’t have to worry about their indexes shifting as I never use the list, I extract the string from the Scriptable Game Object name, and that always matches the enum name of said bonus or attack mod or whatever.

    public static Bonus GetBonusEnum(string name)
    {
        var tempName = name;
        tempName = tempName.Replace(" (Bonus)", "");
        tempName = tempName.Replace("(Bonus)", "");
        tempName = tempName.Replace("(Clone)", "");

        foreach (var enumName in Enum.GetNames(typeof(Bonus)))
        {
            Bonus checkedEnum = (Bonus)Enum.Parse(typeof(Bonus), enumName);
            if (tempName == checkedEnum.ToString())
            {
                return checkedEnum;
            }
        }
        print("enum Not Found for " + tempName);
        return Bonus.None;
    }

var bonusEnum = GetBonusEnum(bonus.ToString());

I should end this by saying so long as you are ALWAYS adding to the end or REPLACING a dead enum index you no longer use (you have to be careful not to delete stuff, ALWAYS replace or it will garble the order) you should be fine.

If you’re positive you are ok with that attack order enum and you only want to add to the end of it and being in alphabetical order isn’t a big deal, you can roll with it.

For all I know there are better and cleaner ways of doing this, but this is the system I ended up throwing together literally just yesterday. I’m so close to throwing the enum list away altogether, but at this point i’ve got it all set up to work and i figure if it’s not broke, don’t fix it.

If I could do this all over again, i’d probably just populate a dynamic list with all Scriptable Objects and run through it checking the Scriptable Object of the bonus name as the means of identifying the proper bonus and avoid the bloat and madness of enums altogether.

The enum list DOES allow me to do one handy thing in code though. I can easily access the scriptable object reference from anywhere using the below function:

    public Bonus GetBonusFromEnum(BonusEnum bonusEnum)
    {
        if (bonusEnum == afterShocks.bonusEnum) return afterShocks; //the bonus enum is derived from the above GetBonusEnum function so it's water tight
        if (bonusEnum == anger.bonusEnum) return anger;
        if (bonusEnum == berserker.bonusEnum) return berserker;
        if (bonusEnum == behemoth.bonusEnum) return behemoth;
        if (bonusEnum == blackJack.bonusEnum) return blackJack;
        if (bonusEnum == block.bonusEnum) return block;
        if (bonusEnum == bloodvision.bonusEnum) return bloodvision;
        if (bonusEnum == blowback.bonusEnum) return blowback;
        if (bonusEnum == bullys.bonusEnum) return bullys;
    }

This returns the proper Scriptable Object bonus based on the enum list, so now we can do things like…

AddBonusLvl(player.GetBonusFromEnum(BonusEnums.Heart)); //No reference to SO required! No chance of misspelling with clunky strings because you are pulling directly from the enum list

Information overload I know, but I now use layouts and put all my bonuses in one folder and I end up with this layout that I can access at any time. If you’re not familiar with layouts they are VERY useful and can expand your use of Unity in powerful ways that don’t require code.


(ignore the compile warning’s i’m posting as I wait on compile times as I literally fix up my project to not rely on enums so much :rofl:)

So now because of the above layout I have all my bonuses laid out right there in a list kind of fashion and I can drag and drop them into the scriptable object reference, and any item equipping this bonus immediately knows what bonus to assign its increased equipment levels to because that bonus is linked right in the script. And again, if you don’t have the proper scriptable object reference like let’s say you have a multiplayer game and multiple entities with multiple lists of unique bonuses, you could cycle through those lists and compare the scriptable object names against each other to find the one you need to modify.

Its perfectly fine useage of a enum. Though make it C# complient name and why internal? make it private. You shouldnt mutate it from outside the class instance. And if you do it should be done through a setter or other method not directly on the field.

edit: ALso I always use explicit numbered enums. Especially when searlizing.

I mean if you want these frequencies to be presented to the player in some form, eg: display name, icon, etc, then scriptable object is the obvious choice.

There’s not really any real reason for it to be an enum.

Not for all use cases, you might want to easy point it out from code. Real world example from our game

public override void PopulateMapIcons(Action<Transform, Icons> addIcon)
{
    var team = Teams.GetTeamForPlayer(Networking.PrimarySocket.Me.NetworkId);

    foreach (var player in team.Players)
    {
        if (player == Networking.PrimarySocket.Me.NetworkId) continue;
        if (!AvatarController.Avatars.ContainsKey(player) || AvatarController.Avatars[player].IsDead()) continue;

        var playerAvatar = AvatarController.Avatars[player];
        addIcon(playerAvatar.transform, Icons.Arrow);
    }
}

Ew. Why would you do that when the data can just be in the frequency objects itself?

It’s amazing how people justify enums with more and more bad code.

Im giving you example when its useful. Can also be built upon. Though gamemode like this being inheritance instead of composiition can be questioned

public override void PopulateMapIcons(Action<Transform, Icons> addIcon)
{
    base.PopulateMapIcons(addIcon);

    if (PlayerIsOnAttackingTeam(Networking.PrimarySocket.Me.NetworkId) && BombCase.Instance)
    {
        addIcon(BombCase.Instance.transform, Icons.Bomb);
    }

    foreach (var placement in settings.PossiblePlacements)
        addIcon(placement.transform, Icons.Explosion);
}

Are we talking about networking though? No we’re not.

I dont see what networking have todo with anyting.
You brought up labels and icons for a use case he doesnt need, not me :smiley:

I just built upon that. Even when you want addional data it can make sense to have it as an enum. WIth my example of icons that you want to reference from code easily without needing to resolve arbitrary assets

But netowkring is another good example. A enum can be a byte. Much cheaper to send over the network than a guid. Not sure you can resolve the guid in runtime either to a instance of a SO

If you want to reference them from code then just make a global config (singleton) scriptable object and pump in properties for all your icons.

I dont see that being more maintainable you still need to mutate the code to add new Properties.
I’m a big advocate of open closed principle. But adding a new entry to a enum is hardly any different than adding a proprety to a SO.

Great discusion though. Everything brough up is food for thought.

Always look at all the tools in the toolbox boys and girls.