Editor modifies objects outside of transactions

Hi there again!

First things first: If my assumptions are wrong this might not be a bug.

The problem:

To my understanding the transaction sytstem should be used as follows:

  1. Start a transaction (they can be nested)
  2. Call modify() on objects that are about to be modified (So they get saved in the transaction)
  3. Set newly created objects to pending kill and call modify on them, then reset the kill flag
  4. Actually modify the objects
  5. Close the transaction

However sometimes the editor modifies relevant UObjects outside of the outermost transaction.
This is bad because there is no way to tell then if an object got modified by a tool and what part got modified which gives people who write editor extensions like myself a hard time.

An example:

The OnBasicShapeCreated Lambda in ComponentTypeRegistry.cpp modifies the material of a component outside of a transaction:

	const auto OnBasicShapeCreated = [](UActorComponent* Component)
	{
		UStaticMeshComponent* SMC = Cast<UStaticMeshComponent>(Component);
		if (SMC)
		{
			const FString MaterialName = TEXT("/Engine/BasicShapes/BasicShapeMaterial.BasicShapeMaterial");
			UMaterial* MaterialAsset = FindOrLoadObject<UMaterial>(MaterialName);
			SMC->SetMaterial(0, MaterialAsset);

			// If the component object is an archetype (template), propagate the material setting to any instances, as instances
			// of the archetype will end up being created BEFORE we are able to set the override material on the template object.
			if (SMC->HasAnyFlags(RF_ArchetypeObject))
			{
				TArray<UObject*> ArchetypeInstances;
				SMC->GetArchetypeInstances(ArchetypeInstances);
				for (UObject* ArchetypeInstance : ArchetypeInstances)
				{
					CastChecked<UStaticMeshComponent>(ArchetypeInstance)->SetMaterial(0, MaterialAsset);
				}
			}
		}
	};

This specific issue can be easily resolved by adding a FScopedTransaction before this bit in SComponentClassCombo.cpp:

				UActorComponent* NewActorComponent = OnComponentClassSelected.Execute(ComponentClass, InItem->GetComponentCreateAction(), InItem->GetAssetOverride());
				if(NewActorComponent)
				{
					InItem->GetOnComponentCreated().ExecuteIfBound(NewActorComponent);
				}

However since all of these issues require a specific fix they can be hard to locate.

How to find issues of this kind:

I found an easy way to check whether an object gets modified outside of a transaction:

  1. Create delegates in the UTransactor class that fire whenever a transaction starts and ends.
  2. Keep track of all the UObjects that get modified during this transaction
  3. At the end of the outermost transaction serialize all UProperties of these UObjects into a StringArray
  4. On Tick compare the StringArray to the current property values of the UObjects by serializing them again
  5. If they mismatch the object got modified outside of the transaction

Fixing these issues would help a great deal in making the UE4 codebase more consistent and more accessible for everyone!

Thanks a lot!

Jannes

Hello JannesNagel,

This seems more like a feature request than a bug report. If you would like these features added to the transaction system, do you have any idea how you would implement them? If so, I would suggest creating a Pull Request on Github. If you do so, a developer will review the changes and decide if we would like to take them or propose any edits that would make it better.

If not, please let me know and I’ll look into getting a feature request put in for it.

We haven’t heard from you in a while, JannesNagel. Are you available to speak about this request? If so, can you respond to my previous comment? In the meantime, I’ll be marking this as resolved for tracking purposes.

I can probably create pull request for individual issues. But due to the nature of the problem one can just not find all the places where the editor does that. Or at least I can not.

Whether this is a bug report or not depends on epics internal coding standards. If you guys want to keep it tight and clean this is indeed a bug. If not - this is just a feature request.

So my hope is just to have someone working on the transaction system take a look over it and decide whether this is something you want to enforce internally or not. Because in really almost all other instances within the editor this was implemented perfectly clean and fixing individual issues is really easy.

So long story short - I just wanted to highlight this issue for you guys - whether or not this is a bug depends on your internal coding standards.

Hello JannesNagel,

I’ve spoken to some developers about this issue and they’ve expressed that making large scale changes that include comparing these values to make sure that this doesn’t happen would be good if it wouldn’t be such a large impact on performance. It seems that this best way to go about these transaction issues would be to get reproduction cases to each individual one that is found and have them fixed as they’re found.

As for the one you’ve presented as an example in this post, I’m not familiar with that area of the code, so would it be possible for you to present a reproduction so that I can enter a bug report for it?