Prefab Tool - Prefab Support for Unreal Engine

[ColdIronPoz]

[MENTION=702]Nate[/MENTION],

Thanks for the fix! We've deployed it and is seems to be working well.

One unrelated thing: The prefab plugin doesn't compile properly in non-unity builds. Mostly it's missing some headers in cpp files. But a harder fix is that PrefabComponentDetails.cpp calls FPrefabAssetEditorModule::ShowHidePrefabActor. But that class and function is only defined in PrefabAssetEditorModule.cpp. I think the class will need to be hoisted up to a header for it to compile.

(We're preparing our code for IWYU, so we're doing some non-unity builds to make sure we're doing the proper includes.)

[Nate]

1.0.3.1 update live on itch.io. Unreal marketplace update submitted.

1.0.3.1 Change Log

1.0.3.1
------------------
* Fix potential editor crash due to name clash when reverting prefab Actor in multi-levels map (thanks ColdIronPoz for the report and fix)
* Fix apply prefab actor only works if world's current level is the same as prefab actor in multi-levels map
* Drop 4.14 support

[Nate]

[MENTION=221987]ColdIronPoz[/MENTION], thanks a lot for the reproduction case and the bug hunting! You're absolutely right about the root cause of the bug: the wrong assumption of current level of world is the same as the prefab actor's outter level.

I'm preparing a hot-fix release now.

[ColdIronPoz]

[MENTION=702]Nate[/MENTION],

I dug around a bit in the code and I think I may have found the problem. I'm not very knowledgeable about this stuff in Unreal , so I might be off base.

The intent of RevertPrefabActor is to replace an existing prefab actor's hierarchy with the one defined in the asset. To do this, it collects a bunch of information about the particular instance and then calls SpawnPrefabInstances to get a new hierarchy to replace the old one. Eventually, ReplacePrefabInstances is called to rename the new actors to their original names. Although the rename code here looks good, somehow a name conflict happens and it crashes.

Although the rename code in ReplacePrefabInstances looks clean, it still dies when using multiple sublevels. When debugging, I noticed that the new actors weren't in the expected level. They were in the base map, not the sub level.

So, I went to look at how they were spawned. SpawnPrefabInstances calls edactPastSelected, which pastes the new actors into the World's current level. So, I decided to set the World's current level before calling SpawnPrefabInstances. Here is my hack/fix:

at approximately PrefabToolHelpers.cpp:1804

			// Spawn prefab instances
			TArray<AActor*> SpawnInstances;
			TArray<AGroupActor*> NewGroupActors;
			PrefabActor->GetLevel()->OwningWorld->SetCurrentLevel(PrefabActor->GetLevel());
			SpawnPrefabInstances(Prefab, PrefabActor->GetLevel()->OwningWorld, SpawnInstances, &NewGroupActors);
			PostSpawnPrefabInstances(Prefab, SpawnInstances, PrefabActor, &NewGroupActors);

This seems to keep the crash from happening. Previously, the new objects were all going into the base map but I think MakeUniqueObjectName makes a unique name in the outer of the old actor. But the place we're pasting into isn't in that context. So we can step on our own toes. This poited out that those actors might not have been created in the right level anyway. So, by fixing the level they are spawned in, it fixes the problem.

At least that's the theory. :-) Let me know if there's a more appropriate fix or if I'm just off-base.

Cheers,
poz

[ColdIronPoz]

[MENTION=702]Nate[/MENTION], We have a reproduction case for the Rename crash. The problem seems to revolve around sub-levels.

Make sure bUpdateAllPrefabActorsWhenOpenMap = true

1. Create a new map/level. Call it MapA
2. In this level drop an actor and name it TestName.
3. Make this actor into a prefab.
4. Save MapA.
5. Create a new map/level. Call it MapB.
6. Drop a new actor and ALSO name it TestName.
7. Make this actor into a prefab.
8. Save MapB.

At this point, you have two maps each with a prefab that has an actor with the same name as the other. This example has two different prefabs, but if you use the same prefab in each map, I believe you'll get the same result.

9. Create a new map/level. Call it WorldMap.
10. In the Levels window, add in MapA and MapB.
11. Save.
12. Exit and restart (so map load happens and bUpdateAllPrefabActorsWhenOpenMap happens).
13. Crash.

I hope this lets you find the problem!

[CoquiGames]

Thanks!

It's still in planning stage (been distracted from migrating to 4.17 release), I will start the implementation next week.

Good to hear! Thanks!

[Nate]

[MENTION=702]Nate[/MENTION], congratulations on getting Prefab Tool in the Marketplace - I just pick up a copy myself .

Any updates on converting prefab to actor blueprint?

Thanks!

It's still in planning stage (been distracted from migrating to 4.17 release), I will start the implementation next week.

[CoquiGames]

[MENTION=702]Nate[/MENTION], congratulations on getting Prefab Tool in the Marketplace - I just pick up a copy myself .

Any updates on converting prefab to actor blueprint?

[ColdIronPoz]

[MENTION=702]Nate[/MENTION], Thanks! If we get a reproduction case, I'll let you know.

[Nate]

[MENTION=221987]ColdIronPoz[/MENTION], thanks a lot for the call stack!

The logic behind the suspicious line is quite simple:

Step 1. rename old actor to something unique
Step 2. rename new actor to old actor's name

The only reason that Step 2 is failed (which causing the crash) is that Step 1 failed silently somehow. My theory is that unique name generator is not reliable during LoadMap call back. (Need more code digging to prove that theory)

My current solution is adding rename test before the actual rename operation to capture possible rename failure in the first place. Will be in next update.

[ColdIronPoz]

[MENTION=702]Nate[/MENTION]: We did get around the crash by toggling that option in the ini file.
We renamed the actors so the map no longer crashes, so I don't have a handy reproduction for you. I'm not entirely sure how it got into the state. It hasn't happened a second time (yet?) :-)

Here's the call stack. It's from the previous version.

Fatal error: [File:D:\src\unrealengine\branches\dev\Engine\Source\Runtime\CoreUObject\Private\UObject\Obj.cpp] [Line: 197] 
Renaming an object (SpotLight /Game/Maps/Rooms/FRE/Corridors/FRE_CO_4_Circle_A.FRE_CO_4_Circle_A:PersistentLevel.SpotLight_0) on top of an existing object (SpotLight /Game/Maps/Rooms/FRE/Corridors/FRE_CO_4_Circle_A.FRE_CO_4_Circle_A:PersistentLevel.SpotLight_3) is not allowed


UE4Editor_Core!FDebug::AssertFailed() [d:\src\unrealengine\branches\dev\engine\source\runtime\core\private\misc\assertionmacros.cpp:349]
UE4Editor_CoreUObject!UObject::Rename() [d:\src\unrealengine\branches\dev\engine\source\runtime\coreuobject\private\uobject\obj.cpp:197]
UE4Editor_Engine!AActor::Rename() [d:\src\unrealengine\branches\dev\engine\source\runtime\engine\private\actor.cpp:823]
UE4Editor_PrefabAsset!FPrefabToolEditorUtil::ReplacePrefabInstances() [d:\src\endeavor\endeavor\plugins\prefabtool\source\prefabasset\private\prefabtoolhelpers.cpp:1637]
UE4Editor_PrefabAsset!FPrefabToolEditorUtil::RevertPrefabActor() [d:\src\endeavor\endeavor\plugins\prefabtool\source\prefabasset\private\prefabtoolhelpers.cpp:2001]
UE4Editor_PrefabAsset!FPrefabToolEditorUtil::RevertAllPrefabActorsInCurrentLevel() [d:\src\endeavor\endeavor\plugins\prefabtool\source\prefabasset\private\prefabtoolhelpers.cpp:2266]
UE4Editor_PrefabAssetEditor!TBaseRawMethodDelegateInstance<0,FPrefabAssetEditorModule,void __cdecl(FString const & __ptr64,bool)>::ExecuteIfSafe() [d:\src\unrealengine\distrib\engine\source\runtime\core\public\delegates\delegateinstancesimpl.h:648]
UE4Editor_UnrealEd!TBaseMulticastDelegate<void,FString const & __ptr64,bool>::Broadcast() [d:\src\unrealengine\branches\dev\engine\source\runtime\core\public\delegates\delegatesignatureimpl.inl:937]
UE4Editor_UnrealEd!FEditorFileUtils::LoadMap()

The line is

NewActor->Rename(*OldActorName.ToString());

[Nate]

[MENTION=221987]ColdIronPoz[/MENTION], very sorry about the bug you run into. May I ask which line trigger the crash? And if possible, a test project for me to reproduce the bug would be much appreciated!

1503: OldActor->Rename(*OldActorReplacedNamed.ToString());
1532: NewActor->Rename(*OldActorName.ToString());
1580: OldActor->Rename(*OldActorName.ToString());

If you turn the "Update All Prefab Actors when Open Map" off, then manually click "Apply" in Prefab Actor's detail panel, will that still trigger the bug?

[Rogelio]

Just started using this at work and oh man, it is awesome!! Thank you for the continued support on it too- I was literally going to ask for "Add Select Parent Prefab Actor context menu" today because it was by far one of the biggest pain points when working with prefabs. Thanks again for the awesome tool- really looking forward to seeing what is to come!!

[ColdIronPoz]

[MENTION=702]Nate[/MENTION]
Prefab tools looks really great so far and we're excited to start trying it in our workflow.

We have run into one problem/bug so far. We sometimes get a crash involving renaming objects. At map load, Prefab calls RevertAllPrefabActorsInCurrentLevel(). This eventually goes to ReplacePrefabInstances(), and then Rename. Rename fails because there's already something in the map with the same name. This is a fatal error.

I'm not entirely certain how the map got into this state. The objects in the map with the same name either came from or were the original source of the prefab. We will try to get better reproduction case for you. But one can imagine that even a normal revert (i.e. one by hand) might be a problem if an object with the same name happened to be created.

Cheers,
shannon

[Yun-Kun]

Perfect! This will greatly speed up our workflow

I love the update video, makes me think of a patch spotlight for a game.

Keep up the good work, you rock.