lucastucious/GASDocumentation
1
0
Fork 0
Code Issues Pull requests 2 Releases Activity

Fix Initialization in AcknowledgePossession, possible race condition #78

Open
linqen wants to merge 1 commit from linqen/patch-1 into master
pull from: linqen/patch-1
merge into: lucastucious:master
Conversation 3 Commits 1 Files changed 1 +15
linqen commented 2022-04-15 13:30:30 +00:00 (Migrated from github.com)
Copy link

After a lot of debugging, I found that there is a possible race condition during bad network conditions (~200ms, 5% packet loss) with 5%~10% of repro chances.

If this happens, ASC will not be initialized properly and you will get LogAbilitySystem: Warning: Can't activate LocalOnly or LocalPredicted ability %s when not local!

This happens because during AcknowledgePossession, the pawn controller is not setup yet, and during FGameplayAbilityActorInfo::InitFromActor, PlayerController will be null forever, and then, when calling UAbilitySystemComponent::InternalTryActivateAbility, bool bIsLocal = AbilityActorInfo->IsLocallyControlled() will be false

I consider that OnRep_Pawn is a good place to call RefreshAbilityActorInfo, that will call FGameplayAbilityActorInfo::InitFromActor again, but the controller will be valid at that point.

After a lot of debugging, I found that there is a possible race condition during bad network conditions (~200ms, 5% packet loss) with 5%~10% of repro chances. If this happens, ASC will not be initialized properly and you will get `LogAbilitySystem: Warning: Can't activate LocalOnly or LocalPredicted ability %s when not local!` This happens because during `AcknowledgePossession`, the pawn controller is not setup yet, and during `FGameplayAbilityActorInfo::InitFromActor`, `PlayerController` will be null forever, and then, when calling `UAbilitySystemComponent::InternalTryActivateAbility`, `bool bIsLocal = AbilityActorInfo->IsLocallyControlled()` will be false I consider that `OnRep_Pawn` is a good place to call `RefreshAbilityActorInfo`, that will call `FGameplayAbilityActorInfo::InitFromActor` again, but the controller will be valid at that point.
linqen commented 2022-10-17 17:37:24 +00:00 (Migrated from github.com)
Copy link

@tranek this issue is still reproduceable, and following the documentation examples will end in this issue sooner or later in any networked game.

If there is a way I can help you to repro this, and waste less time, let me know.

@tranek this issue is still reproduceable, and following the documentation examples will end in this issue sooner or later in any networked game. If there is a way I can help you to repro this, and waste less time, let me know.
SalahAdDin commented 2023-06-02 18:34:44 +00:00 (Migrated from github.com)
Copy link

@linqen Is it reproducible yet?

@linqen Is it reproducible yet?
linqen commented 2023-07-13 11:56:48 +00:00 (Migrated from github.com)
Copy link

@linqen Is it reproducible yet?

It is using UE 5.0. I don't have much free time right now, so I can't confirm on newer versions.

> @linqen Is it reproducible yet? It is using UE 5.0. I don't have much free time right now, so I can't confirm on newer versions.
😕 1
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin linqen/patch-1:linqen/patch-1
git switch linqen/patch-1

Merge

Merge the changes and update on Forgejo.
git switch master
git merge --no-ff linqen/patch-1
git switch linqen/patch-1
git rebase master
git switch master
git merge --ff-only linqen/patch-1
git switch linqen/patch-1
git rebase master
git switch master
git merge --no-ff linqen/patch-1
git switch master
git merge --squash linqen/patch-1
git switch master
git merge --ff-only linqen/patch-1
git switch master
git merge linqen/patch-1
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
breaking
Introduces a breaking change.

  
effort
1

  
effort
13

  
effort
2

  
effort
3

  
effort
5

  
effort
8

  
good first issue
Good for newcomers.

  
help
Help wanted for this issue.

  
priority
2day
 The priority is high

  
priority
now
 The priority is critical

  
priority
soon
 The priority is medium

  
state
approved
 Approved to proceed.

  
state
blocked
 Something is blocking action.

  
state
inactive
 No action needed or possible. The issue is fixed, addressed elsewhere, or out of scope.

  
state
pending
 Pending requirements, dependencies, data, or more information.

  
type
bug
 Something isn"t working.

  
type
chore
 Converting measurements, reorganizing folder structure, and less impactful tasks.

  
type
discussion
 Questions, proposals and info that requires discussion.

  
type
docs
 Related to documentation and information.

  
type
feature
 Brand new functionality, features, pages, workflows, endpoints, etc.

  
type
fix
 Iterations on existing features or infrastructure.

  
type
security
 Something is vulnerable or not secure.

  
type
testing
 Related to testing.

No labels
breaking
effort
1
effort
13
effort
2
effort
3
effort
5
effort
8
good first issue
help
priority
2day
priority
now
priority
soon
state
approved
state
blocked
state
inactive
state
pending
type
bug
type
chore
type
discussion
type
docs
type
feature
type
fix
type
security
type
testing
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lucastucious/GASDocumentation!78
No description provided.