Interact Ability and Ability System in general (long and detailed bug-ish report)

[DavidC]

So, overall, writing Abilities from scratch is a lot of work, so ideally it'd be better to extend and overwrite similar abilities (for example, Height Change if you were to make a shrink/growth ability).

I've had luck with this in some cases, but this latest time, I've been hitting a number of roadblocks.

Problem 1: Hidden requirements. Example: Interact Ability requiring a "MoveTowardsLocation" component.

Problem 2: Unextendable classes.

public class MyInteractExtentsion : Interact

Calls in Interact.cs
public override MoveTowardsLocation[] GetMoveTowardsLocations() { if (m_Interactable.gameObject.GetCachedComponents<MoveTowardsLocation>() != null) return m_Interactable.gameObject.GetCachedComponents<MoveTowardsLocation>(); else return null; }

Which fails because you cannot assign type of MyInteractClassExtension to m_Interactable

Then, the ability never starts because MoveTowardsLocation isn't found.

UltimateCharacterLocomotion.cs
898: if (m_MoveTowardsAbility.StartMoving(ability.GetMoveTowardsLocations(), ability))

Problem 3: Appearance in inspector of control, but is actually hardcoded

The reason I'm trying to overwrite Interact in the first place is because I was NOT wanting the interact ability to automatically end after the animation complete event (which fires after the first loop, even if the animation continues looping).

```m_InteractCompleteEvent.RegisterUnregisterAnimationEvent(true, m_GameObject, "OnAnimatorInteractComplete", InteractComplete);```

Which took me awhile to figure out because the Ability "Stop Type" menu/variable was still exposed, and the stop event bool under "Interact" is basically unreachable because of line 287 of Interact.CS calling StopAbility();

Desired changes:

1. Use interfaces for variables if it makes sense a class may be extended. Especially in the above case.
2. Change interact (and other abilities working similarly), so that "Animation Event" is a Stop Type and only used if that Stop Type is selected.
3. I'm aware there's bools in "Interact Event" but I would move the animation event to the ability level to unify it with start and stop types and also because animations are going to be used in most abilities.

Let me know if any further clarifications are needed.

 

[Justin]

Thanks for your feedback!

Problem 1: Hidden requirements. Example: Interact Ability requiring a "MoveTowardsLocation" component.

If the MoveTowardsLocation is null then the Interact ability can still start - it just won't move anywhere.

Which fails because you cannot assign type of MyInteractClassExtension to m_Interactable

I am not following - m_Interactable is a Interactable which is separate from the Interact class so you wouldn't be able to even cast to Interact.

Which took me awhile to figure out because the Ability "Stop Type" menu/variable was still exposed, and the stop event bool under "Interact" is basically unreachable because of line 287 of Interact.CS calling StopAbility();

Line 287 is a comment for me, but I'm guessing that you are referring to InteractComplete? The Interact component does not override CanStopAbility so if for example you set it to a button toggle then it would stop.

The Manual stop type is used by the Interact ability and this is what signifies that the code is stopping the ability. The Interact Complete Event is exposed at the inspector level so you can either use animation events or the timer.

 

[DavidC]

Forgot to mention, I updated to 3.0.10 just before this.

If the MoveTowardsLocation is null then the Interact ability can still start - it just won't move anywhere.

| get a NullReferenceException. Not on the step of MoveTowardsLocation, but because m_Interactable is null.

The target of the interaction has Interactable, Move Towards Location, and a script that implements IInteractableTarget as per comment stating the interface.

I am not following - m_Interactable is a Interactable which is separate from the Interact class so you wouldn't be able to even cast to Interact.

Oh, you're right. I was following -able = interface convention. Further analysis and attempts to get it work:


1. Reran debugger, still hitting the same error at this same point. (Screenshot 1)
2. In Interact.cs, the ValidateObject function isn't being inherited when I extend the class because it's private. Even after fixing that and spending even more time in the debugger, there's a lot of things going on preventing me from extending the class, and I think at this point, I could spend many more hours trying to make it work. At this point, it'll be faster to use a workaround.

Line 287 is a comment for me, but I'm guessing that you are referring to InteractComplete? The Interact component does not override CanStopAbility so if for example you set it to a button toggle then it would stop.

The Manual stop type is used by the Interact ability and this is what signifies that the code is stopping the ability. The Interact Complete Event is exposed at the inspector level so you can either use animation events or the timer.

I am saying this is bugged/not working as intended. Simply commenting out this line in Interact.cs changes the behavior to what you would expect. To test this, you would need to make a looping animation, or an ability that lasts longer than the animation clip (for example, drinking a potion, but then you continue to get Attribute regeneration for 30 seconds). (Screenshot 2)

 

[Justin]

The target of the interaction has Interactable, Move Towards Location, and a script that implements IInteractableTarget as per comment stating the interface.

You should remove the MoveTowardsLocation if you don't want the ability to use it. m_Interactable is set within ValidateObject. This is called during CanStartAbility and does not require a MoveTowardsLocation

I am saying this is bugged/not working as intended. Simply commenting out this line in Interact.cs changes the behavior to what you would expect. To test this, you would need to make a looping animation, or an ability that lasts longer than the animation clip (for example, drinking a potion, but then you continue to get Attribute regeneration for 30 seconds). (Screenshot 2)

In that situation you can wait for the animation but not send any animation event. You can then stop it through script. No need to adjust the ability.

 

[DavidC]

You should remove the MoveTowardsLocation if you don't want the ability to use it. m_Interactable is set within ValidateObject. This is called during CanStartAbility and does not require a MoveTowardsLocation

I am saying, if m_Interactable isn't properly set, you get a null reference error in the code during the execution stream of the line.
So, code tracing:

if (m_MoveTowardsAbility.StartMoving(ability.GetMoveTowardsLocations(), ability)); // function call


public override MoveTowardsLocation[] GetMoveTowardsLocations()
{
return m_Interactable.gameObject.GetCachedComponents<MoveTowardsLocation>(); //actual error is thrown here, before the IF resolves
}

However, even fixing this, there's like 10 other problems with extending Interact, currently, so it's really just not at all possible without just editing Interact itself.

In that situation you can wait for the animation but not send any animation event.

Are you talking about in the animator, or by having the bool "Wait for animation event" unchecked? Because it IS unchecked. That's the problem. I can find no logic that gates the StopAbility(); call nor am I seeing the expected behavior.

If you're talking about the Unity Animator system, that is very unclear.

 

[Justin]

I am saying, if m_Interactable isn't properly set, you get a null reference error in the code during the execution stream of the line.
So, code tracing:

If the interactable isn't set then that'll definitely lead to problems. What is the ability start type? Can you trace why it's not being set within ValidateObject?

Are you talking about in the animator, or by having the bool "Wait for animation event" unchecked? Because it IS unchecked. That's the problem. I can find no logic that gates the StopAbility(); call nor am I seeing the expected behavior.

You should leave it checked, and then not call the event within the animator. This will prevent InteractComplete from ever being called and you can stop it manually.