tranek / GASShooter

Advanced FPS/TPS Sample Project for Unreal Engine 4's GameplayAbilitySystem plugin
MIT License
953 stars 265 forks source link

OnRep_PlayerState CreateHUD can fail if Player state has replicated on GSHeroCharacter.cpp before the player controller #22

Open ArchageXIII opened 3 years ago

ArchageXIII commented 3 years ago

Only noticed this when I was testing my code on dedicated server that I had set up with player state with the GAS components on it as the example, but guess it applies to yours as well.

When you call PC->CreateHUD(); line 855 GSHeroCharacter.cpp we know we have a valid Player state as it's called from OnRep_PlayerState and we also check the cast.

However On dedicated server I think there is a timing issue and in more real world situation I was getting a lot of occasions where the check for Player state in void AGSPlayerController::CreateHUD() (line 35) GSPlayerCOntroller.cpp was nullptr even tho it was replicated in the base character so the hud would create but not populate with the initial replicated values.

As the calling function was always guaranteed to have player state as it was called OnRep_PlayerState I updated the code to just pass a reference to that player state over to the player controller and not rely on the player controller also replicating at the same time and having it available to set the initial HUD values.

so just updated to

void CreateHud(class AGSPlayerState* PS);

and always passing it then it works every time.

Only small tweak but it was really obscure and seemingly random so thought I'd share just in case it was worth updating and it was small update.

I also tweaked the HUD class so you explicitly call start listening for attributes from C++ via blueprint event rather than having it start on HUD construct so I could really make sure initial replication had happened first before listening for more changes.

thanks again for the resource, learning so much from the code base.

ameaninglessname commented 3 years ago

Add a parameter is good, but by the way, why not use PlayerController->GetPawn() and then use pawn->GetPlayerState()? Anyway pawn is retrieved here

tranek commented 3 years ago

Passing a parameter sounds like a good approach to this race condition. Feel free to make a PR with that change if you have the spare cycles.