I mean… your first option isn’t wrong necessarily.
It’s certainly not “OOP”. But OOP isn’t necessarily “right”.
My only issue personally with your first option is that as you add more and more card types is going to get bigger and bigger and bigger. Just thousands of lines of code. Which I personally don’t like having to deal with.
I’d rather several individual small classes. Sure it’s more script files, but once the game is compiled all of those things get turned into a single dll. So it’s not “more bloat”.
But that’s all personal preference really, and on a forum like this you’ll get all sorts of opinions in regards to that.
…
With that said some options you could go with are:
1) The route you already described with the interface and different SO’s.
2) User the new/ish ‘SerializeReference’ attribute. Create a “ICardEffect” interface. Then each card effect implements that interface. Then on your scriptable object you’ll have a field/var that is that type like so:
public interface ICardEffect
{
void PerformEffect();
}
public class Card : ScriptableObject
{
[SerializeReference]
public ICardEffect Effect;
public void PerformEffect()
{
Effect?.PerformEffect();
}
}
public class CardEffectTypeA : ICardEffect
{
public void PerformEffect()
{
//your effect's logic here
}
}
This route will require creating some custom editor that allows you to change which “effect” it uses. Something like this:
[CustomEditor(typeof(Card))]
public class CardInspector : Editor
{
private static List<System.Type> _availableTypes;
private static GUIContent[] _dropDownOptions;
static CardInspector()
{
_availableTypes = GetTypes((t) => typeof(ICardEffect).IsAssignableFrom(t) && t.IsClass && !t.IsAbstract).ToList();
_dropDownOptions = _availableTypes.Select(t => new GUIContent(t.Name)).ToArray();
}
public override void OnInspectorGUI()
{
this.serializedObject.Update();
EditorGUILayout.PropertyField(this.serializedObject.FindProperty("m_Script"));
var prop = this.serializedObject.FindProperty("Effect");
if((prop.isExpanded = EditorGUILayout.Foldout(prop.isExpanded, prop.displayName)))
{
EditorGUI.indentLevel++;
int index = GetIndexOf(_availableTypes, (t) => GetUnityFormattedFullTypeName(t) == prop.managedReferenceFullTypename);
EditorGUI.BeginChangeCheck();
index = EditorGUILayout.Popup(index, _dropDownOptions);
if (EditorGUI.EndChangeCheck())
{
var tp = index >= 0 ? _availableTypes[index] : null;
var obj = tp != null ? System.Activator.CreateInstance(tp) : null;
prop.managedReferenceValue = obj;
this.serializedObject.ApplyModifiedProperties();
}
if(prop.hasChildren)
{
while(prop.NextVisible(true))
{
EditorGUILayout.PropertyField(prop);
}
}
EditorGUI.indentLevel--;
}
this.serializedObject.ApplyModifiedProperties();
}
public static IEnumerable<System.Type> GetTypes(System.Func<System.Type, bool> predicate)
{
foreach (var assemb in System.AppDomain.CurrentDomain.GetAssemblies())
{
foreach (var tp in assemb.GetTypes())
{
if (predicate == null || predicate(tp)) yield return tp;
}
}
}
public static int GetIndexOf(IList<System.Type> lst, System.Func<System.Type, bool> predicate)
{
for(int i = 0; i < lst.Count; i++)
{
if (predicate(lst[i])) return i;
}
return -1;
}
public static string GetUnityFormattedFullTypeName(System.Type tp)
{
return string.Format("{0} {1}", tp.Assembly.GetName().Name, tp.FullName);
}
}
3) Forego the ScriptableObject route and instead use prefabs (this is actually how we used to get SO like behaviour back in the day before SO’s existed… prefabs that had no graphical aspects to them). Then you attach the card/effect components however you want. May it be a ‘Card’ class and you inherit from it for each card type. Or a ‘Card’ class, a ICardEffect interface, and a CardEffectXXX component for each type.
This allows even more robust compositing of behaviours via the existing gameobject component options.
4) You could go completely non-traditional routes. Like using an enum instead of the type name lookup from your existing design. Or a factory pattern that built the effect object from the enum/type name so you weren’t using the switch statement every time you called. Or a factory that just returned a delegate instead.
…
How would I do it!?
Ehhhhhhh… that’s hard to say since I don’t k now exactly what future expansion you may do to this.
Knowing me though I’d likely go the route of Option 2 with some tweaking. I’d have a GameObject ref on the Card SO to reference that GUI prefab that is instantiated and shown on screen. I’d have a more robust inpsector than what I showed as an example. I’d create a factory as well to create cards on the fly at runtime.
But yeah… that’s likely what I’d do today.
Back in the day… before SerializeReference existed. I would have gone sort of the option 3 route. I actually did something like this in one of our games. Where I had a ScriptableObject that represented the entire thing, and then GameObject prefabs for the GUI aspect of it. But if I needed some composited behaviour like you are looking for I attached it to the GameObject prefab and pulled it off of that.
I did this for inventory items where some items had custom behaviour. And I also did this with my footstep sound effects. And a few other things.
…
With all that said.
If you got something to work right now. Work on getting more of your game to work.
Don’t refactor your current system until it becomes unbearable to maintain in the current state.
You know…
Don’t fix what ain’t broke.
