OOP design confusion

Unity Engine
1

This was a lot longer than I thought so I really appreciate anyone who takes the time to read this. I forgot to mention that this is a 2D side scroller.

I’m having some major issues and I feel as though my classes are getting more convoluted as my systems become more complex. So, I’ll just dive right in to my problems.

I have a few bools that have a lot of control over player movement, things like:
canJump
canRun
canSlide
They’re pretty self explanatory, I use them inside methods to enable/disable control during certain actions. Like not being able to move when attacking (set canRun to false), or not having as much air control (again, set canRun to false and use a different vector that gives less control). Things like this have been working well.

I currently have 2 controllers for movement (BasicMovement and AdvancedMovement. BasicMovement handles collision and ground movement, it also has a default jump. AdvancedMovement inherits BasicMovement and handles wall jumping and more complex character movement I’ve reserved for the player. I figured enemies wouldn’t need the advanced functionality which is the reason I decided to split the classes.

Now, all of this worked fine until I started to add player skills. I just implemented a type of charge skill. So naturally, I had canRun set to false while charging to prevent the player from moving backwards once they initiated the charge. I decided that this class would inherit AdvancedMovement which made sense and allowed me to call the bool to restrict movement. However, this exposed a flaw in my class design. I’m now calling my movement functions from the instantiation of this new class. This also means I’ll have to endlessly have my classes inherit from the previous in order to pass along the canRun type bools that allow me to inhibit player movement when I need to.

The simplest solution that I can think of would be to create global variables using a singleton. Then I could call them wherever I’d like without the need to illogically inherit off of every previous class. The issue with this is that people say it’s dangerous and that it’s also bad design to use global variables. It might also prevent me from reusing some of these classes forcing me to duplicate some of my code.

I have yet to start animating my player and I’ll probably use sprites. I assume animations prevent other animations from running over top so maybe this problem would resolve itself later and I don’t need to use this sort of logic.

There are a lot of resources on how you should program but many of them are vague statements on the practices you should follow. A lot of them contradict each other or never explain why something is actually bad/good practice. I never know which standards to follow so I’d love for anyone to give me ideas on how to fix this or even point me towards something that really breaks down the actual reasons for avoiding certain coding practices. If you’ve read up to here, thank you so much for your time. Any input would be greatly appreciated.

2

In Unity, it would be best to embrace the component based approach instead OOP. You mentioned your Basic movement has these bools:

canJump
canRun
canSlide

You should have created small scripts that implement a certain functionality. Like:

JumpingBehaviour
RunningBehaviour
SlideBehaviour

Now notice, if you don’t want your enemies to have certain functionality, you just don’t add them that component. What you experienced, unfortunately, is the common pitfall of going full OOP with your class design.

If you can afford it, I would suggest you to rewrite all of your classes anew. As a rule of thumb, don’t subclass unless it really makes sense. In a RPG, Abilities could be a good contender for subclass since you will have many of them and they all share Activate, Deactivate methods, etc.

Please refer to 3rd post of the thread in the link:

Create your PlayerController and AIController, and make them implement IController interface. That way you will avoid all of the subclassing woes you experience and allow them each to handle their logic in their own way. All that matters is that they send an event that they, e.g. jump and your JumpingBehaviour receives that event. Logic behind is not important, since your PlayerController could use Input.GetKeyDown(KeyCode.Space) and AIController could check if platform is in front of him.

If you have any more questions, please let me know!

And yes, it may sound like a lot of time and effort, but rewrite all of your seemingly dysfunctional classes anew, you will be much more happy in the long run :slight_smile:

1 Like
3

I think I’ll take a stab at rewriting my code. I may be back to ask some questions again.

Thank you so much for the help.

4

You really want to keep your responsibilities in separate components, rather then in separate derived classes. I would suggest a movement component, and advanced movement component, and separate components for each skill. This makes the code more modular, and also makes it easier to modify things.

You can use GetComponent to talk between components on the same GameObject.

And finally, in some shameless self promotion, here is a video on the same concept:

3 Likes