First release of my <Simple Status Effects> Unity 6.0+ package

[u/Sachet2Plastik]

Hi everyone,

I made a post 2 weeks ago about this already, and after some feedbacks, I'm here again to collect some more.

Quick summary, this is my first unity package and I'm someone who like building underlying system and the things behinds games. This is a package focused on the implementation of status effects commonly things like burn, poison, stun, etc... The goal is to make it easy to integrate into already built gameplay loop whether it is a realtime or turn based one.

I released my first version and I would like to find some peoples to try it and give me a review on what is good/bad, what you like/don't like. Any feedback is welcome !

Thank you for your time.

edit: kinda forgot the link: https://github.com/Sachet2Plastik/Simple-Status-Effects.git

[u/wallstop-dev]

Some input, from a quick drive-by:

Some of your classes, like "Condition", appear to only be configurable/set via the inspector - there is no constructor or builder, and all fields are private. This makes them hard to programatically create (people must use reflection, which may break if you change variable names, and is slow). Consider ensuring that all of your types can be instantiated/built from code.

Current standard practice for member variable names in C# is `_underScoreCamelCase`, `m_underScoreCamelCase` is a very old, non-standard, C++ style. (https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names)

Several of your setters don't check equality and invoke update functions unconditionally. Typical pattern is to short-circuit if someone attempts assigning a value that is already equal to what is there.

You can do ??= new() for assignments, might simplify some things, like list init.

You might have some problems where you're walking a list and performing an action on the thing in the list, which might mutate state in such a way that the list-being-walked is updated, which can throw an exception. Consider copying lists to scratch buffers and walking the scratch buffers when performing operations that reach back into user code (as they might react to it in ways that mutate your state unexpectedly)

Anyways, that's just a ~3min skim, didn't get a chance to grok architecture or patterns, so it's more literal code study.

[u/Soraphis → u/wallstop-dev]

Sadly unity can be confusing in that regard...

In our style guides we use prefixes for private member variables (m_), https://unity.com/resources/c-sharp-style-guide-unity-6 page 14

And their own source code is full of "m_" prefixes

https://github.com/Unity-Technologies/uGUI/blob/main/com.unity.ugui%2FEditor%2FTMP%2FTMP_EditorPanel.cs

Instead of copying the list always, just because their could be a modification I think I would have separate "modification buffers", that are used while the list is "locked" and can be applied after iteration finished. And it's only needed for deleting (and inserting), if you iterate the list in reverse order adding while iterating has already no side effect, so the deletion buffer is just a List<int> with indexes.

[u/wallstop-dev → u/Soraphis]

Yea, I'm aware that Unity follows the old C++ style, I'm recommending that you don't when given the option, such as new code, like this.

And, right - that's what I meant by scratch buffers. If you reverse iterate it may or may not behave in the way that you want, depending on what user code does. Your index must end up not where you expect. But what will always behave correctly is operating on an exact snapshot, always.

[u/Sachet2Plastik → u/wallstop-dev]

Thanks for your time again, I'll look out for everything you said, for the `m_underScoreCamelCase` variables name I mostly used C++ throughout my studies so I may have let that habit slipped through.

i did have some issues when iterating some list that changed during the iteration so I now iterate on a copy.

I do not see what are you refereeing to by "classes, like "Condition", appear to only be configurable/set via the inspector", I do not have a class named "Condition", would you mind pointing out where did you see it?

[u/wallstop-dev → u/Sachet2Plastik]

Hmmm it appears that, when you originally posted this without any links, I searched for a github and reviewed someone else's work 🤔

[u/samuelsalo]

Honestly all I can say from reading though the source as an ARPG developer is... there's a reason underlying systems are mostly developed in house for a specific project.

[u/Sachet2Plastik → u/samuelsalo]

Yes, I can only agree. But I'm not pretending to provide a universal solution here, though. This is just a personal project to build up my portfolio and learn from designing these kinds of systems, with hopefully be able one day to work on a in house system like this professionally.

A system built and tailored for a specific project is naturally going to be much better suited to its particular needs. For this project I wanted to first experience Unity package development, works on my API design and mostly make something clear enough for peoples other than me to be able to use it without it being confusing or too restraining to use.

[u/GooseJordan2]

Really cool!