How to divide responsibility

I have a script that is responsible for interacting with an interactive object (highlight the object when hovering, process the logic when clicking, when you click on the object, you can rotate it).

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class InteractionManager : MonoBehaviour
{
    public static InteractionManager Instance;
    private ClickableObject _currentHovered;
    public ClickableObject CurrentObject => _currentHovered;
    private Camera _mainCamera;
    private bool isInteract = false;
    private bool isDraging = false;
    private float _rightClickDurationStart;
    private const float RIGHT_CLICK_DURATION = 0.3f;
    private const float ROTATION_SENSITIVITY = 0.2f;
    private Vector3 _lastMousePosition;

    private void Awake()
    {
        Instance = this;
    }
    private void Start()
    {
        _mainCamera = Camera.main;
    }
    void Update()
    {
        if (!isInteract)
        {
            FindInteractObject();
            if (_currentHovered != null)
            {
                if (Input.GetMouseButtonDown(0)) LeftClick();
            }
        }
        
        if (isInteract)
        {
            if (Input.GetMouseButtonDown(1)) RightClickStart();
            if(Input.GetMouseButton(1)) ObjectRotate();
            if (Input.GetMouseButtonUp(1)) RightClickEnd();
        }
    }

    private void FindInteractObject()
    {
        Ray ray = _mainCamera.ScreenPointToRay(Input.mousePosition);
        if (Physics.Raycast(ray, out RaycastHit hit))
        {
            if (hit.collider.TryGetComponent(out ClickableObject obj))
            {
                if (_currentHovered != obj)
                {
                    _currentHovered?.OnHoverExit();
                    _currentHovered = obj;
                    _currentHovered.OnHoverEnter();
                }
            }
            else
            {
                _currentHovered?.OnHoverExit();
                _currentHovered = null;
            }
        }
    }

    private void RightClickStart()
    {
        isDraging = false;
        _rightClickDurationStart = Time.time;
        _lastMousePosition = Input.mousePosition;
    }
    private void RightClickEnd()
    {
        if(Time.time - _rightClickDurationStart <= RIGHT_CLICK_DURATION && !isDraging)
        {
            GameEvents.TriggerInteracionEnd();
            _currentHovered.OnClick(1);
            isInteract = false;
        }
        _rightClickDurationStart = -1f;
    }
    private void LeftClick()
    {
        GameEvents.TriggerInteracionStart();
        _currentHovered.OnClick(0);
        isInteract = true;
    }

    private void ObjectRotate()
    {
        Vector3 currentMousePosition = Input.mousePosition;
        if(Vector3.Distance(currentMousePosition, _lastMousePosition) > 0.3f)
        {
            isDraging = true;

            Vector3 mouseDelta = currentMousePosition - _lastMousePosition;

            float rotationX = mouseDelta.y * ROTATION_SENSITIVITY;
            float rotationY = -mouseDelta.x * ROTATION_SENSITIVITY;
            _currentHovered.transform.Rotate(Vector3.up, rotationY, Space.World); 
            _currentHovered.transform.Rotate(Vector3.right, rotationX, Space.World); 

            _lastMousePosition = currentMousePosition;
        }
    }
}

It’s all in one script, and I think it’s wrong, so I want to split this script into several. But the problem is that I do not know how to do this correctly, because all these scripts should know about the current interactive object, should know whether I clicked on it now or not, whether I rotate it or not. Due to my inexperience, it is very difficult for me to formulate my problem and find a solution on the Internet. I would be very grateful if you could help.

Why do you think it’s wrong? Does it create any problems for you that all is in one script?

I don’t see any reason to split the script, it’s not a big script and in fact has no public methods, only private ones, so the only interaction it has is through Unity’s event methods. I don’t know why would you need to split it.

If I see any problematic areas with you script, is that static public Instance you use to access it, without making sure it is a singleton, that can create errors in the future in case you have two of those scripts, and that static Gameevents you are accessing. What does this Instance serve? What do you access in this script other than the public CurrentObject? Why not make this static and don’t have an Instance?

If anything I would say you will have problems with global state by creating dependencies with too many statics that in the future will make everything that has to do with testing and debugging harder, but this has nothing to do with the splitting. You don’t need to divide this script.

3 Likes

It might make sense to relocate ObjectRotate to be part of ClickableObject, so that it can be overridden and customized for different interaction targets.

Other than that this makes sense to me.

I wouldn’t advice trying to forcefully divide classes into smaller ones, just because the single-responsibility principle exists, if there’s no practical benefit to it that you can identify.

It can actually often be better to have a single somewhat larger class that neatly encapsulates a chunk of complexity behind an easy-to-use API.

If you try to forcefully divide a class into smaller ones, it can break encapsulation, and result in multiple more leaky and less-useful abstractions.

4 Likes

Thank you very much for your answers. I decided to ask this question because interactive objects contain a worldspace UI. Let’s say if I need to disable the rotation of an object when I click a button, I will need to expand this code. Or when I clicked on the button, the Panel popped out, which is removed if I right-click, I need to expand the code again, because the interactive object is removed with the right mouse button.

Focus on solving problems when they arise, don’t attempt to preemptively address issues that don’t yet exist.

When you expand the code, and if the code seems to become too big and the class has more than one reason to change, and if this creates a problem in your codebase, address it at that point.

Until then keep coding and if these challenges arise, we will still be here to help with the new code.

3 Likes

I cant remember which game it is but there is a big major game which famously apparently has shockingly badly done code in it.

As has been pointed out - often dont waste your life making your code pretty - your users dont care, they dont see it. If the code is maintainable, working, and clear, job is done. It is far too easy to get wrapped up in a ton of stuff for zero gain, you can split everything out into tons of classes, and add reflection or injection or whole hosts of things… but if your game is say guess a number… then… really why?

Sometimes you want to split things out to add functionality, thats fine, do that when you get to it.

2 Likes

Yandere Simulator?

_
Also

Maintainable Code is usually readable code. In this specific example, the whole thing has only 100 lines, No reason to split it up. But for monolithic 800+ line classes splitting them up makes it more maintainable - spending less time scrolling and more time actually getting something done.

1 Like

To be fair even hugely popular games like Terraria and Subnautica have astonishingly bad code.

If you want a horror movie, use dotpeek or whatnot to look at Subnautica’s main assembly and find Base.cs, and be prepared to be horrified.

I’m worried that if I don’t improve the code now, I’ll never improve it and I won’t learn how to write clean code. To be honest, I’ve abandoned a lot of projects because of this mindset :/.

Then allocate yourself say 2 hours max, pick the problem you think is the biggest - such as its a mega class of everything, refactor it out. (of course you’re using source control in case you stuff it all up right), break things down a bit, get a bit nearer to that single responsibility idea.

If you have an actual issue caused by your code, such as poor performance, address those first…

2 Likes

I mean what we’re saying isn’t to say that there aren’t other parts of the project that can be improved.

And there’s no harm in doing some code gardening, especially when you learn better ways to do things (which you absolutely will the more you code).

And in time you will grow a sense of how to design things to avoid future issues.

What’s important is to notice when you start having pain points in your code base, and looking to fix them. You may need to do some research, get creative, or hit up the forums to come up with a solution.

But you won’t sense those pain points by simply writing a script and then asking yourself of your peers what’s wrong with it. Issues generally present themselves over time, or with testing. Sometimes it can be a harsh lesson, but they will be important lessons for future projects.

3 Likes

Instead of having a global interaction manager you could place the following script on your ‘ClickableObjects’ and it will rotate them when you drag the mouse over them.

using UnityEngine;
public class InteractionManager : MonoBehaviour
{
    void OnMouseDrag()
    {
        Vector3 mouseDelta = new Vector3(-Input.GetAxis("Mouse X"),Input.GetAxis("Mouse Y"),0);
        transform.Rotate(Vector3.up, mouseDelta.x, Space.World); 
        transform.Rotate(Vector3.right, mouseDelta.y, Space.World);
    }
}
1 Like

You meant to say you can’t remember which big major game is famous for having shockingly good code?

In that sense, neither can I. :grin:

No I dont remember because it wasnt a game I played… and so on a personal level dont care, so didnt think to commit it to memory