MoveTo and StartMoveAction in the same class

[seankao31]

I’m a bit skeptical on the approach where both MoveTo and StartMoveAction are in the Mover class. They are fundamentally quite different I would say. MoveTo is only meant to be called by other actions (at the moment, Fighter’s attack behavior), while StartMoveAction should be for all other external clients (for example, controllers).
I think logically we’re merging two classes into one. Logically there’s a core Movement functionality, the MoveTo method, which Fighter depends on. And then there’s the action Move, which also depends on the core MoveTo functionality. I’m think the action level move (StartMoveAction) and the underlying move (MoveTo) should probably be separated into different classes. We can identify this problem easier if we actually try to write documentation for these two methods: we’ll have to clarify which caller is supposed to call which method.
Fighter depends on MoveTo. It shouldn’t depend on StartMoveAction. Similarly, the AIController shouldn’t have access to MoveTo, and should only use StartMoveAction.

This isn’t really that big of a deal at the end of the day, but I think this is kind of a bad smell, albeit inconsequential

[Brian_Trotter]

This was considered, and if you like there’s no reason you can’t spin this off into two classes. Here’s the general reasoning behind what we did:

The actual class responsible for movement is the NavMeshAgent. At the end of the day, this is the class that determines the path and executes it. Mover itself is more of a wrapper for this NavMeshAgent. We felt it was more important that one class act as the gatekeepter to NavMeshAgent. For example, we could have simply had Fighter call NavMeshAgent directly, which would have left two classes manipulating the Agent.

While Sam doesn’t implement further use of Mover, many students (and in my own projects myself) also utilize Mover in other classes, like walking to a Pickup before picking it up (Inventory), walking to a speaker before starting a Dialogue (Dialogues and Quests) or walking to a Storekeeper before opening the store’s window (Shops and Abilities). Each of these classes simply needs to tell Mover to MoveTo a location and the NavMeshAgent remains firmly in the hands of Mover.

What this means is if at a later point, you find a different means of getting from point A to point B, you only have to change the logic in one place, the Mover, not in these other classes. It’s not something we’re covering directly in these courses but the NavMeshAgent is not the only pathfinding solution out there. If you decided at a later date to use the A* Project (an asset in the Unity Asset Store), you change the logic in Mover, and Mover only to make it work.