sinbad / SPUD

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

Controllers are incorrectly respawned #63

Closed krojew closed 10 months ago

krojew commented 10 months ago

Looks like stateful controllers should be added to USpudState::ShouldRespawnRuntimeActor to the ESpudRespawnMode::Default branch. Right now, controllers are being respawned by default, which results in double controllers present. In fact, stateful controller support is quite problematic in SPUD - since a controller can be spawned by an actor already present on a level, but also by a runtime spawned one, there is no easy solution to store one. Guid cannot be used, since it needs a consistent initial value. Name override is also hard to use, unless it's made to depend on something else.

sinbad commented 10 months ago

Hmm, I've never stored state on a controller, convention is that they're stateless (hence the M part of the MVC pattern). The design idea behind controllers is that they can be swapped out. Is there a particular reason you're making them hold state?

krojew commented 10 months ago

They store some state related to AI, which needs to be restored. What workaround would you suggest?

sinbad commented 10 months ago

Personally I'd put that state on the actor rather than the controller and just have the controller use it from there. But you can control the respawn behaviour by overriding ISpudObject::GetSpudRespawnMode on your controller class.

sinbad commented 10 months ago

I'd also accept a PR for the USpudState default behaviour if that makes sense. I'm not working on SPUD much right now so I haven't pondered the full ramifications but I can see it might make sense.

krojew commented 10 months ago

I'll probably add a PR in the future.

On a side note - seems like arrays of actor references require the actors to have SpudGuid and don't use OverrideName(). Is this by design? I'm trying to figure out how to store a reference to player pawn, which uses OverrideName per the manual.

sinbad commented 10 months ago

I think I just haven't come across that combo before, thanks for the PR.