Able Ability System Info and Support Thread

[U N E A S Y]

Great.
i like it

[ExtraLifeMatt]

Submitted Able v2.26

- Fixed a crash when adding Passives to yourself during the Ability update.

- Fixed an issue with Cancel Ability Task not cancelling Abilities.

- Cancelling an Ability is now safe to do during the Ability update, it will simply be queued up and executed next frame - much like activating an Ability works.

@Rthomp1 I was able to successfully cancel Abilities through the Cancel Ability Task in this update, so let me know if it still doesn't work for you.

mikecoker Still looking into the AI changes you proposed. However, the bug you were experiencing should be fixed now.

[ExtraLifeMatt]

On the reverse iteration, yes that's what I meant when I said the adds would be skipped for the current iteration. That IS okay for my specific case, but maybe there is an issue for others? Not sure.

I do see where you have protection in ActivateAbility while processing an update, so I started digging into the issue all the way through the stack and found the issue. I added a picture of where the problem is. It's in the ActivateAbility IsNetworked() block when the game IS authoritative. There is no IsProcessingUpdate check.


On a completely unrelated topic (or two), is there a reason you chose to make this an engine plugin instead of a project plugin?

Also, I know you like data driven design, so I was curious why your AI tasks take an explicit ability reference rather than using a FBlackboardKeySelector to get the ability from the blackboard. I actually modified several of the AI tasks to use the BB and it works out nicer for my case. I have a shared Attack tree that just plays whatever Ability is on the blackboard for the AI using the tree. That lets me put data in a data table and push it to the BB.

Yea, I think I'd prefer to have it delay the application of the new abilities till the next frame during an update. And yes, that line is the offending line. It should check if we're in the middle of processing an update and defer like the non-authoritative logic does above it. I'll get in a fix this weekend.

As for the Engine Plugin vs Project Plugin, UE tends to play nicer with Engine Plugins in my experience. That's the only reason.The AI feedback is really good, I'll look into adding that rather than the explicit version (or give you an option for either - I can see use cases for both).

[jimmay.10]

Hey Mike.

That change makes sense. The only problem I see is if you do a reverse iteration, your new passives may not be updated (which may or may not be intended) that frame, which may or may not be fine.

I'm slightly shocked you were able to modify that passive in place, they should be queued up to be applied the next frame. How are you applying the passives? Just calling ActivateAbility?

On the reverse iteration, yes that's what I meant when I said the adds would be skipped for the current iteration. That IS okay for my specific case, but maybe there is an issue for others? Not sure.

I do see where you have protection in ActivateAbility while processing an update, so I started digging into the issue all the way through the stack and found the issue. I added a picture of where the problem is. It's in the ActivateAbility IsNetworked() block when the game IS authoritative. There is no IsProcessingUpdate check.


On a completely unrelated topic (or two), is there a reason you chose to make this an engine plugin instead of a project plugin?

Also, I know you like data driven design, so I was curious why your AI tasks take an explicit ability reference rather than using a FBlackboardKeySelector to get the ability from the blackboard. I actually modified several of the AI tasks to use the BB and it works out nicer for my case. I have a shared Attack tree that just plays whatever Ability is on the blackboard for the AI using the tree. That lets me put data in a data table and push it to the BB.

[ExtraLifeMatt]

Thanks ExtraLifeMatt. Even with the change I'm not having any success with the CancelAbility task.

Aside from that, in my multiplayer match when a game ends I am getting a crash here:

Assertion failed: !IsUnreachable() [File:\Build\++UE4+Release-4.18+Compile\Sync\Engine\Source\Runtime\CoreUObject\Private\UObject\ScriptCore.cpp] [Line: 1158] None None.None Function: '/Script/AbleCore.AblAbility:OnAbilityEndBP'

ProjectName!FDebug::AssertFailed()
ProjectName!UObject::ProcessEvent()
ProjectName!UAblAbility::OnAbilityEndBP()
ProjectName!UAblAbilityComponent::BeginDestroy() [d:\...\able\source\ablecore\private\ablabilitycomponent.cpp:227]
ProjectName!UObject::ConditionalBeginDestroy()
ProjectName!CollectGarbageInternal()
ProjectName!CollectGarbage()
ProjectName!UEngine::TrimMemory()
ProjectName!UEngine::LoadMap()
ProjectName!UEngine::Browse()
ProjectName!UEngine::TickWorldTravel()
ProjectName!UGameEngine::Tick()
ProjectName!FEngineLoop::Tick()
ProjectName!GuardedMain()
ProjectName!GuardedMainWrapper()
ProjectName!WinMain()
ProjectName!__scrt_common_main_seh() [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283]
kernel32
ntdll

I'm not actually overriding the functionality of OnAbilityEnd in any of my ability BP's, I've even tried deleting the node from the ability event graphs but I still get this at the end of every match. Any clues?

Not off the top of my head. If possible, can you zip up your project (or create a quick sample) and upload it here. That will help me diagnose things and make sure I get the proper fix out.

[Rthomp01]

Sorry for the delay on this reply, Epic's forums don't like notifying me about posts here from some reason...

Ah, that should be a "if (!TargetActor.IsValid())", weird... not sure how that bug got in. I'll get a fix in ASAP.

Thanks ExtraLifeMatt. Even with the change I'm not having any success with the CancelAbility task.

Aside from that, in my multiplayer match when a game ends I am getting a crash here:

Assertion failed: !IsUnreachable() [File:\Build\++UE4+Release-4.18+Compile\Sync\Engine\Source\Runtime\CoreUObject\Private\UObject\ScriptCore.cpp] [Line: 1158] None None.None Function: '/Script/AbleCore.AblAbility:OnAbilityEndBP'

ProjectName!FDebug::AssertFailed()
ProjectName!UObject::ProcessEvent()
ProjectName!UAblAbility::OnAbilityEndBP()
ProjectName!UAblAbilityComponent::BeginDestroy() [d:\...\able\source\ablecore\private\ablabilitycomponent.cpp:227]
ProjectName!UObject::ConditionalBeginDestroy()
ProjectName!CollectGarbageInternal()
ProjectName!CollectGarbage()
ProjectName!UEngine::TrimMemory()
ProjectName!UEngine::LoadMap()
ProjectName!UEngine::Browse()
ProjectName!UEngine::TickWorldTravel()
ProjectName!UGameEngine::Tick()
ProjectName!FEngineLoop::Tick()
ProjectName!GuardedMain()
ProjectName!GuardedMainWrapper()
ProjectName!WinMain()
ProjectName!__scrt_common_main_seh() [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283]
kernel32
ntdll

I'm not actually overriding the functionality of OnAbilityEnd in any of my ability BP's, I've even tried deleting the node from the ability event graphs but I still get this at the end of every match. Any clues?

[ExtraLifeMatt]

ExtraLifeMatt Hi Matt, I wanted to alert you to another problem we've encountered with Passives. If you have a passive ability that uses PlayAbility to spawn a couple more passives on the same actor, there is a crash (exception in editor) that can occur. If you look in the TickComponent code where you update Passives, you're using this loop with an iterator:

for (UAblAbilityInstance* Passive : m_PassiveAbilityInstances)

If any of the InternalUpdateAbility(Passive,...) calls spawn new passives (like in the case above) then the m_PassiveAbilityInstance array will be modified. We've had a few crashes when this happens so I changed the code to do a reverse iteration like so:

for (int idx=m_PassiveAbilityInstances.Num()-1; idx >= 0; idx--)

This should allow for removals and additions (though adds will be skipped in the current iteration) without throwing any exceptions. You may choose to solve it a different way, but I wanted to let you know there was a problem.

Thanks,
Mike

Hey Mike.

That change makes sense. The only problem I see is if you do a reverse iteration, your new passives may not be updated (which may or may not be intended) that frame, which may or may not be fine.

I'm slightly shocked you were able to modify that passive in place, they should be queued up to be applied the next frame. How are you applying the passives? Just calling ActivateAbility?

[jimmay.10]

ExtraLifeMatt Hi Matt, I wanted to alert you to another problem we've encountered with Passives. If you have a passive ability that uses PlayAbility to spawn a couple more passives on the same actor, there is a crash (exception in editor) that can occur. If you look in the TickComponent code where you update Passives, you're using this loop with an iterator:

for (UAblAbilityInstance* Passive : m_PassiveAbilityInstances)

If any of the InternalUpdateAbility(Passive,...) calls spawn new passives (like in the case above) then the m_PassiveAbilityInstance array will be modified. We've had a few crashes when this happens so I changed the code to do a reverse iteration like so:

for (int idx=m_PassiveAbilityInstances.Num()-1; idx >= 0; idx--)

This should allow for removals and additions (though adds will be skipped in the current iteration) without throwing any exceptions. You may choose to solve it a different way, but I wanted to let you know there was a problem.

Thanks,
Mike

[jimmay.10]

Interesting... I would expect those values to be different (unless you happen to have a local passive play immediately as a replication comes in - at the same frame, which shouldn't be possible given how the order of operations happen). The easy fix is to simply always walk the passives list when we get a replication notification from the server for those fields. Basically make

PassiveChanged = PrePendingPassiveCount != m_PassiveAbilityInstances.Num() || ReplicatedPassivesThisFrame;

Thanks for the report, I'll do some digging and get a fix.

ok, thanks for looking into it. I'll also add some logging and continue to see if there is something else weird on my end.

EDIT: I did verify that the PassiveChanged logic is the issue. It detects removals fine, just not adds.

Edit #2: ExtraLifeMatt " when we get a replication notification from the server for those fields." -- I just re-read that. The problem is actually on the server side, not the client. The server doesn't send the values to the client in the case I mentioned above. The passives go into m_PassiveAbilityInstances locally on the server, but nothing causes them to move into m_ServerPassiveAbilities unless it gets past the PassivesChanged check. It could also be that your cachedPassiveSize set is too late. I didn't investigate that. I only added a bool that said passives changed that fram. The set of cached passive size does happen right after the active ability update, which is responsible for creating the passive, so perhaps the cache check should be above that?

[ExtraLifeMatt]

Hey Matt,

I'm having some trouble getting the CancelAbility task to perform how I'd expect.

I'm using an active ability called HealOverTime to play some effects, add a tag to the player and play a looping passive ability on the player called RegenerateHealth. For the passive ability, I want to use the CancelAbility task to check if the player is at full health, if not it keeps looping and healing, if they are then cancel the RegenerateHealth ability. I'm overriding the ShouldCancelAbility function in the abilitie's event graph but can't reach any breakpoints inside the blueprint. In the source, the task always exits after this line:

void UAblCancelAbilityTask::OnTaskStart(const TWeakObjectPtr<const UAblAbilityContext>& Context) const
{
check(Context.IsValid());

Super::OnTaskStart(Context);

TArray<TWeakObjectPtr<AActor>> TaskTargets;
GetActorsForTask(Context, TaskTargets);

for (TWeakObjectPtr<AActor> TargetActor : TaskTargets)
{
if (TargetActor.IsValid())
{
continue;
}
...
}
...
}

My target is self and the actor pointer is valid at this point (makes sense since it just continues then exits).

So I'm not sure if I'm misunderstanding the intended use of the cancel ability task or if it's even sensible to use an active ability to add a passive ability and have it loop indefinitely in this way.

Sorry for the delay on this reply, Epic's forums don't like notifying me about posts here from some reason...

Ah, that should be a "if (!TargetActor.IsValid())", weird... not sure how that bug got in. I'll get a fix in ASAP.

ExtraLifeMatt Can you please help me understand your vision of how networked abilities should work? Say I have an AoE ability that stuns a bunch of characters in an area. This ability is responsible for doing a collision query (server side) and then for each target playing the stunned ability (server side) on them. The actual stun ability is marked as a passive and has a custom event (server side) to disable controls and a particle fx task (client side) that should play on all characters.

I would expect that particle to play on all clients. I've looked at the code that creates the new ability to play and it ultimately calls AbilityComponent ActivateAbility. That code does a check to see if it is authoritative and if so (it is) it calls internalStartAbility. It sees it is a passive and goes through the ActivatePassiveAbility. I can't see where this is getting sent to the clients to also activate the passive. The result is that I don't see that particle fx play on the clients but I do see it on the local client for the server.

Is there something I'm missing here?

EDIT: Actually, I see you're using the m_PassiveAbilityInstances to replicate passives. Unfortunately in the TickComponent the PrePendingPassiveCount and m_PassiveAbilityInstances.Num seem to be the same so PassivesChanged is false. ALso, I am running PIE with a single process to test multiplayer.

Interesting... I would expect those values to be different (unless you happen to have a local passive play immediately as a replication comes in - at the same frame, which shouldn't be possible given how the order of operations happen). The easy fix is to simply always walk the passives list when we get a replication notification from the server for those fields. Basically make

PassiveChanged = PrePendingPassiveCount != m_PassiveAbilityInstances.Num() || ReplicatedPassivesThisFrame;

Thanks for the report, I'll do some digging and get a fix.

[jimmay.10]

ExtraLifeMatt Can you please help me understand your vision of how networked abilities should work? Say I have an AoE ability that stuns a bunch of characters in an area. This ability is responsible for doing a collision query (server side) and then for each target playing the stunned ability (server side) on them. The actual stun ability is marked as a passive and has a custom event (server side) to disable controls and a particle fx task (client side) that should play on all characters.

I would expect that particle to play on all clients. I've looked at the code that creates the new ability to play and it ultimately calls AbilityComponent ActivateAbility. That code does a check to see if it is authoritative and if so (it is) it calls internalStartAbility. It sees it is a passive and goes through the ActivatePassiveAbility. I can't see where this is getting sent to the clients to also activate the passive. The result is that I don't see that particle fx play on the clients but I do see it on the local client for the server.

Is there something I'm missing here?

EDIT: Actually, I see you're using the m_PassiveAbilityInstances to replicate passives. Unfortunately in the TickComponent the PrePendingPassiveCount and m_PassiveAbilityInstances.Num seem to be the same so PassivesChanged is false. ALso, I am running PIE with a single process to test multiplayer.

[Rthomp01]

Hey Matt,

I'm having some trouble getting the CancelAbility task to perform how I'd expect.

I'm using an active ability called HealOverTime to play some effects, add a tag to the player and play a looping passive ability on the player called RegenerateHealth. For the passive ability, I want to use the CancelAbility task to check if the player is at full health, if not it keeps looping and healing, if they are then cancel the RegenerateHealth ability. I'm overriding the ShouldCancelAbility function in the abilitie's event graph but can't reach any breakpoints inside the blueprint. In the source, the task always exits after this line:

void UAblCancelAbilityTask::OnTaskStart(const TWeakObjectPtr<const UAblAbilityContext>& Context) const
{
check(Context.IsValid());

Super::OnTaskStart(Context);

TArray<TWeakObjectPtr<AActor>> TaskTargets;
GetActorsForTask(Context, TaskTargets);

for (TWeakObjectPtr<AActor> TargetActor : TaskTargets)
{
if (TargetActor.IsValid())
{
continue;
}
...
}
...
}

My target is self and the actor pointer is valid at this point (makes sense since it just continues then exits).

So I'm not sure if I'm misunderstanding the intended use of the cancel ability task or if it's even sensible to use an active ability to add a passive ability and have it loop indefinitely in this way.

[ExtraLifeMatt]

I can confirm you resolved the socket rotation. TY

Excellent! Happy to hear.

[KamiSkilletLTD]

I can confirm you resolved the socket rotation. TY

[KamiSkilletLTD]

Able v2.25 has been submitted.

Notes:

• Added support for UE 4.20
• Fixed an issue where the socket rotation (for targeting or playing various effects at a location) was in the wrong transform space.
• Fixed an issue where CalculateCooldown would return 0 by default, rather than the specified cooldown.
• Added a few more guards to prevent a crash that could occur if you had an Ability Component playing an Ability when it was Garbage Collected.
• Added a few more gaurds to prevent a crash with Verbose mode.

As always, please let me know if you run into any issues. And if you can provide a repro for a crash (or a simple project showing the issue) - that is always welcomed and helps me diagnose things quickly.

Glad you were able to fix the bug I reported. Excellent support!