sinbad / SPUD

Steve's Persistent Unreal Data library
MIT License
300 stars 44 forks source link

BeginPlay called after Restore ? #74

Closed DanSteph1024 closed 2 months ago

DanSteph1024 commented 2 months ago

Dear Steve,

First thanks for this wonderful plugin.

My problem: I have an AActor class that need saved variable in begin play(). Maybe I'm doing something wrong, but actually my values are not ready in BeginPlay and are okay later in Tick. SpudRestoreCustomData_Implementation is also called after my BeginPlay().

I found a temporary solution which seem to work, but I don't know if it's valid and does not break something ? -Deffer spawn actor in USpudState::RespawnActor -Finish Spawning in USpudState::RestoreActor

SpudState.cpp USpudState::RespawnActor 508 // Important to spawn using level's world, our GetWorld may not be valid it turns out 509 auto World = Level->GetWorld(); 510 Params.bDeferConstruction = true; // <----Modification here to deffer spawning 511 AActor* Actor = World->SpawnActor<AActor>(Class, Params);

SpudState.cpp USpudState::RestoreActor 613 RestoreCoreActorData(Actor, ActorData->CoreData); 614 RestoreObjectProperties(Actor, ActorData->Properties, LevelData->Metadata, RuntimeObjects); 615 PostRestoreObject(Actor, ActorData->CustomData, LevelData->GetUserDataModelVersion()); 616 Actor->FinishSpawning(FTransform()); // <----Modification here to finish spawning

Is this correct ? If yes, I plan to add a parameter so only choosen AActor have this load order.

Best regards,

Dan

sinbad commented 2 months ago

Yes unfortunately BeginPlay is a bit problematic with loading. Instead you should have a separate Init function which you make sure can only be run once (make it dependent on a bool which is NOT saved), and call it from both BeginPlay (for non-loaded cases) and ISpudObjectCallback::SpudPostRestore (for loaded cases). Then you can be sure you're initialising with all the loaded data ready.

DanSteph1024 commented 2 months ago

That's clever, thanks for the tips. Yet it work without problem with my solution. The only minor inconvenient is that "FinishSpawning" issue a warning in editor and it's a private Unreal source so I can't re-use only the useful part of FinishSpawning without modifying unreal's source.

Actor.cpp AActor::FinishSpawning 3832 if (ensure(!bHasFinishedSpawning)) // Issue a warning the first time it run in editor

I'll work on a complete solution during one week to get everything clear and solid and come back to give feedback and close this issue. If you think about something during this time, please let me know.

DanSteph1024 commented 2 months ago

Solution to have BeginPlay() of dynamic object called AFTER data are restored

Simple and work flawlessly in editor and packaged now.

Modify in SpudState.cpp USpudState::RespawnActor 508 // Important to spawn using level's world, our GetWorld may not be valid it turns out 509 auto World = Level->GetWorld(); 510 Params.bDeferConstruction = true; //<--Line added here to deffer spawning 511 AActor* Actor = World->SpawnActor<AActor>(Class, Params);

Modify in SpudState.cpp USpudState::RestoreActor 612 PreRestoreObject(Actor, LevelData->GetUserDataModelVersion()); 613 RestoreCoreActorData(Actor, ActorData->CoreData); 614 RestoreObjectProperties(Actor, ActorData->Properties, LevelData->Metadata, RuntimeObjects); 615 if(bRespawned==true) //<--Line added here 616 Actor->FinishSpawning(FTransform()); //<--Line added here 617 PostRestoreObject(Actor, ActorData->CustomData, LevelData->GetUserDataModelVersion());

Note: I had the editor warning because I didn't checked if it was bRespawned, now it's okay.

DanSteph1024 commented 2 months ago

...

sinbad commented 2 months ago

Thanks; if you could submit a Pull Request that would be ideal, it'll be easier for me to apply & then you'd get credit for it in the repo.

DanSteph1024 commented 2 months ago

WILCO Sir, Despite I started to code in 1985 I'm fairly new to GitHub, and after 12 hours of coding today (and counting) my brain start to melt, can you just point me in the right direction to do so ? Should I create a fork first or other ?

Much thanks

sinbad commented 2 months ago

Yeah you'll want to create a fork first, then follow this guide: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork

No problem if it's too much trouble though, it's just nice for people to get proper credit for their contributions.

DanSteph1024 commented 2 months ago

Much thanks. I'll probably take a few days because after one year of work I'm in save/restore game process and I might discovers/add some more things. I'd like to add named variable or something similar which would be more update proof for my need. It's a building game, and build part as well as chest content with backpack inventory save/restore well. Very happy with your plugin.