1
\$\begingroup\$

Commands may have different target type; like NONE, COORD, UNIT.
Some example commands may be: Attack (target UNIT), Move (target COORD), AttackMove (target COORD), Stop (target NONE), UseAbility (target ability.getTargetType).
Commands will be executed by UI or AI.
So in an OOP approach here, what methods should my Command interface have?
For the command execution has access to the target the command wants.

\$\endgroup\$
1
\$\begingroup\$

There are not so many combinations when you write them all down.

We just have a bunch of overloaded methods alike:

CmdArmy(cmdType)
CmdArmy(cmdType, Unit)
CmdArmy(cmdType, Coord)
CmdArmy(cmdType, Param)
CmdArmy(cmdType, Coord, Param)

Where cmdType is a key alike:

ctAttack, ctMove, ctStop, ctAbility, ctEtc ...

It's not OOP, but it's going along the lines of KISS quite well.

\$\endgroup\$
  • \$\begingroup\$ +1, but I want my commands to be objects so I can store them in queues, histories and send them over the network. I'm thinking of a CommandData class, a CommandType interface, and a final Command class composite both type and data. CommandProcessSystem will just execute the CommandType.execute with the CommandData. \$\endgroup\$ – hope_is_grim Sep 1 '14 at 18:41
  • \$\begingroup\$ Sounds like an unnecessary complication, if you can store any command in 12 bytes by a pointer. \$\endgroup\$ – Kromster says support Monica Sep 2 '14 at 5:35
0
\$\begingroup\$

I'd use a simple base class. That allows you to write commands like this (You didn't specify a language so I'll write something C#-like, using some concepts from Unity):

abstract class Command
{
    abstract string Description {get;} //Maybe you want all commands to have a description in your UI
    abstract string Name {get;}//This is also for the UI. Neither of these affect your logic
    //...More things you want all commands to have

    abstract void Apply(UnitController unitController, Action onCompletedCallback);

}

class MoveCommand : Command
{
    override string Description 
    {
       return "This moves a Unit" //You'd probably have some logic here for localized strings instead of a literal
    }

    override string Name 
    {
        return "Move" //See above
    }

    private readonly Vector3 TargetLocation;

    //...More command specific members

    MoveCommand(Vector3 targetLocation)
    {
        TargetLocation = targetLocation;
    }

    override void Apply(UnitController unitController,Action onCompletedCallback)
    {

        //Something along the lines of:
        unitController.MoveTo(TargetLocation) //Unit controller owns the logic for movement 

        // --------------------Or---------------
        var pathFinder = unitController.GetComponent<Pathfinder>(); 
        if(pathFinder == null) 
             //Throw some exception
        pathFinder.Move(TargetLocation,onCompletedCallback); //Path finder owns the logic for movement

        //---------------------Or---------------
        var movementMangager = GlobalManagers.Resolve<IUnitMovementManager>();
        movementManager.MoveUnit(unitController,TargetLocation); //Some class owns the logic for movement.
    }
}
\$\endgroup\$
  • \$\begingroup\$ so I need to repeat the target logic in all my command and hard-code downcast every command in order to use it? \$\endgroup\$ – hope_is_grim Sep 1 '14 at 18:43
  • \$\begingroup\$ My last comment got mangled, what I was trying to say was "You wouldn't need to repeat any logic, but without knowing more about your setup I can't go into details on how to prevent repeated logic, but I can give a small example.", I added the examples \$\endgroup\$ – Selali Adobor Sep 1 '14 at 19:52
  • \$\begingroup\$ still have to downcast Command to MoveCommand in order to set TargetLocation; so, hard code? \$\endgroup\$ – hope_is_grim Sep 1 '14 at 22:55
  • 1
    \$\begingroup\$ TargetLocation is part of the constructor and only used within the MoveCommand class. So you call the constructor for MoveCommand with TargetLocation (eg. new MoveCommand(SomeLocation)), but pass it around as a Command. You don't ever need to cast Command to MoveCommand, or vice-versa, because MoveCommand inherits from Command which exposes Apply, and TargetLocation can only be set from the constructor of MoveCommand (and should only be accessible from MoveCommand, I simply omitted scope in my example). \$\endgroup\$ – Selali Adobor Sep 1 '14 at 23:44
  • \$\begingroup\$ To clarify my intention of the use of TargetLocation I made it a private readonly member. \$\endgroup\$ – Selali Adobor Sep 1 '14 at 23:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.