PedrelliLuca / ScalarField

A TopDown RPG Game, where magic works by interacting with the environment.
4 stars 0 forks source link

Command Pattern for Movement Logic #24

Closed PedrelliLuca closed 2 years ago

PedrelliLuca commented 2 years ago

What?

Employ the Command Pattern to make the movement logic in the controllers settable from skills.

Why?

While working on #22, I realized that the "skill + skill execution state" combination determines the movement logic of the caster. For example:

Of course, the logic depends on whether the controller is a Player or an AI controller. A "translation and rotation" logic for a player controller is based on mouse clicks and cursor location. In contrast, AI's logic for the same movement kind might be based on an algorithm searching for the closest enemy on the map.

PedrelliLuca commented 2 years ago

Here is the first draft of the UML for the Command pattern. Green classes are additions needed to close this issue, pink ones are classes to be implemented in the future that are represented just to make a point. Violet attributes are set by designers using the editor.

movementCmd

@startuml
skinparam sequenceArrowThickness 2
skinparam roundcorner 20
skinparam sequenceParticipant underline
skinparam linetype ortho

package MovementCommand <<Rectangle>> {
    class FMovementCommandInvoker #palegreen {
        + void SetMovementCommand(ICommand* movCmd)
        + const IMovementCommand* GetMovementCommand() const
        --
        - TSharedPtr<IMovementCommand> _movementCommand
    }

    interface IMovementCommand #palegreen {
        + virtual void OnSetDestination() = 0
        + virtual void OnStopMovement() = 0
        + virtual void OnMovementTick() = 0
    }

    class FPlayerTranslationRotationMovement #palegreen {
        // Virtual functions' override
        --
        - UNiagaraSystem* <color:darkviolet> _fxCursor
        - double <color:darkviolet> _shortPressThreshold
    }

    class FPlayerRotationOnlyMovement #palegreen {
        // Virtual functions' override
    }

    class FAITranslationRotationMovement #pink {
        // Virtual functions' override
    }

    FMovementCommandInvoker --> IMovementCommand
    IMovementCommand <|.. FPlayerTranslationRotationMovement
    IMovementCommand <|.. FPlayerRotationOnlyMovement
    IMovementCommand <|.. FAITranslationRotationMovement #line:red
}
note bottom of FAITranslationRotationMovement: To be implemented
package ScalarField <<Rectangle>> {
    class AScalarFieldPlayerController

    class AScalarFieldAIController #pink
}
note bottom of AScalarFieldAIController: To be implemented

FMovementCommandInvoker <|-- AScalarFieldPlayerController 
FMovementCommandInvoker <|-- AScalarFieldAIController #line:red

package SkillSystem <<Rectangle>> {
    struct FSkillParameters {
        + TSubclassOf<IMovementCommand> <color:darkviolet> TargetingMovementCommand
        + TSubclassOf<IMovementCommand> <color:darkviolet> CastingMovementCommand
        + TSubclassOf<IMovementCommand> <color:darkviolet> ChannelingMovementCommand
        + // Other public fields
    }

    abstract class UAbstractSkill {}

    UAbstractSkill --> FSkillParameters
}

FMovementCommandInvoker <.. UAbstractSkill
IMovementCommand <.. FSkillParameters

@enduml
PedrelliLuca commented 2 years ago

Here is an high-level overview on how the modules are related to the new "MovementCommand" module: movementCmdModules

@startuml
skinparam sequenceArrowThickness 2
skinparam roundcorner 20
skinparam sequenceParticipant underline
skinparam linetype ortho

package SkillSystem <<Rectangle>> {    
}

package ScalarField <<Rectangle>> {
}

package MovementCommand <<Rectangle>> #palegreen {
}

ScalarField +-- MovementCommand
ScalarField +-- SkillSystem
SkillSystem +-- MovementCommand

@enduml
PedrelliLuca commented 2 years ago

UML, take 2: addition of an enum that allows the skill to be completely unaware of the kind of controller they're dealing with.

image

PedrelliLuca commented 2 years ago

I realized that two interfaces are needed due to the fact that:

  1. Movement is extremely dependent on APlayerController and AAIController implementations
  2. I can't make a class implementing a general UMovementCommandInvoker and AController, that would require me to rewrite Unreal's APlayerController and AAIController 😰

image

@startuml
skinparam sequenceArrowThickness 2
skinparam roundcorner 20
skinparam sequenceParticipant underline
skinparam linetype ortho

package MovementCommand <<Rectangle>> {
    enum EMovementMode #palegreen {
        MM_Still
        MM_Rotation
        MM_Translation
        MM_RotoTranslation
    }

    interface IMovementCommandInvoker #palegreen {
        + virtual bool IsInMovementMode(EMovementMode mode) = 0
        + virtual void SetMovementMode(EMovementMode mode) = 0
    }

    class UPlayerMovementCommandInvoker #palegreen {
        // Virtual functions' override
        --
        # TScriptInterface<const IPlayerMovementCommand> _getMovementCommand()
        --
        - TMap<EMovementMode, TSubclassOf<IPlayerMovementCommand>> <color:darkviolet> _modesToCommandClasses
        - EMovementMode _activeMovementMode
        - TScriptInterface<IPlayerMovementCommand> _activeMovementCommand
    }

    class UAIMovementCommandInvoker #palegreen {
        // Virtual functions' override
        --
        # TScriptInterface<const IAIMovementCommand> _getMovementCommand()
        --
        - TMap<EMovementMode, TSubclassOf<IAIMovementCommand>> <color:darkviolet> _modesToCommandClasses
        - EMovementMode _activeMovementMode
        - TScriptInterface<IAIMovementCommand> _activeMovementCommand
    }

    EMovementMode <-- IMovementCommandInvoker
    IMovementCommandInvoker <|.. UPlayerMovementCommandInvoker
    IMovementCommandInvoker <|.. UAIMovementCommandInvoker

    interface IPlayerMovementCommand #palegreen {
        + virtual void OnSetDestination(const TObjectPtr<APlayerController>& playerController) = 0
        + virtual void OnStopMovement(const TObjectPtr<APlayerController>& playerController) = 0
        + virtual void OnMovementTick(const TObjectPtr<APlayerController>& playerController) = 0
    }

    interface IAIMovementCommand #palegreen {
        + virtual void OnSetDestination(const TObjectPtr<AAIController>& aiController) = 0
        + virtual void OnStopMovement(const TObjectPtr<AAIController>& aiController) = 0
        + virtual void OnMovementTick(const TObjectPtr<AAIController>& aiController) = 0
    }

    UPlayerMovementCommandInvoker --> IPlayerMovementCommand
    UAIMovementCommandInvoker --> IAIMovementCommand

    class UPlayerRotoTranslationMovement #palegreen {
        // Virtual functions' override
        --
        - UNiagaraSystem* <color:darkviolet> _fxCursor
        - double <color:darkviolet> _shortPressThreshold
    }

    class UPlayerRotationOnlyMovement #palegreen {
        // Virtual functions' override
    }

    IPlayerMovementCommand <|.. UPlayerRotoTranslationMovement
    IPlayerMovementCommand <|.. UPlayerRotationOnlyMovement

    class UAITranslationRotationMovement #pink {
        // Virtual functions' override
    }

    IAIMovementCommand <|.. UAITranslationRotationMovement #line:red

}

package ScalarField <<Rectangle>> {
    class AScalarFieldPlayerController

    class AScalarFieldAIController #pink
}

UPlayerMovementCommandInvoker <|-- AScalarFieldPlayerController 
UAIMovementCommandInvoker <|-- AScalarFieldAIController #line:red

package SkillSystem <<Rectangle>> {
    struct FSkillParameters {
        + EMovementMode <color:darkviolet> TargetingMovementMode
        + EMovementMode <color:darkviolet> CastingMovementMode
        + EMovementMode <color:darkviolet> ChannelingMovementMode
        + // Other public fields
    }

    abstract class UAbstractSkill {}

    UAbstractSkill --> FSkillParameters
}

IMovementCommandInvoker <.. UAbstractSkill
EMovementMode <.. FSkillParameters

@enduml
PedrelliLuca commented 2 years ago

The previous UML contains a subtle class diamond:

The final result is that AScalarFieldPlayerController inherits from UObject two times ♦️

PedrelliLuca commented 2 years ago

To fix the previously described diamond problem, here is a UML based on components:

image

The drawback of this approach is that, potentially, UAbstractSkills could call IPlayerMovementCommand functions. With the previous inheritance-based approach this was impossible. However, since this little code smell is nothing compared to the diamond problem explained above, I'll go for this approach.

@startuml
skinparam sequenceArrowThickness 2
skinparam roundcorner 20
skinparam sequenceParticipant underline
skinparam linetype ortho

package MovementCommand <<Rectangle>> {
    enum EMovementMode #palegreen {
        MM_Still
        MM_Rotation
        MM_Translation
        MM_RotoTranslation
    }

    interface IMovementModeSetter #palegreen {
        + virtual bool IsInMovementMode(EMovementMode mode) = 0
        + virtual void SetMovementMode(EMovementMode mode) = 0
    }

    EMovementMode <.. IMovementModeSetter

    class UPlayerMovementCommandComponent #palegreen {
        // Virtual functions' override
        + TScriptInterface<IPlayerMovementCommand> _getMovementCommand()
        --
        - TMap<EMovementMode, TSubclassOf<IPlayerMovementCommand>> <color:darkviolet> _modesToCommandClasses
        - EMovementMode _activeMovementMode
        - TScriptInterface<IPlayerMovementCommand> _activeMovementCommand
    } 

    class UAIMovementCommandComponent #palegreen {
        // Virtual functions' override
        + TScriptInterface<IAIMovementCommand> _getMovementCommand()
        --
        - TMap<EMovementMode, TSubclassOf<IAIMovementCommand>> <color:darkviolet> _modesToCommandClasses
        - EMovementMode _activeMovementMode
        - TScriptInterface<IAIMovementCommand> _activeMovementCommand
    } 

    IMovementModeSetter <|.. UPlayerMovementCommandComponent
    IMovementModeSetter <|.. UAIMovementCommandComponent

    interface IPlayerMovementCommand #palegreen {
        + virtual void OnSetDestination(const TObjectPtr<APlayerController>& playerController) = 0
        + virtual void OnStopMovement(const TObjectPtr<APlayerController>& playerController) = 0
        + virtual void OnMovementTick(const TObjectPtr<APlayerController>& playerController) = 0
    }

    interface IAIMovementCommand #palegreen {
        + virtual void OnSetDestination(const TObjectPtr<AAIController>& aiController) = 0
        + virtual void OnStopMovement(const TObjectPtr<AAIController>& aiController) = 0
        + virtual void OnMovementTick(const TObjectPtr<AAIController>& aiController) = 0
    }

    UPlayerMovementCommandComponent --> IPlayerMovementCommand
    UAIMovementCommandComponent --> IAIMovementCommand

    class UPlayerRotoTranslationMovement #palegreen {
        // Virtual functions' override
        --
        - UNiagaraSystem* <color:darkviolet> _fxCursor
        - double <color:darkviolet> _shortPressThreshold
    }

    class UPlayerRotationOnlyMovement #palegreen {
        // Virtual functions' override
    }

    IPlayerMovementCommand <|.. UPlayerRotoTranslationMovement
    IPlayerMovementCommand <|.. UPlayerRotationOnlyMovement

    class UAITranslationRotationMovement #pink {
        // Virtual functions' override
    }

    IAIMovementCommand <|.. UAITranslationRotationMovement #line:red
}

package ScalarField <<Rectangle>> {
    class AScalarFieldPlayerController

    class AScalarFieldAIController #pink
}

UPlayerMovementCommandComponent <-- AScalarFieldPlayerController
UAIMovementCommandComponent <-- AScalarFieldAIController

package SkillSystem <<Rectangle>> {
    struct FSkillParameters {
        + EMovementMode <color:darkviolet> TargetingMovementMode
        + EMovementMode <color:darkviolet> CastingMovementMode
        + EMovementMode <color:darkviolet> ChannelingMovementMode
        + // Other public fields
    }

    abstract class UAbstractSkill {}

    UAbstractSkill --> FSkillParameters
}

EMovementMode <-- FSkillParameters
IMovementModeSetter <.. UAbstractSkill

@enduml
PedrelliLuca commented 2 years ago

Below is the UML that has been realized in #23. The only significant difference with the one above is that the player and AI movement commands are now abstract classes instead of interfaces. This change was needed to use NewObject() with custom blueprints when changing movement command via SetMovementMode()

image

@startuml
skinparam sequenceArrowThickness 2
skinparam roundcorner 20
skinparam sequenceParticipant underline
skinparam linetype ortho

package MovementCommand <<Rectangle>> {
    enum EMovementMode #palegreen {
        MM_Still
        MM_Rotation
        MM_Translation
        MM_RotoTranslation
    }

    interface IMovementModeSetter #palegreen {
        + virtual bool IsInMovementMode(EMovementMode mode) = 0
        + virtual void SetMovementMode(EMovementMode mode) = 0
    }

    EMovementMode <.. IMovementModeSetter

    class UPlayerMovementCommandComponent #palegreen {
        // Virtual functions' override
        + TScriptInterface<UPlayerMovementCommand> _getMovementCommand()
        --
        - TMap<EMovementMode, TSubclassOf<UPlayerMovementCommand>> <color:darkviolet> _modesToCommandClasses
        - EMovementMode _activeMovementMode
        - TScriptInterface<UPlayerMovementCommand> _activeMovementCommand
    } 

    class UAIMovementCommandComponent #palegreen {
        // Virtual functions' override
        + TScriptInterface<UAIMovementCommand> _getMovementCommand()
        --
        - TMap<EMovementMode, TSubclassOf<UAIMovementCommand>> <color:darkviolet> _modesToCommandClasses
        - EMovementMode _activeMovementMode
        - TScriptInterface<UAIMovementCommand> _activeMovementCommand
    } 

    IMovementModeSetter <|.. UPlayerMovementCommandComponent
    IMovementModeSetter <|.. UAIMovementCommandComponent

    abstract class UPlayerMovementCommand #palegreen {
        + virtual void OnSetDestination(const TObjectPtr<APlayerController>& playerController) = 0
        + virtual void OnStopMovement(const TObjectPtr<APlayerController>& playerController) = 0
        + virtual void OnMovementTick(const TObjectPtr<APlayerController>& playerController) = 0
    }

    abstract class UAIMovementCommand #palegreen {
        + virtual void OnSetDestination(const TObjectPtr<AAIController>& aiController) = 0
        + virtual void OnStopMovement(const TObjectPtr<AAIController>& aiController) = 0
        + virtual void OnMovementTick(const TObjectPtr<AAIController>& aiController) = 0
    }

    UPlayerMovementCommandComponent --> UPlayerMovementCommand
    UAIMovementCommandComponent --> UAIMovementCommand

    class UPlayerRotoTranslationMovement #palegreen {
        // Virtual functions' override
        --
        - UNiagaraSystem* <color:darkviolet> _fxCursor
        - double <color:darkviolet> _shortPressThreshold
    }

    class UPlayerRotationOnlyMovement #palegreen {
        // Virtual functions' override
    }

    class UPlayerStillMovement #palegreen {
        // Virtual functions' override
    }

    UPlayerMovementCommand <|-- UPlayerRotoTranslationMovement
    UPlayerMovementCommand <|-- UPlayerRotationOnlyMovement
    UPlayerMovementCommand <|-- UPlayerStillMovement

    class UAITranslationRotationMovement #pink {
        // Virtual functions' override
    }

    UAIMovementCommand <|-- UAITranslationRotationMovement #line:red
}

package ScalarField <<Rectangle>> {
    class AScalarFieldPlayerController

    class AScalarFieldAIController #pink
}

UPlayerMovementCommandComponent <-- AScalarFieldPlayerController
UAIMovementCommandComponent <-- AScalarFieldAIController

package SkillSystem <<Rectangle>> {
    struct FSkillParameters {
        + EMovementMode <color:darkviolet> TargetingMovementMode
        + EMovementMode <color:darkviolet> CastingMovementMode
        + EMovementMode <color:darkviolet> ChannelingMovementMode
        + // Other public fields
    }

    abstract class UAbstractSkill {}

    UAbstractSkill --> FSkillParameters
}

EMovementMode <-- FSkillParameters
IMovementModeSetter <.. UAbstractSkill: FSM reads the skills' movement mode for the current\nstate and sets it on the controller 
@enduml