`ISpudObject` should allow opting-out of saving Transform data

[error454]

In researching use of this plugin in our project, one particular issue keeps coming up. I believe that there should be a way to opt out of storing transform data for any implementor of ISpudObject.

The first use case where storing the transform does not make sense is the case of a static actor. For instance, a blueprint with 1 or more static mesh components set to static mobility. This is commonly done on blueprint actors where you want baked lighting, but is not exclusive to that. Imagine as an example, a checkpoint altar that turns on a particle when the checkpoint is reached with static mesh having baked lighting.

In the past (and I believe still currently), the engine prevented you from moving an actor containing any subcomponent with static mobility. So saving the transform is not helpful at best, and at worst, if the engine let you change the transform it would break your baked lighting.

The second use case I see is any actor whose design-time placement is deliberate and critical. For instance, an actor that defines a spawn point for enemies. This actor would be placed throughout a level at design time and would have a SaveGame variable that tracks whether the spawner has been defeated.

Developers are going to need to shift these over time as production continues, landscapes change and props are added. Unfortunately, if a user's save game automatically restores the transform of these actors, you're now undoing deliberate design. Say a rock was added directly on top of a spawner and the dev nudged the spawner to be on top of the rock, but when the game is loaded, the spawner is restored to be inside of the rock!

"Hmm, works on my end, these users must be crazy." 🤣

I believe a simple way to facilitate this would be to add a new function to either ISpudObject or ISpudObjectCallback e.g. bool ShouldSaveTransform() whose default implementation returns true but allows opting out of saving transforms. No doubt this would add a little bit of complexity to versioning, particularly if someone were to opt-in to saving transforms in a later version of the save file.

Would a change like this be in line with your goal of this plug-in?

[sinbad]

Thanks for raising this, a good point. I propose:

1. Static actors never have their transform restored
2. ISpudObject::ShouldRestoreTransform (default true)

The transform will always be stored, but not always applied on restore. This is handy because it doesn't require a breaking change to the save format, and it also allows you to change your mind about this later and still have data to use.

I've been considering adding support for special tags on actors so that you don't have to use C++ to change the GetSpudRespawnMode behaviour, and this would help with this too. Something like if an actor has a tag named "SPUDNoTransform" then the transform isn't restored. Then you have the option of doing it in C++ or just via a tag.

[error454]

1. Static actors never have their transform restored

This would work and it seems that checking mobility of the root component would be sufficient to determine overall mobility. This seems to be exactly what SetActorLocation does via USceneComponent::MoveComponentImpl using CheckStaticMobilityAndWarn

EDIT To follow up on my previous statement:

if the engine let you change the transform it would break your baked lighting

Because of CheckStaticMobilityAndWarn, setting actor transforms on static objects ends up being a no-op. So this is absolutely not a concern. If you're in editor it prints a warning but no-ops in production.

I've been considering adding support for special tags on actors

If you're considering this path, I wonder if GameplayTag's could be a fit. They're a little more structured than actor tags, so SPUD could provide a set of static tags in its own namespace and there'd be no possibility of typing the wrong tag name. This would mean ISpudObject would probably need to inherit from IGameplayTagAssetInterface and actors wanting to save state using the tag system would need to create an FGameplayTagContainer and return it in said interface.

Every project I work on uses GameplayTags already, so it would be low friction in my world 🤷. Some good refs:

• Official Docs
• Tom Looman Article

If we can imagine a set of bools that would control various aspects of Saving/Loading State then GameplayTags seem like a really great fit. No need to implement a bunch of interface calls in each actor, just toggle a bunch of tags. . . If, however, there's only 1 boolean option and a mishmash of other options (ints, floats, etc) then I suppose it's less of a win 🤔.

[sinbad]

Because of CheckStaticMobilityAndWarn, setting actor transforms on static objects ends up being a no-op. So this is absolutely not a concern. If you're in editor it prints a warning but no-ops in production.

I still think it's worth doing, to avoid the editor warning and to be a good citizen, since you're not supposed to do this.

If you're considering this path, I wonder if GameplayTag's could be a fit.

Although I'm aware of them I've not needed Gameplay tags so far. I had thought it would require an extra module to be imported but it looks like it doesn't. I also don't want to force people to register SPUD tags in their project settings, but it looks like I could register SPUD tags at subsystem init time via AddNativeGameplayTag? If so then yeah that could be a good option.

[error454]

it looks like I could register SPUD tags at subsystem init time via AddNativeGameplayTag?

For this system, I would recommend that SPUD has its own Config/<GreatFileName>.ini where tags are maintained. At plugin init, it would register the ini file with the gameplay tags manager.

James Baxter has a good example of how to register this in the plugin module startup lifecycle.

You can also see an example of this in the new modular game features plugin with a different entry point.

[sinbad]

Thanks for the info! I think I prefer the tags being code constants though, there are unlikely to be many and it's nice not to have things externalised unless there's a good reason (like designer additions, which doesn't apply here).

I'll take a look at implementing this in a few days.

[sinbad]

I've just realised that SPUD already doesn't apply the transform for static actors, so that's good, we're more well behaved than I thought 😁:

const auto RootComp = Actor->GetRootComponent();
if (RootComp && RootComp->Mobility == EComponentMobility::Movable)
{
    // Only set the actor transform if movable, to avoid editor warnings about static/stationary objects
    Actor->SetActorTransform(XForm, false, nullptr, ETeleportType::ResetPhysics);

[sinbad]

OK I've implemented the option to disable transform restoration (and also velocity restoration) in C++. I'll look at gameplay tags probably next week.

    /// Return whether this object should restore its transform from the save data, if it's Movable
    /// You can override this to false if you want this object to always retain its level location on restore.
    /// This can only be changed in C++ implementations and not Blueprints since they don't support this default impl
    virtual bool ShouldRestoreTransform() const { return true; }

    /// Return whether this object should restore its velocity from the save data. Only applies if it's Movable, has opted
    /// in to restoring transform, and has either physics sim enabled, or a movement component.
    /// You can override this to false if you want this object to manage its own velocity on load.
    /// This can only be changed in C++ implementations and not Blueprints since they don't support this default impl
    virtual bool ShouldRestoreVelocity() const { return true; }

[sinbad]

I started blocking out the Gameplay Tags thing, and FYI since I've realised you do in fact need to import the GameplayTags module in C++, I've made this a private dependency and not exposed any GT types in my API. Otherwise, anyone using SPUD in C++ would get build errors unless they added this to their own Build.cs.

Instead, I've registered them privately and exposed just the names to C++. Also to Blueprints, although you don't really need them there since they appear in the tag list:

This is in a separate branch right now since it's unfinished, will finish & merge next week.

[error454]

This is in a separate branch right now since it's unfinished, will finish & merge next week.

I'm happy to kick the tires on it once you have the basic implementation completed. Having a project that uses gameplay tags and ability systems heavily might help reveal some edge cases.

[error454]

Just a note that setting the InitialSpeed in ASPUDExamplesProjectile gives the false impression that this feature is not working correctly. When a projectile is initially created, if you opt-out of restoring velocity, you inherit the initial speed of the projectile.

[sinbad]

Just a note that setting the InitialSpeed in ASPUDExamplesProjectile gives the false impression that this feature is not working correctly. When a projectile is initially created, if you opt-out of restoring velocity, you inherit the initial speed of the projectile.

Hmm I think that's OK? The idea being that if you opt out of velocity restoration, it does whatever it would normally do by default without the state system changing it, which is this? If someone wanted to control that further that's a post-restore call.

[error454]

Hmm I think that's OK?

Absolutely, I just wanted to call it out because I initially thought things were not working because I expected restored projectiles to drop to the floor with no velocity.