Merged PR 623: Fix one handed only manipulation not responding if gra…

…bbed by both hands

Currently if you set a generic manipulator to use only one handed manipulation and grab it with both hands it will no longer respond to interactions. Fixing this at the GrabTargetComponent level gives a better user experience as we can prevent the second grab entirely instead of just ignoring the second pointer, which is what MRTK-Unity does.

* Move the _ManipulationModes_ setting up to the `UxtGrabTargetComponent` and rename it to _GrabModes_
* Update the `UxtGrabTargetComponent` to:
  * Only accept grab pointers up to the _GrabMode_ limit
  * Trigger grab/release events when it's grab mode has been satisfied and not for every grab (e.g. when using only two handed grabs, a grab event is only triggered when both hands are grabbing the object)
* Improve the `UxtGrabTargetComponent` tests
  * Clean up exiting interaction tests
  * Introduce tests for focus/grab events
  * Introduce tests for grab mode settings
* Up the limit on when to use unity builds for the tests, as they cannot compile in a unity build

Related work items: #827

Docs/ReleaseNotes.md

@@ -46,6 +46,9 @@ The pinch slider component now has the option to use stepped movement. This can
 
 The _Target Component_ now uses a component picker to select its target. Due to this, the generic manipulator can no longer target components on other actors when being configured from the editor.
 
+The _ManipulationModes_ setting on the generic manipulator has been moved up the hierarchy to the `UxtGrabTargetComponent` and renamed to _GrabModes_. Its associated enum has been renamed to `EUxtGrabMode`.
+This change means that the `UxtGrabTargetComponent` will now respond to its _GrabModes_ and will only trigger grab/release events when its grab mode has been satisfied. (e.g. when using only two handed grabs, a grab event is only triggered when both hands are grabbing the object)
+
 ### UxtGrabTargetComponent
 
 The `UxtGrabTargetComponent` has been converted from a `SceneComponent` to an `ActorComponent`. This affects the classes derived from `UxtGrabTargetComponent` such as the `UxtManipulatorComponentBase` and the `UxtGenericManipulatorComponent`.

UXToolsGame/Plugins/UXTools/Source/UXTools/Private/Interactions/Constraints/UxtConstraintManager.cpp

@@ -37,7 +37,7 @@ void UxtConstraintManager::Initialize(FTransform& WorldPose)
 void UxtConstraintManager::ApplyConstraintsForType(
 	FTransform& Transform, bool IsOneHanded, bool IsNear, EUxtTransformMode TransformType) const
 {
-	int32 HandMode = static_cast<int32>(IsOneHanded ? EUxtGenericManipulationMode::OneHanded : EUxtGenericManipulationMode::TwoHanded);
+	int32 HandMode = static_cast<int32>(IsOneHanded ? EUxtGrabMode::OneHanded : EUxtGrabMode::TwoHanded);
 	int32 InteractionMode = static_cast<int32>(IsNear ? EUxtInteractionMode::Near : EUxtInteractionMode::Far);
 
 	for (UUxtTransformConstraint* Constraint : Constraints)

UXToolsGame/Plugins/UXTools/Source/UXTools/Private/Interactions/UxtGenericManipulatorComponent.cpp

@@ -22,7 +22,6 @@ UUxtGenericManipulatorComponent::UUxtGenericManipulatorComponent()
 	bAutoActivate = true;
 
 	// Default values
-	ManipulationModes = static_cast<int32>(EUxtGenericManipulationMode::OneHanded | EUxtGenericManipulationMode::TwoHanded);
 	OneHandRotationMode = EUxtOneHandRotationMode::MaintainOriginalRotation;
 	TwoHandTransformModes = static_cast<int32>(EUxtTransformMode::Translation | EUxtTransformMode::Rotation | EUxtTransformMode::Scaling);
 	ReleaseBehavior = static_cast<int32>(EUxtReleaseBehavior::KeepVelocity | EUxtReleaseBehavior::KeepAngularVelocity);
@@ -173,7 +172,7 @@ bool UUxtGenericManipulatorComponent::IsNearManipulation() const
 
 void UUxtGenericManipulatorComponent::UpdateOneHandManipulation(float DeltaTime)
 {
-	if (!(ManipulationModes & static_cast<int32>(EUxtGenericManipulationMode::OneHanded)))
+	if (!(GrabModes & static_cast<int32>(EUxtGrabMode::OneHanded)))
 	{
 		return;
 	}
@@ -194,7 +193,7 @@ void UUxtGenericManipulatorComponent::UpdateOneHandManipulation(float DeltaTime)
 
 void UUxtGenericManipulatorComponent::UpdateTwoHandManipulation(float DeltaTime)
 {
-	if (!(ManipulationModes & static_cast<int32>(EUxtGenericManipulationMode::TwoHanded)))
+	if (!(GrabModes & static_cast<int32>(EUxtGrabMode::TwoHanded)))
 	{
 		return;
 	}

UXToolsGame/Plugins/UXTools/Source/UXTools/Private/Interactions/UxtGrabTargetComponent.cpp

@@ -103,6 +103,7 @@ UUxtGrabTargetComponent::UUxtGrabTargetComponent()
 {
 	bTickOnlyWhileGrabbed = true;
 	InteractionMode = static_cast<int32>(EUxtInteractionMode::Near | EUxtInteractionMode::Far);
+	GrabModes = static_cast<int32>(EUxtGrabMode::OneHanded | EUxtGrabMode::TwoHanded);
 }
 
 const TArray<FUxtGrabPointerData>& UUxtGrabTargetComponent::GetGrabPointers() const
@@ -327,7 +328,7 @@ void UUxtGrabTargetComponent::OnExitGrabFocus_Implementation(UUxtNearPointerComp
 
 void UUxtGrabTargetComponent::OnBeginGrab_Implementation(UUxtNearPointerComponent* Pointer)
 {
-	if (!(InteractionMode & static_cast<int32>(EUxtInteractionMode::Near)))
+	if (!(InteractionMode & static_cast<int32>(EUxtInteractionMode::Near)) || !ShouldAcceptGrab())
 	{
 		return;
 	}
@@ -344,9 +345,12 @@ void UUxtGrabTargetComponent::OnBeginGrab_Implementation(UUxtNearPointerComponen
 	// Lock the grabbing pointer so we remain the focused target as it moves.
 	Pointer->SetFocusLocked(true);
 
-	OnBeginGrab.Broadcast(this, GrabData);
-
-	UpdateComponentTickEnabled();
+	// Only trigger events when the grab mode requirement has been met.
+	if (IsGrabModeRequirementMet())
+	{
+		OnBeginGrab.Broadcast(this, GrabData);
+		UpdateComponentTickEnabled();
+	}
 }
 
 void UUxtGrabTargetComponent::OnUpdateGrab_Implementation(UUxtNearPointerComponent* Pointer)
@@ -370,9 +374,16 @@ void UUxtGrabTargetComponent::OnUpdateGrab_Implementation(UUxtNearPointerCompone
 
 void UUxtGrabTargetComponent::OnEndGrab_Implementation(UUxtNearPointerComponent* Pointer)
 {
-	FUxtGrabPointerData PointerData;
+	if (!IsGrabbingPointer(Pointer))
+	{
+		return;
+	}
+
+	// Only trigger release events if the grab mode is active.
+	const bool bIsEndingGrab = IsGrabModeRequirementMet();
 
-	const int NumRemoved = GrabPointers.RemoveAll([this, Pointer, &PointerData](const FUxtGrabPointerData& GrabData) {
+	FUxtGrabPointerData PointerData;
+	GrabPointers.RemoveAll([this, Pointer, &PointerData](const FUxtGrabPointerData& GrabData) {
 		if (GrabData.NearPointer == Pointer)
 		{
 			// Unlock the pointer focus so that another target can be selected.
@@ -383,20 +394,20 @@ void UUxtGrabTargetComponent::OnEndGrab_Implementation(UUxtNearPointerComponent*
 		}
 		return false;
 	});
-	check(NumRemoved <= 1); // we might have already removed the pointer while switching interaction mode
 
-	if (NumRemoved != 0)
+	if (bIsEndingGrab)
 	{
 		OnEndGrab.Broadcast(this, PointerData);
-	}
-	// make sure to update initial ptr transforms once a pointer gets removed to ensure
-	// calculations are performed on the correct starting values
-	for (FUxtGrabPointerData& GrabData : GrabPointers)
-	{
-		InitGrabTransform(GrabData);
-	}
 
-	UpdateComponentTickEnabled();
+		// make sure to update initial ptr transforms once a pointer gets removed to ensure
+		// calculations are performed on the correct starting values
+		for (FUxtGrabPointerData& GrabData : GrabPointers)
+		{
+			InitGrabTransform(GrabData);
+		}
+
+		UpdateComponentTickEnabled();
+	}
 }
 
 bool UUxtGrabTargetComponent::IsFarFocusable_Implementation(const UPrimitiveComponent* Primitive) const
@@ -435,10 +446,11 @@ void UUxtGrabTargetComponent::InitGrabTransform(FUxtGrabPointerData& GrabData) c
 
 void UUxtGrabTargetComponent::OnFarPressed_Implementation(UUxtFarPointerComponent* Pointer)
 {
-	if (!(InteractionMode & static_cast<int32>(EUxtInteractionMode::Far)))
+	if (!(InteractionMode & static_cast<int32>(EUxtInteractionMode::Far)) || !ShouldAcceptGrab())
 	{
 		return;
 	}
+
 	FUxtGrabPointerData PointerData;
 	PointerData.FarPointer = Pointer;
 	PointerData.StartTime = GetWorld()->GetTimeSeconds();
@@ -448,15 +460,27 @@ void UUxtGrabTargetComponent::OnFarPressed_Implementation(UUxtFarPointerComponen
 
 	// Lock the grabbing pointer so we remain the hovered target as it moves.
 	Pointer->SetFocusLocked(true);
-	OnBeginGrab.Broadcast(this, PointerData);
-	UpdateComponentTickEnabled();
+
+	// Only trigger events when the grab mode requirement has been met.
+	if (IsGrabModeRequirementMet())
+	{
+		OnBeginGrab.Broadcast(this, PointerData);
+		UpdateComponentTickEnabled();
+	}
 }
 
 void UUxtGrabTargetComponent::OnFarReleased_Implementation(UUxtFarPointerComponent* Pointer)
 {
-	FUxtGrabPointerData PointerData;
+	if (!IsGrabbingPointer(Pointer))
+	{
+		return;
+	}
 
-	const int NumRemoved = GrabPointers.RemoveAll([this, Pointer, &PointerData](const FUxtGrabPointerData& GrabData) {
+	// Only trigger release events if the grab mode is active.
+	const bool bIsEndingGrab = IsGrabModeRequirementMet();
+
+	FUxtGrabPointerData PointerData;
+	GrabPointers.RemoveAll([this, Pointer, &PointerData](const FUxtGrabPointerData& GrabData) {
 		if (GrabData.FarPointer == Pointer)
 		{
 			Pointer->SetFocusLocked(false);
@@ -465,20 +489,20 @@ void UUxtGrabTargetComponent::OnFarReleased_Implementation(UUxtFarPointerCompone
 		}
 		return false;
 	});
-	check(NumRemoved <= 1); // we might have already removed the pointer while switching interaction mode
 
-	if (NumRemoved != 0)
+	if (bIsEndingGrab)
 	{
 		OnEndGrab.Broadcast(this, PointerData);
-	}
-	// make sure to update initial ptr transforms once a pointer gets removed to ensure
-	// calculations are performed on the correct starting values
-	for (FUxtGrabPointerData& GrabData : GrabPointers)
-	{
-		InitGrabTransform(GrabData);
-	}
 
-	UpdateComponentTickEnabled();
+		// make sure to update initial ptr transforms once a pointer gets removed to ensure
+		// calculations are performed on the correct starting values
+		for (FUxtGrabPointerData& GrabData : GrabPointers)
+		{
+			InitGrabTransform(GrabData);
+		}
+
+		UpdateComponentTickEnabled();
+	}
 }
 
 void UUxtGrabTargetComponent::OnFarDragged_Implementation(UUxtFarPointerComponent* Pointer)
@@ -508,3 +532,47 @@ void UUxtGrabTargetComponent::ResetLocalGrabPoint(FUxtGrabPointerData& PointerDa
 	const FTransform TransformNoScale = FTransform(Owner->GetActorRotation(), Owner->GetActorLocation());
 	PointerData.LocalGrabPoint = PointerData.GrabPointTransform * TransformNoScale.Inverse();
 }
+
+bool UUxtGrabTargetComponent::ShouldAcceptGrab() const
+{
+	if (GrabModes & static_cast<uint32_t>(EUxtGrabMode::OneHanded) && GrabModes & static_cast<uint32_t>(EUxtGrabMode::TwoHanded) &&
+		GrabPointers.Num() < 2)
+	{
+		return true;
+	}
+	else if (GrabModes & static_cast<uint32_t>(EUxtGrabMode::OneHanded) && GrabPointers.Num() < 1)
+	{
+		return true;
+	}
+	else if (GrabModes & static_cast<uint32_t>(EUxtGrabMode::TwoHanded) && GrabPointers.Num() < 2)
+	{
+		return true;
+	}
+
+	return false;
+}
+
+bool UUxtGrabTargetComponent::IsGrabModeRequirementMet() const
+{
+	if (GrabModes & static_cast<uint32_t>(EUxtGrabMode::OneHanded) && GrabModes & static_cast<uint32_t>(EUxtGrabMode::TwoHanded) &&
+		GrabPointers.Num() < 2)
+	{
+		return true;
+	}
+	else if (GrabModes & static_cast<uint32_t>(EUxtGrabMode::OneHanded) && GrabPointers.Num() == 1)
+	{
+		return true;
+	}
+	else if (GrabModes & static_cast<uint32_t>(EUxtGrabMode::TwoHanded) && GrabPointers.Num() == 2)
+	{
+		return true;
+	}
+
+	return false;
+}
+
+bool UUxtGrabTargetComponent::IsGrabbingPointer(const UUxtPointerComponent* Pointer)
+{
+	return GrabPointers.ContainsByPredicate(
+		[&Pointer](const FUxtGrabPointerData& GrabData) { return GrabData.NearPointer == Pointer || GrabData.FarPointer == Pointer; });
+}

UXToolsGame/Plugins/UXTools/Source/UXTools/Public/Interactions/Constraints/UxtTransformConstraint.h

@@ -37,8 +37,8 @@ class UXTOOLS_API UUxtTransformConstraint : public UActorComponent
 	FComponentReference TargetComponent;
 
 	/** Whether this constraint applies to one hand manipulation, two hand manipulation or both. */
-	UPROPERTY(EditAnywhere, BlueprintReadWrite, Category = Constraints, meta = (Bitmask, BitmaskEnum = EUxtGenericManipulationMode))
-	int32 HandType = static_cast<int32>(EUxtGenericManipulationMode::OneHanded | EUxtGenericManipulationMode::TwoHanded);
+	UPROPERTY(EditAnywhere, BlueprintReadWrite, Category = Constraints, meta = (Bitmask, BitmaskEnum = EUxtGrabMode))
+	int32 HandType = static_cast<int32>(EUxtGrabMode::OneHanded | EUxtGrabMode::TwoHanded);
 
 	/** Whether this constraint applies to near manipulation, far manipulation or both. */
 	UPROPERTY(EditAnywhere, BlueprintReadWrite, Category = Constraints, meta = (Bitmask, BitmaskEnum = EUxtInteractionMode))

UXToolsGame/Plugins/UXTools/Source/UXTools/Public/Interactions/UxtGenericManipulatorComponent.h

@@ -46,10 +46,6 @@ class UXTOOLS_API UUxtGenericManipulatorComponent : public UUxtManipulatorCompon
 	virtual void BeginPlay() override;
 
 public:
-	/** Enabled manipulation modes. */
-	UPROPERTY(EditAnywhere, BlueprintReadWrite, Category = GenericManipulator, meta = (Bitmask, BitmaskEnum = EUxtGenericManipulationMode))
-	int32 ManipulationModes;
-
 	/** Mode of rotation to use while using one hand only. */
 	UPROPERTY(EditAnywhere, BlueprintReadWrite, Category = GenericManipulator)
 	EUxtOneHandRotationMode OneHandRotationMode;

UXToolsGame/Plugins/UXTools/Source/UXTools/Public/Interactions/UxtGrabTargetComponent.h

@@ -264,6 +264,15 @@ class UXTOOLS_API UUxtGrabTargetComponent
 	/** Compute the grab transform relative to the current actor world transform. */
 	void ResetLocalGrabPoint(FUxtGrabPointerData& PointerData);
 
+	/** Can the object handle another grab pointer. */
+	bool ShouldAcceptGrab() const;
+
+	/** Has the grab mode requirement been fulfilled. */
+	bool IsGrabModeRequirementMet() const;
+
+	/** Is the pointer currently grabbing the object. */
+	bool IsGrabbingPointer(const class UUxtPointerComponent* Pointer);
+
 	void UpdateComponentTickEnabled();
 
 	void InitGrabTransform(FUxtGrabPointerData& GrabData) const;
@@ -309,6 +318,10 @@ class UXTOOLS_API UUxtGrabTargetComponent
 	UPROPERTY(EditAnywhere, BlueprintReadWrite, Category = Interaction, meta = (Bitmask, BitmaskEnum = EUxtInteractionMode))
 	int32 InteractionMode;
 
+	/** Enabled grab modes. */
+	UPROPERTY(EditAnywhere, BlueprintReadWrite, Category = Interaction, meta = (Bitmask, BitmaskEnum = EUxtGrabMode))
+	int32 GrabModes;
+
 private:
 	/** List of currently grabbing pointers. */
 	UPROPERTY(BlueprintGetter = "GetGrabPointers", Category = "Grabbable")

UXToolsGame/Plugins/UXTools/Source/UXTools/Public/Interactions/UxtInteractionMode.h

@@ -18,3 +18,15 @@ enum class EUxtInteractionMode : uint8
 	Far = 1 << 1,
 };
 ENUM_CLASS_FLAGS(EUxtInteractionMode)
+
+/** Grab modes supported */
+UENUM(BlueprintType, meta = (Bitflags, UseEnumValuesAsMaskValuesInEditor = "true"))
+enum class EUxtGrabMode : uint8
+{
+	None = 0 UMETA(Hidden),
+	/** Grab objects with one hand */
+	OneHanded = 1 << 0,
+	/** Grab objects with two hands */
+	TwoHanded = 1 << 1,
+};
+ENUM_CLASS_FLAGS(EUxtGrabMode)

UXToolsGame/Plugins/UXTools/Source/UXTools/Public/Interactions/UxtManipulationFlags.h

@@ -7,18 +7,6 @@
 
 #include "UxtManipulationFlags.generated.h"
 
-/** Manipulation modes supported by the generic manipulator. */
-UENUM(BlueprintType, meta = (Bitflags, UseEnumValuesAsMaskValuesInEditor = "true"))
-enum class EUxtGenericManipulationMode : uint8
-{
-	None = 0 UMETA(Hidden),
-	/** Move and rotate objects with one hand. */
-	OneHanded = 1 << 0,
-	/** Move, rotate, scale objects with two hands. */
-	TwoHanded = 1 << 1,
-};
-ENUM_CLASS_FLAGS(EUxtGenericManipulationMode)
-
 /** Specifies how the object will rotate when it is being grabbed with one hand. */
 UENUM(BlueprintType)
 enum class EUxtOneHandRotationMode : uint8