[UPD] GameplayCueLocal handlers refreshed to use UGameplayCueManager static functions

vorixo commented

2021-09-20 10:58:33 +00:00

(Migrated from github.com)

Now we are using the avatar as the Target for a better consistency. See: InvokeGameplayCueEvent
These static functions also call Add/RemoveLooseGameplayTag on the target's ASC, which might be convenient for gameplay

- Now we are using the avatar as the Target for a better consistency. See: InvokeGameplayCueEvent - These static functions also call `Add/RemoveLooseGameplayTag` on the target's ASC, which might be convenient for gameplay

tranek commented

2021-12-03 21:40:20 +00:00

(Migrated from github.com)

Those _NonReplicated functions were added in UE5. GASDocumentation supports 4.27 at the moment. UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue() is the correct way to do it in 4.27.

I'm not entirely convinced on the AvatarActor yet, but that's not saying that it's wrong. I'm trying to think of a case where you could have an OwnerActor but not an AvatarActor where you would still want GCs to fire. I believe that there are guards in GAS code for the scenario of having an OwnerActor but no AvatarActor so Epic is expecting that to happen.

I'm imagining if you're in a spectator where the ASC is on the PlayerState but you don't set the AvatarActor as the SpectatorPawn (we might want to set that as the AvatarActor anyway). We'd still want to trigger GCs locally. This scenario breaks down if we're using an ASC proxy, but that's non-standard GAS. It really depends on how the game is architected. The OwnerActor seems to me to be a more general catch-all case. That said, I see the benefits of using the AvatarActor for some of the internal debug code there - printing out a character's name versus a PlayerState's name is more useful.

Those `_NonReplicated` functions were added in UE5. GASDocumentation supports 4.27 at the moment. `UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue()` is the correct way to do it in 4.27. I'm not entirely convinced on the AvatarActor *yet*, but that's not saying that it's wrong. I'm trying to think of a case where you could have an OwnerActor but not an AvatarActor where you would still want GCs to fire. I believe that there are guards in GAS code for the scenario of having an OwnerActor but no AvatarActor so Epic is expecting that to happen. I'm imagining if you're in a spectator where the ASC is on the PlayerState but you don't set the AvatarActor as the SpectatorPawn (we might want to set that as the AvatarActor anyway). We'd still want to trigger GCs locally. This scenario breaks down if we're using an ASC proxy, but that's non-standard GAS. It really depends on how the game is architected. The OwnerActor seems to me to be a more general catch-all case. That said, I see the benefits of using the AvatarActor for some of the internal debug code there - printing out a character's name versus a PlayerState's name is more useful.

vorixo commented

2021-12-03 21:51:54 +00:00

(Migrated from github.com)

Those _NonReplicated functions were added in UE5. GASDocumentation supports 4.27 at the moment. UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue() is the correct way to do it in 4.27.

I'm not entirely convinced on the AvatarActor yet, but that's not saying that it's wrong. I'm trying to think of a case where you could have an OwnerActor but not an AvatarActor where you would still want GCs to fire. I believe that there are guards in GAS code for the scenario of having an OwnerActor but no AvatarActor so Epic is expecting that to happen.

I'm imagining if you're in a spectator where the ASC is on the PlayerState but you don't set the AvatarActor as the SpectatorPawn (we might want to set that as the AvatarActor anyway). We'd still want to trigger GCs locally. This scenario breaks down if we're using an ASC proxy, but that's non-standard GAS. It really depends on how the game is architected. The OwnerActor seems to me to be a more general catch-all case. That said, I see the benefits of using the AvatarActor for some of the internal debug code there - printing out a character's name versus a PlayerState's name is more useful.

Hey Dan! I agree, UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue() should be the way to go in 4.27. Let's file this part of the PR for the UE5 version If you agree.

The AvatarActor part concept is introduced in InvokeGameplayCueEvent engine function. Take a look in there, I basically did the same for consistency with the rest of the codebase. I think the reasoning behind this is that avatars are the ones supposed to play animations and effects in any present use case. An ASC without avatar will not play cues regarding to these snippets. So this change goes towards standardization more than convenience.

> Those `_NonReplicated` functions were added in UE5. GASDocumentation supports 4.27 at the moment. `UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue()` is the correct way to do it in 4.27. > > I'm not entirely convinced on the AvatarActor _yet_, but that's not saying that it's wrong. I'm trying to think of a case where you could have an OwnerActor but not an AvatarActor where you would still want GCs to fire. I believe that there are guards in GAS code for the scenario of having an OwnerActor but no AvatarActor so Epic is expecting that to happen. > > I'm imagining if you're in a spectator where the ASC is on the PlayerState but you don't set the AvatarActor as the SpectatorPawn (we might want to set that as the AvatarActor anyway). We'd still want to trigger GCs locally. This scenario breaks down if we're using an ASC proxy, but that's non-standard GAS. It really depends on how the game is architected. The OwnerActor seems to me to be a more general catch-all case. That said, I see the benefits of using the AvatarActor for some of the internal debug code there - printing out a character's name versus a PlayerState's name is more useful. Hey Dan! I agree, `UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue()` should be the way to go in 4.27. Let's file this part of the PR for the UE5 version If you agree. The AvatarActor part concept is introduced in [`InvokeGameplayCueEvent`](https://github.com/EpicGames/UnrealEngine/blob/c3caf7b6bf12ae4c8e09b606f10a09776b4d1f38/Engine/Plugins/Runtime/GameplayAbilities/Source/GameplayAbilities/Private/AbilitySystemComponent.cpp#L1154) engine function. Take a look in there, I basically did the same for consistency with the rest of the codebase. I think the reasoning behind this is that avatars are the ones supposed to play animations and effects in any present use case. An ASC without avatar will not play cues regarding to these snippets. So this change goes towards standardization more than convenience.

tranek commented

2021-12-03 22:19:26 +00:00

(Migrated from github.com)

Let's file this part of the PR for the UE5 version If you agree.

Yep, let's circle back to it in UE5. Thanks!

The AvatarActor part concept is introduced in InvokeGameplayCueEvent engine function. Take a look in there, I basically did the same for consistency with the rest of the codebase. I think the reasoning behind this is that avatars are the ones supposed to play animations and effects in any present use case. An ASC without avatar will not play cues regarding to these snippets. So this change goes towards standardization more than convenience.

I believe that UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue() will work just fine without an AvatarActor. The routing looks for a named function on the TargetActor and it falls back to the global GameplayCue set otherwise with the TargetActor passed through as the parameter into the GC.

I've done some things where I execute a local GC but don't need the local player's AvatarActor. For example, when a replicated Actor in the world is destroyed, I might hook into its EndPlay() to execute a local explosion GC by grabbing the local player's ASC and passing in the destroyed Actor through the GC parameters. While I've been able to get away with that out of sheer laziness, it's probably better to just put a GC interface on those Actors and execute it on them directly (no ASC needed).

Anyway, I think it comes down to engineer/designer choice if you want the MyTarget parameter in the GC to be the OwnerActor or AvatarActor and how you want to handle the scenario if the AvatarActor is nullptr. I opted for OwnerActor to handle AvatarActor's being invalid. I'm going to leave it that way for now in the code snippet, but I think parts of this conversation would make for good commentary next to it so that other people can make their own choice.

> Let's file this part of the PR for the UE5 version If you agree. Yep, let's circle back to it in UE5. Thanks! > The AvatarActor part concept is introduced in InvokeGameplayCueEvent engine function. Take a look in there, I basically did the same for consistency with the rest of the codebase. I think the reasoning behind this is that avatars are the ones supposed to play animations and effects in any present use case. An ASC without avatar will not play cues regarding to these snippets. So this change goes towards standardization more than convenience. I believe that `UAbilitySystemGlobals::Get().GetGameplayCueManager()->HandleGameplayCue()` will work just fine without an AvatarActor. The routing looks for a named function on the `TargetActor` and it falls back to the global GameplayCue set otherwise with the `TargetActor` passed through as the parameter into the GC. I've done some things where I execute a local GC but don't need the local player's AvatarActor. For example, when a replicated Actor in the world is destroyed, I might hook into its `EndPlay()` to execute a local explosion GC by grabbing the local player's ASC and passing in the destroyed Actor through the GC parameters. While I've been able to get away with that out of sheer laziness, it's probably better to just put a GC interface on those Actors and execute it on them directly (no ASC needed). Anyway, I think it comes down to engineer/designer choice if you want the `MyTarget` parameter in the GC to be the OwnerActor or AvatarActor and how you want to handle the scenario if the AvatarActor is nullptr. I opted for OwnerActor to handle AvatarActor's being invalid. I'm going to leave it that way for now in the code snippet, but I think parts of this conversation would make for good commentary next to it so that other people can make their own choice.

👍 1 😆 1

vorixo commented

2021-12-04 09:45:35 +00:00

(Migrated from github.com)

I agree, with my latest commit I provide a more generic solution, which can be used in any actor that implements the receiving interface.

In addition, only those actors with an ASC can benefit from the loose tag system :)

Might also want to remove the local bSuppressGameplayCues, since this can be used for "remote cueing" on actors with no ASC. What do you think?

I agree, with my latest commit I provide a more generic solution, which can be used in any actor that implements the receiving interface. In addition, only those actors with an ASC can benefit from the loose tag system :) Might also want to remove the local `bSuppressGameplayCues`, since this can be used for "remote cueing" on actors with no ASC. What do you think?

tranek commented

2021-12-07 03:24:14 +00:00

(Migrated from github.com)

If they are going to pass in a TargetActor as a parameter, then they might as well be static functions in a BlueprintFunctionLibrary. Because at that point they are not coupled at all to the ASC where the functions live. I can't comment on those functions though since I'm not looking at UE5 for GASDocumentation until it's released.

Might also want to remove the local bSuppressGameplayCues, since this can be used for "remote cueing" on actors with no ASC. What do you think?

I'm not actually familiar with that bool in regards to remote cueing.

If those _NonReplicated functions are what you want to expose to BP, then just make a static wrapper function in BlueprintFunctionLibrary class and let them handle internally whether or not suppression should happen.

If they are going to pass in a TargetActor as a parameter, then they might as well be static functions in a BlueprintFunctionLibrary. Because at that point they are not coupled at all to the ASC where the functions live. I can't comment on those functions though since I'm not looking at UE5 for GASDocumentation until it's released. > Might also want to remove the local bSuppressGameplayCues, since this can be used for "remote cueing" on actors with no ASC. What do you think? I'm not actually familiar with that bool in regards to remote cueing. If those _NonReplicated functions are what you want to expose to BP, then just make a static wrapper function in BlueprintFunctionLibrary class and let them handle internally whether or not suppression should happen.

vorixo commented

2021-12-07 09:42:37 +00:00

(Migrated from github.com)

If they are going to pass in a TargetActor as a parameter, then they might as well be static functions in a BlueprintFunctionLibrary. Because at that point they are not coupled at all to the ASC where the functions live.

Indeed, I agree. In fact this conversation we had about "remote cueing", covering the examples you exposed should be all dealt differently, as you comment, in a BPFL function (for example).

While at the same time, if you want to keep such functionality in the ASC without passing in a TargetActor, and still complying with the standardized approach, then we might fall back to the implementation of InvokeGameplayCueEvent, however in this case, using the non_replicated calls. By doing that, we won't have parts of the code in which the owner is passed over and parts of the code in which the avatar is passed over. That's why my first proposal used the AvatarActor.

If those _NonReplicated functions are what you want to expose to BP, then just make a static wrapper function in BlueprintFunctionLibrary class and let them handle internally whether or not suppression should happen.

Right, revising your comments I can draw a conclusion.

To sum up, my next proposal would consist of

A GameplayCueLocal handle in the ASC that uses the avatar as the target actor like InvokeGameplayCueEvent, for consistency.
A couple of functions in the GAS BPFL to handle remote cueing in which we do pass-in the TargetActor as a parameter.

I'll do a PR later on addressing this :)

> If they are going to pass in a TargetActor as a parameter, then they might as well be static functions in a BlueprintFunctionLibrary. Because at that point they are not coupled at all to the ASC where the functions live. Indeed, I agree. In fact this conversation we had about "remote cueing", covering the examples you exposed should be all dealt differently, as you comment, in a BPFL function (for example). While at the same time, if you want to keep such functionality in the ASC without passing in a TargetActor, and still complying with the standardized approach, then we might fall back to the implementation of [`InvokeGameplayCueEvent`](https://github.com/EpicGames/UnrealEngine/blob/c3caf7b6bf12ae4c8e09b606f10a09776b4d1f38/Engine/Plugins/Runtime/GameplayAbilities/Source/GameplayAbilities/Private/AbilitySystemComponent.cpp#L1154), however in this case, using the non_replicated calls. By doing that, we won't have parts of the code in which the owner is passed over and parts of the code in which the avatar is passed over. That's why my first proposal used the AvatarActor. > If those _NonReplicated functions are what you want to expose to BP, then just make a static wrapper function in BlueprintFunctionLibrary class and let them handle internally whether or not suppression should happen. Right, revising your comments I can draw a conclusion. To sum up, my next proposal would consist of - A `GameplayCueLocal` handle in the ASC that uses the avatar as the target actor like `InvokeGameplayCueEvent`, for **consistency**. - A couple of functions in the GAS BPFL to handle remote cueing in which we do pass-in the TargetActor as a parameter. I'll do a PR later on addressing this :)

SalahAdDin commented

2023-06-02 18:37:30 +00:00

(Migrated from github.com)

@vorixo Did you open the PR?

This pull request has changes conflicting with the target branch.

README.md

View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Merge commit title

Merge pull request '[UPD] GameplayCueLocal handlers refreshed to use UGameplayCueManager static functions' (#66) from vorixo/patch-1 into master

Merge commit body

Reviewed-on: https://ferretforge.fr/lucastucious/GASDocumentation/pulls/66

Merge pull request '[UPD] GameplayCueLocal handlers refreshed to use UGameplayCueManager static functions' (#66) from vorixo/patch-1 into master

Reviewed-on: https://ferretforge.fr/lucastucious/GASDocumentation/pulls/66

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin vorixo/patch-1:vorixo/patch-1

git switch vorixo/patch-1

Merge

Merge the changes and update on Forgejo.

git switch master

git merge --no-ff vorixo/patch-1

git switch vorixo/patch-1

git rebase master

git switch master

git merge --ff-only vorixo/patch-1

git switch vorixo/patch-1

git rebase master

git switch master

git merge --no-ff vorixo/patch-1

git switch master

git merge --squash vorixo/patch-1

git switch master

git merge --ff-only vorixo/patch-1

git switch master

git merge vorixo/patch-1

git push origin master

Rows
Columns

[UPD] GameplayCueLocal handlers refreshed to use UGameplayCueManager static functions #66

Manual merge helper

Checkout

Merge