getsentry / sentry-unreal

Unreal Engine
https://docs.sentry.io/platforms/unreal/
MIT License
84 stars 35 forks source link

Plugin Engine Version Number can't be parsed #99

Closed Edstub207 closed 2 years ago

Edstub207 commented 2 years ago

Environment

How do you use Sentry? Sentry.io

Which version of the SDK? Latest from Github Main Releases

How did you install the package? (Git-URL, Assetstore) Manually

Which version of Unreal? 5.0.1

Is this happening in Unreal (editor) or on a player like Android, iOS, Windows? TeamCity Compile job

Steps to Reproduce

  1. Compile the editor

Expected Result

No Warnings

Actual Result

Got a warning: LogPluginManager: Warning: Engine version string in E:/TCWorkDir/213dfa932477ea94/Project/Plugins/External/SentryUnrealPlugin/Sentry.uplugin could not be parsed ("5.0")

Any logs or screenshots

vaind commented 2 years ago

We're currently setting this versionn in the build process, in pack.ps1, to explicitly produce packages that are compatible with multiple engine versions. My assumption when originally implementing this (based on the info found online) was that the version could be specified as partial, i.e. 5.0.x patch version would be compatible if we specify "EngineVersion" 5.0. This seems to have been wrong, based on the following code that checks the compatibility - it will fail if the version is not exactly the same as the current editor version... If it can't be parsed (or is empty), it will say it's compatible 🤔

bool FPluginManager::IsPluginCompatible(const FPlugin& Plugin)
{
    if (Plugin.Descriptor.EngineVersion.Len() > 0)
    {
        FEngineVersion Version;
        if (!FEngineVersion::Parse(Plugin.Descriptor.EngineVersion, Version))
        {
            UE_LOG(LogPluginManager, Warning, TEXT("Engine version string in %s could not be parsed (\"%s\")"), *Plugin.FileName, *Plugin.Descriptor.EngineVersion);
            return true;
        }

        EVersionComparison Comparison = FEngineVersion::GetNewest(FEngineVersion::CompatibleWith(), Version, nullptr);
        if (Comparison != EVersionComparison::Neither)
        {
            UE_LOG(LogPluginManager, Warning, TEXT("Plugin '%s' is not compatible with the current engine version (%s)"), *Plugin.Name, *Plugin.Descriptor.EngineVersion);
            return false;
        }
    }
    return true;
}

This is then used only once (as of v5.0.3 source code):

for (TPair<FString, FPlugin*>& Pair : EnabledPlugins)
{
    FPlugin& Plugin = *Pair.Value;

#if !IS_MONOLITHIC
    // Mount the binaries directory, and check the modules are valid
    if (Plugin.Descriptor.Modules.Num() > 0)
    {
        // Mount the binaries directory
        const FString PluginBinariesPath = FPaths::Combine(*FPaths::GetPath(Plugin.FileName), TEXT("Binaries"), FPlatformProcess::GetBinariesSubdirectory());
        FModuleManager::Get().AddBinariesDirectory(*PluginBinariesPath, Plugin.GetLoadedFrom() == EPluginLoadedFrom::Project);
    }

    // Check the declared engine version. This is a soft requirement, so allow the user to skip over it.
    if (!IsPluginCompatible(Plugin) && !PromptToLoadIncompatiblePlugin(Plugin))
    {
        UE_LOG(LogPluginManager, Display, TEXT("Skipping load of '%s'."), *Plugin.Name);
        continue;
    }
#endif
    Plugin.bEnabled = true;
}

I'm trying to run CI where I've defined the EngineVersion with the zero at the end, e.g. 5.0.0 - let's see how that goes and whether not being compatible has any actual implications.

Maybe we should also revisit this once more and see if the EngineVersion does indeed need to be explicitly specified when uploading to the marketplace.

tustanivsky commented 2 years ago

@vaind Yes, EngineVersion has to be specified explicitly according to Marketplace publishing guide.

Also, I believe adding zero at the end should do the trick here.

Edstub207 commented 2 years ago

@tustanivsky I did try that locally and had no luck during local testing. I've been looking at other things since, but happy to take another look if needed over the next few days.

vaind commented 2 years ago

CI seems to work fine with plugin's EngineVersion '4.27.0' when including in '4.27.2':

LogInit: Engine Version: 4.27.2-17155196+++UE4+Release-4.27
LogInit: Engine Version: 4.27.2-17155196+++UE4+Release-4.27
LogInit: Compatible Engine Version: 4.27.0-17155196+++UE4+Release-4.27
LogInit: Compatible Engine Version: 4.27.0-17155196+++UE4+Release-4.27
Edstub207 commented 2 years ago

CI seems to work fine with plugin's EngineVersion '4.27.0' when including in '4.27.2':

LogInit: Engine Version: 4.27.2-17155196+++UE4+Release-4.27
LogInit: Engine Version: 4.27.2-17155196+++UE4+Release-4.27
LogInit: Compatible Engine Version: 4.27.0-17155196+++UE4+Release-4.27
LogInit: Compatible Engine Version: 4.27.0-17155196+++UE4+Release-4.27

đź‘Ť Will test tomorrow on UE5 to see if I can reproduce. Are you only building for initial releases or are you supporting hotfixes? It's a shame they don't have wildcard for 5.0.X for example by the looks of things - Or a range?

Edstub207 commented 2 years ago

Can confirm it seems to be okay today.

imacs1 commented 1 year ago

Thank you. Works perfectly but 5.1.1 is really 5.1.0.nnnnnn so using 5.1.0, 5.2.0, 5.3.0 etc.

Edstub207 commented 1 year ago

Thank you. Works perfectly but 5.1.1 is really 5.1.0.nnnnnn so using 5.1.0, 5.2.0, 5.3.0 etc.

I'm not sure that's correct - Based on the Build.Version files, they only have Major, Minor, Patch.

imacs1 commented 1 year ago

When I package using the Package Plugin option (rather than the command line options I normally use), I get a warning in the Output Log using 5.1.1 that the version is really 5.1.0.nnnnnn. Switching to 5.1.0 makes the warning go away. Please try packaging from within the running plugin using the built-in UI and check the Output Log.

Notice that there is a warning when staring from Visual Studio, a Warning in the Output Log but the About Screen shows the Version as 5.1.1 once the application starts. Using 5.1.0 in the .uplugin file clears all of these problems. 5 1 1 Version 5 1 1 Warning 5 1 1

imacs1 commented 1 year ago

Similarly for 5.2.1.

I do have both the downloaded and source code builds on my computer. The application might open the file using one version of the engine and package it using another... 5 2 1 Warning

Edstub207 commented 1 year ago

@imacs1 I suspect what's happening is you're packaging on a "promoted build" - EG: Precompiled from Epic Games Store or using the promotion logic in Unreal itself.

I'd expect packaging to be done on an unpromoted build, EG: Compiled locally, which would set that version as 0. The same logic is used when saving asset versions.

Look at https://github.com/EpicGames/UnrealEngine/blob/a3cb3d8fdec1fc32f071ae7d22250f33f80b21c4/Engine/Source/Programs/AutomationTool/BuildGraph/Tasks/SetVersionTask.cs#L61 for context.

imacs1 commented 1 year ago

Thank you!

imacs1 commented 1 year ago

Eddie,

I built the engine from source and still have a problem with version incompatibility after packaging. Would you mind taking this offline? @.**@.>

Thanks, Michael

From: Eddie Stubbington @.> Sent: Monday, September 11, 2023 1:02 PM To: getsentry/sentry-unreal @.> Cc: Michael Lustig @.>; Mention @.> Subject: Re: [getsentry/sentry-unreal] Plugin Engine Version Number can't be parsed (Issue #99)

@imacs1https://github.com/imacs1 I suspect what's happening is you're packaging on a "promoted build" - EG: Precompiled from Epic Games Store or using the promotion logic in Unreal itself.

I'd expect packaging to be done on an unpromoted build, EG: Compiled locally, which would set that version as 0. The same logic is used when saving asset versions.

Look at https://github.com/EpicGames/UnrealEngine/blob/a3cb3d8fdec1fc32f071ae7d22250f33f80b21c4/Engine/Source/Programs/AutomationTool/BuildGraph/Tasks/SetVersionTask.cs#L61 for context.

— Reply to this email directly, view it on GitHubhttps://github.com/getsentry/sentry-unreal/issues/99#issuecomment-1714263829, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFRPYPP3ZUHTFS5QNG25RZLXZ47YBANCNFSM6AAAAAAQKRBYC4. You are receiving this because you were mentioned.Message ID: @.**@.>>

Edstub207 commented 1 year ago

@imacs1 As this appears to be a problem with a different plugin, I'd recommend raising an issue with Epic - I don't work for Sentry or Epic so the above is just based on my experience so far!

imacs1 commented 1 year ago

It’s my plugin and not a 3rd party plugin. I’m trying to publish new versions to the marketplace and need a little guidance creating plugins compatible with each version of the engine (5.1, 5.2 and 5.3). I’ve been successful until now. I did what you suggested and generated the plugin from a source code build of UE. It does change the minor revision to 0 (as you anticipated) in the packaged plugin (e.g. 5.2.1 becomes 5.2.0 in the package). Unfortunately, each plugin won’t load with the corresponding production Unreal Engine version due to a incompatibility issue that I don’t know how to fix. I’ve tried generating the plugins both from the UI and the command line without success. It seems that you understand this issue way better than I do. Thanks!

From: Eddie Stubbington @.> Sent: Tuesday, September 12, 2023 1:37 PM To: getsentry/sentry-unreal @.> Cc: Michael Lustig @.>; Mention @.> Subject: Re: [getsentry/sentry-unreal] Plugin Engine Version Number can't be parsed (Issue #99)

@imacs1https://github.com/imacs1 As this appears to be a problem with a different plugin, I'd recommend raising an issue with Epic - I don't work for Sentry or Epic so the above is just based on my experience so far!

— Reply to this email directly, view it on GitHubhttps://github.com/getsentry/sentry-unreal/issues/99#issuecomment-1716154463, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFRPYPLOXTH2YQ4INUANBVDX2CMR7ANCNFSM6AAAAAAQKRBYC4. You are receiving this because you were mentioned.Message ID: @.**@.>>