ArticySoftware / Articy3ImporterForUnreal

Articy Importer plugin for the Unreal Engine 4 and Unreal Engine 5 (work in progress).
MIT License
98 stars 39 forks source link

Crash when executing Kismet Event Commands #68

Closed DarioVangelista closed 2 years ago

DarioVangelista commented 2 years ago

Hello, we have encountered a crash on UE 4.27.1 when executing a Kismet Event Command. We are using v1.3.1 of the importer.

It happens in both our project and the ManiacMansion demo.

Repro steps:

From what I understand, the ArticyPackage needs to provide an implementation of UWorld* GetWorld() const;

This is blocking other people on the team from using Kismet Event Commands which is a problem. I have avoided the crash locally by changing the implementation of UArticyBaseObject::GetWorld() to always return nullptr, but this is clearly not an actual fix and I have no idea how many things it might be breaking in Articy.

What should the actual implementation of UArticyPackage::GetWorld() be?

Thanks, Dario @ YellowBrickGames

Articy_Crash

.

peter-sabath commented 2 years ago

Hi Dario,

thanks for reporting this.

We think that the implementation in UArticyPackage::GetWorld() should be

UWorld* UArticyPackage::GetWorld() const
{
    return GetOuter() ? GetOuter()->GetWorld() : nullptr;
}

We will do some further checks for other unwanted side-effects but are confident that this would solve the problem.

DarioVangelista commented 2 years ago

Hi Peter, I tried your change and unfortunately it doesnt change the result.

I have UArticyBaseObject::GetWorld() as in the original code: UWorld* GetWorld() const override { return GEngine->GetWorldFromContextObjectChecked(GetOuter()); }

And UArticyPackage::GetWorld() as you proposed: UWorld* GetWorld() const override { return GetOuter() ? GetOuter()->GetWorld() : nullptr; }

Thanks, Dario @ YellowBrickGames

Articy_Crash_2
christian-schildt commented 2 years ago

Hi Dario,

thanks for the further information.

As far as I can tell, your workaround to always return nullptr should be fine for now.

It seems like that overriding GetWorld() in our plugin is no longer needed. A few quick tests confirmed this. Probably, removing GetWorld() will be our solution. But won’t publish the change until the new year.