Resolved calling "base base" implementation in inheritance

Hey,

I am sorry for asking two questions in the span of only a few days but this is something I have been thinking about for many projects and wanted to ask for the "best practice" approach. I will simply use the context of my latest project as an example.

I have a abstract class Interactable.

Code (CSharp):

1. public abstract class Interactable : MonoBehaviour
2. {
3.     public abstract bool OnInteract(PlayerController playerInteracting);
4.     public abstract bool CanInteract(PlayerController playerInteracting);
5. }

From this class I derrive another abstract class Unlockable.

Code (CSharp):

1. public abstract class Unlockable : Interactable
2. {
3.     public virtual void OnUnlock(PlayerController playerInteracting)
4.     { // Base Implementation}
5.  
6.     public override bool CanInteract(PlayerController playerInteracting)
7.     { // Create a new Base Implementation by overriding here}
8. }

From there I derrive a class called Display.

Code (CSharp):

1. public class Display : Unlockable
2. {
3.     public override bool OnInteract(PlayerController playerInteracting)
4.     { // Defines Interaction Logic }
5.  
6.     // no changes to CanInteract
7.     public override bool CanInteract(PlayerController playerInteracting)
8.     {
9.         return base.CanInteract(playerInteracting);
10.     }
11.  
12.     public override void OnUnlock(PlayerController playerInteracting)
13.     {
14.         base.OnUnlock(playerInteracting);
15.        
16.         // play specific display unlock animation here
17.     }
18. }

From Display I once again derrive a class called Seedbox.

Code (CSharp):

1. public class Seedbox : Display
2. {
3.     public override bool OnInteract(PlayerController playerInteracting)
4.     { // Defines more specific Interaction Logic }
5.  
6.     public override bool CanInteract(PlayerController playerInteracting)
7.     { // Defines more specific Interaction Contraints}
8.  
9.     public override void OnUnlock(PlayerController playerInteracting)
10.     {
11.         /*
12. Here I need a good way to call the OnUnlock Implementation from the Unlockable class because the Display class plays its specifc unlock animation in the override
13.         */
14.         base.OnUnlock(playerInteracting);
15.  
16.         // other specific unlock animation
17.     }
18. }

(There are many other classes in the hierarchy and since there are many common implementations I ditched multiple interfaces a while ago)

How would I go about calling the base Implementation of OnUnlock from the Unlockable class in the Seedbox class while ignoring the changes Display made. Am I just going about the problem the wrong way?

Should I actually use interfaces with a base implementation instead? Ive always been discouraged to create base implementations in interfaces and to rather use inheritance in such cases instead but I dont see a way to make this work here.

Best regards!

 

Favoring composition can facilitate this better.

You can't. You've inherited the implementation of Display so thusly you get the implementation of Display. The best option you have is to created a protected method called something like 'OnUnlockDisplayImp' and 'OnUnlockUnlockableImp' in their respective classes and then you only call the ones you want in your override without calling 'base'. Though really all you're doing at this point is a really really really round about poor man's composition.

Should you? Ehhh.

Would I? Yes.

I would use interfaces and if I have any common implementations that are complex I'd abstract them out into compositable objects that can be consumed by my implementations.

...

I have to ask... why is a Seedbox inheriting from a Display?

Is a Seedbox a display?

or... using composition terms...

Does a Seedbox HAVE a display?

This "have a" approach is composition. Think like how a Rigidbody isn't a BoxCollider... a Rigidbody HAS A BoxCollider (or some other collider). You attach a component for the collilder and a rigidbody. Compositing the 2 together.

This isn't to say inheritance should be completely ignored. It has its purposes. Again... a BoxCollider IS A Collider. It doesn't have a collider, it is one. I mean I guess you could composite the functionality of BoxCollider as a Collider and a GeometricShape. But then your GeometricShape gets inherited as BoxShape... and well... now you're heading into overengineering realm. So it makes sense to just have BoxCollider, SphereCollider, CapsuleCollider to all inherit from Collider and then let Rigidbody composite that. This works because the idea of having a collider with no rigidbody is a plausible configuration.

 

Thanks for your detailed insight!

In my case the display class is something like a pedestal that can hold one specific entity that can be retrieved or placed upon. (Think 1 storage slot chest)
The seedbox always holds an entity and allows a copy of such entity to be retrieved. (Think infinite shop)

Okay, lets see about a composition approach.

My main goals are

• find a way to reuse the same base implementation FROM many different scripts IN many different scripts

I would need to somehow abstract things enough to get

• a display component that displays the items in question
• a "buyable" component that only allows interaction if a specifc amout of currency is present
• an "unlockable" component that overrides any interactions to attempts to unlock
  
• a way of interacting with the object in the first place

How exactly would I go about defining the specific interaction implementations then? Would I still have a "Seedbox" or "Display" script that implements an IInteractable interface? Since they would still have similar implementations, Id probably end up using an abstract base class again.


Or how would I implement something like an "unlockable" component that has to somehow intercept those OnInteract() calls in order to unlock the object in question.
But I always get lost at the point that since we cannot be certain that an "unlockable" component is even present in the first place, we'd have to check for its existance, in every single script that might make use of that functionality.

Thank you so much for your ideas! I always tried using a composite approach but I struggled with thoughts like the ones above.

Best regards!

 

What is the difference between a Seedbox and any other 'Display'? Is there some logical difference between a 'Seedbox' and a 'MilkCrate'? (made up milkcrate since I don't know what other 'display's you have). Or is it just something like 'seedboxes hold seeds, and milkcrates hold milk...' in which case do you really need to distinguish them by type? Couldn't that just be determined by what is in the display?

Sounds to me like you just described a few different components:

ItemDisplay - logic that shows the object (what is show?)

IInteractable - an interactable
-> Purchasable - an interactable that only succeeds if enough currency is available
-> Unlockable - an interactable that on interact performs the action known as 'unlock'

InteractorLogic - the logic that calls to interact with an IInteractable

Is there a reason that the interaction logic needs to care if the thing its interacting with is an unlockable versus a purchasable?

I don't know your requirements necessarily beyond what you've said up to this point. But to me it sounds like we just need an Interact method.

...

Lets say we have a door and the door has 3 states:
Locked
Unlocked & Closed
Unlocked & Open

We could represent this as:

Code (csharp):

1. public interface IInteractable
2. {
3.     void Interact(PlayerController controller);
4. }
5.  
6. public class Door : MonoBehaviour, IInteractable
7. {
8.  
9.     public enum States
10.     {
11.         Locked,
12.         Closed,
13.         Open
14.     }
15.  
16.     public States state;
17.     public UnityEvent onUnlocked = new UnityEvent();
18.     public UnityEvent onOpened = new UnityEvent();
19.     public UnityEvent onClosed = new UnityEvent();
20.  
21.  
22.     public void Interact(PlayerController controller)
23.     {
24.         switch (state)
25.         {
26.             case States.Locked:
27.                 state = States.Closed;
28.                 onUnlocked.Invoke();
29.                 break;
30.             case States.Closed:
31.                 state = States.Open;
32.                 onOpened.Invoke();
33.                 break;
34.             case States.Open:
35.                 state = States.Closed;
36.                 onClosed.Invoke();
37.                 break;
38.         }
39.     }
40.  
41. }

You could use the UnityEvent's here to play sfx and anims and the sort. The door itself is just a state machine really.

We could even add logic here like you need keys to do it. We might also want to get an interaction status.

Code (csharp):

1. public enum InteractionResult
2. {
3.     NotInteractable = -1, //distinct from Failed in that 'IsInteractable' = false
4.     Failed = 0,
5.     Success = 1
6. }
7.  
8. public interface IInteractable
9. {
10.     bool IsInteractable { get; }
11.     InteractionResult Interact(PlayerController controller);
12. }
13.  
14. public class Door : MonoBehaviour, IInteractable
15. {
16.  
17.     public enum States
18.     {
19.         Locked,
20.         Closed,
21.         Open
22.     }
23.  
24.     public States state;
25.     public UnityEvent onUnlocked = new UnityEvent();
26.     public UnityEvent onOpened = new UnityEvent();
27.     public UnityEvent onClosed = new UnityEvent();
28.  
29.     public bool IsInteractable => this.isActiveAndEnabled;
30.  
31.     public InteractionResult Interact(PlayerController controller)
32.     {
33.         if (!this.isActiveAndEnabled) return InteractionResult.NotInteractable;
34.      
35.         switch (state)
36.         {
37.             case States.Locked:
38.                 if (controller.Inventory.Consume("DoorKey")) //doorkey could be configurable
39.                 {
40.                     state = States.Closed;
41.                     onUnlocked.Invoke();
42.                     return InteractionResult.Success;
43.                 }
44.                 else
45.                 {
46.                     return InteractionResult.Failed;
47.                 }
48.             case States.Closed:
49.                 state = States.Open;
50.                 onOpened.Invoke();
51.                 return InteractionResult.Success;
52.             case States.Open:
53.                 state = States.Closed;
54.                 onClosed.Invoke();
55.                 return InteractionResult.Success;
56.             default:
57.                 return InteractionResult.Failed;
58.         }
59.     }
60.  
61. }

Now lets say you have a shop with your display. Well for starters... it sounds to me like displays have nothing to do with interaction and are just for showing things. So that can be its own script completely independent of all of this.

Code (csharp):

1. public class ItemDisplay : MonoBehaviour
2. {
3.  
4.     //TODO - display logic... this class has nothing to do with interactions so nothing here really
5.     //pertains to the discussion at hand, which is interactions
6.  
7. }
8.  
9. public class PurchasableItem
10. {
11.     public float price;
12.     public string itemId;
13.     public UnityEvent onInteract = new UnityEvent();
14.  
15.     public bool IsInteractable => this.isActiveAndEnabled;
16.  
17.     public InteractionResult Interact(PlayerController controller)
18.     {
19.         if (!this.isActiveAndEnabled) return InteractionResult.NotInteractable;
20.      
21.         if (controller.Wallet.Spend(price))
22.         {
23.             onInteract.Invoke();
24.             controller.Inventory.Give(itemId);
25.             return InteractionResult.Success;
26.         }
27.      
28.         return InteractionResult.Failed;
29.     }
30. }
31.  
32. public class UnlockableItem
33. {
34.  
35.     public string goalId;
36.     public PurchasableItem purchasableState;
37.     public UnityEvent onInteract = new UnityEvent();
38.  
39.     void Start()
40.     {
41.         if (purchasableState) purchasableState.enabled = false;
42.     }
43.  
44.     public bool IsInteractable => this.isActiveAndEnabled;
45.  
46.     public InteractionResult Interact(PlayerController controller)
47.     {
48.         if (!this.isActiveAndEnabled) return InteractionResult.NotInteractable;
49.      
50.         if (controller.ProgressionStatus.Contains(goalId))
51.         {
52.             onInteract.Invoke();
53.             if (purchasableState) purchasableState.enabled = true;
54.             this.enabled = false;
55.             return InteractionResult.Success;
56.         }
57.      
58.         return InteractionResult.Failed;
59.     }
60. }

Note that in this idea of it. Only PurchasableItem or UnlockableItem is enabled at a given time and by interacting with UnlockableItem and the condition is met. It enables the PurchasableItem and disables itself.

This could have been changed to something like destroying self and spawning a new prefab (say the visuals change). You could have also abstracted away from 'PurchasableItem' as the 'state' reference to support unlocking things other than PurchasableItem. I'm just being generic here based on my limited knowledge of your setup.

As for our interaction logic. It could look something like:

Code (csharp):

1. public class PlayerInteractionLogic : MonoBehaviour
2. {
3.  
4.     public PlayerController controller;
5.     public UnityEvent onInteractSuccess = new UnityEvent();
6.     public UnityEvent onInteractFailed = new UnityEvent();
7.     public UnityEvent onNothingToInteractWith = new UnityEvent();
8.  
9.     void Update()
10.     {
11.         if (Input.GetButtonDown("Interact"))
12.         {
13.             this.InteractWithSurroundings();
14.         }
15.     }
16.  
17.     public void InteractWithSurroundings()
18.     {
19.         //lets just assume we use colliders to gather up nearby obejects
20.         //you could add sorting logic here to sort by some 'precedence' property, or angle off the player's forward, or something
21.         var colliders = Physics.OverlapSphere(this.transform.position, 1f, MASK_INTERACTABLELAYER, QueryTriggerInteraction.Collide);
22.         var targets = colliders.Select(o => o.GetComponent<IInteractable>()).Where(o => o != null && o.IsInteractable);
23.      
24.         foreach (var targ in targets)
25.         {
26.             switch (targ.Interact(controller))
27.             {
28.                 case InteractionResult.NotInteractable:
29.                     //do nothing, allow foreach loop to continue
30.                     break;
31.                 case InteractionResult.Failed:
32.                     onInteractFailed.Invoke();
33.                     return;
34.                 case InteractionResult.Success:
35.                     onInteractSuccess.Invoke();
36.                     return;
37.             }
38.         }
39.  
40.         onNothingToInteractWith.Invoke();
41.     }
42.  
43. }

That's a general idea...

Note - everything here was written here in the browser and should be treated as pseudo-code. There is likely spelling mistakes galore along with generalizations. Copying it into your IDE could result in errors.

 

First of all: a massive thank you for such a detailed answer and explanation, I really appreciate it!

There are only slight changes. A basic Display will allow for someone to take the item on top, place another item on top of it, or switch the carried item with the item on the display.
A Seedbox essentially acts as a shop that always displays a specific item and only allows someone to take a copy of its item, never depleting it.


I really love the idea of using an enum as a returnvalue. Up until now I have only used a boolean value to indicate if the transaction succeeded or not. Using an enum makes it much reasier to add to! thank you.

My main problem was, that I was trying too hard to avoid coupling the logic of unlocking to the logic of the specific items. Take the display example:
The player can hold exactly one thing at a time and so can a display. Interacting with said display basically just swapped the items. Some displays can start of "locked" which requires the player to interact with them and spend some currency in order to unlock them.
Since effectively any single Interactable thing can start of "locked" I felt like writing the functionality to unlock said objects seperately every single time is bad design but putting that logic in a "Unlockable" component script would have required this specific script to somehow know about the interaction happening without the main script knowing about "Unlockables" existance in the first place.

To put it simply I also come back to the following idea and I dont know if I am just overengineering at this point.

I have a script (display, door, shop...) that does something OnInteract.
I can add an "Unlockable" script to the same object that intercepts the OnInteract of the main object if it is not yet unlocked.

With your insights I now feel like the best approach would actually be to disable the main script and let the "Unlockable" script also implement the IInteractable interface.
This could then enable the main script and disable itself upon successful interaction.

Dont worry, I dont ask questions to get code to copy haha. I'd consider myself slightly experienced but I sometimes get stuck in my own thoughts trying to solve some problems so I ask for input ^^

Best regards

 

I think @lordofduct has already covered that thoroughly. Though Eric Lippert (one of the authors of the C# compiler) has a great list of why what you want is considered bad code.