AdditionalPluginDirectories with subdirectories

Hello!

We make use of the AdditionalPluginDirectories feature, and we like to organize our plugins into subdirectories.

This means we have this in our project file:

	"AdditionalPluginDirectories": [
		"../SharedPlugins"
	],

and we have plugins in subdirectories of this path, e.g. /SharedPlugins/OurPlugins/OneOfOurPlugins/OneOfOurPlugins.uplugin or /SharedPlugins/Marketplace/SomeBoughtPlugin/SomeBoughtPlugin.uplugin.

We’ve noticed that the content from those plugins cannot be loaded in a cooked build. Dropping the plugins directly into the /SharedPlugins/… folder or explicitly including all the subfolders so that they are all directly nested inside an AdditionalPluginDirectories entry fixes the issue.

After digging around a bit, it seems the culprit is the handling of paths in FCookSandbox::TryConvertUncookedFilenameToCookedRemappedPluginFilename (and FCookSandbox::ConvertCookedPathToUncookedPath for converting the path back). These functions drop all plugin content from an additional paths directly into the /RemappedPlugins/… root folder without preserving and additional subdirectories. So e.g. content from /SharedPlugins/OurPlugins/OneOfOurPlugins/… will end up in /RemappedPlugins/OneOfOurPlugins/…

The staging code however will preserve the subdirectories, so the mount point for the corresponding plugin will be /RemappedPlugins/OurPlugins/OneOfOurPlugins/…, and consequently package paths will point at the wrong directory.

We’ve fixed this locally by modifying the two functions mentioned above to preserve the subdirectory structure, so that all cooked assets will end up in the correct directories including their subpaths, along the staged files.

We aren’t 100% sure if we are causing other issues with this, but so far it looks good?

Ciao, Daniel!

[Attachment Removed]

Hello! Thanks for digging into this and working on a fix. Would you be open to creating a pull request on GitHub for your change?

I found a couple of areas in the automation tool that have similar expectations/assumptions that might also need updating, if they’re not already considered.

https://github.com/EpicGames/UnrealEngine/blob/5\.8\.2\-release/Engine/Source/Programs/AutomationTool/AutomationUtils/DeploymentContext.cs\#L1239\-L1246

https://github.com/EpicGames/UnrealEngine/blob/5\.8\.2\-release/Engine/Source/Programs/AutomationTool/AutomationUtils/ProjectParams.cs\#L2999\-L3000

[Attachment Removed]

Thank you for digging even further into this, and I completely understand not having the bandwidth to tackle the pull request process.

I’m going to write this up and share it with the relevant engine team for further review and investigation.

They’re best placed to make the decision on the right way to handle the subpaths across the various systems.

[Attachment Removed]

Hi!

Thanks for looking into this!

We didn’t consider those code paths in the automation tool yet, because in our experience the staging part “does everything right”. But the areas you pointed out would also need updating.

I looked into why the staging works right now, because it currently copies the staged files to the full subdirectory path instead of directly into RemappedPlugins/<PluginName>.

The culprit is DeploymentContext::GetStagedFileLocation, which simply does this:

public StagedFileReference GetStagedFileLocation(FileReference InputFile)
	{
		StagedFileReference OutputFile;
		foreach (DirectoryReference AdditionalPluginDir in AdditionalPluginDirectories)
		{
			if (InputFile.IsUnderDirectory(AdditionalPluginDir))
			{
				// This is a plugin that lives outside of the Engine/Plugins or Game/Plugins directory so needs to be remapped for staging/packaging
				// We need to remap C:\SomePath\PluginName\RelativePath to RemappedPlugins\PluginName\RelativePath
				OutputFile = new StagedFileReference(
					String.Format("RemappedPlugins/{0}", InputFile.MakeRelativeTo(AdditionalPluginDir)));
				return OutputFile;
			}
		}

InputFile.MakeRelativeTo will keep the entire subpath alive, so e.g. /AdditionalPlugins/SubPath/PluginName/PluginName.uplugin will end up in /RemappedPlugins/SubPath/PluginName/PluginName.uplugin.

This is also the cause of the disparity between the cooked paths and the paths the game then expects the assets to come from:

  • Cooking will drop the cooked plugin content directly into /RemappedPlugins/PluginName/…
  • The staging step will copy all uncooked staged files into /RemappedPlugins/SubPath/PluginName/…, including the .uplugin file.
  • At runtime, the plugin then gets detected and loaded from /RemappedPlugins/SubPath/PluginName/PluginName.uplugin, and the mount point also points at that location.
  • This then causes all package loading to attempt to load from the mount point including the /SubPath/ bit, where it can’t find anything.

I think it shouldn’t matter if the /SubPath/ is included or not, as long as it is consistent between cooking and staging. Plugin names have to be unique anyway.

Considering that ADDING the subpath would modify 5 or so code locations (I think there’s another one in DeploymentContext::UnmakeRelativeStagedReference), and DROPPING the /SubPath/ should only require modification of that one location in DeploymentContext::GetStagedFileLocation, I am leaning more towards dropping the subpath entirely.

What do you suggest?

As for opening a pull request - theoretically possible. We’ve been wanting to open pull requests for several of our engine changes for a long time. But considering opening a proper pull request means:

  • Extracting the single change from our engine branch
  • Cleaning it up (we sometimes need to sacrifice code quality in order to keep changes more localized and engine-update-proof than they probably should be if they were done by you)
  • Testing it again in isolation
  • Making the pull request, and reacting to feedback
  • And doing all of the above while being a tiny studio with limited resources

I’m not sure we have the bandwidth to do that.

Ciao, Daniel!

[Attachment Removed]

No actually it’s worse, after looking at the code used in the locations you pointed out, they ‘accidently’ keep the entire subpath alive as well.

Given a path like “D:\Project\SharedPlugins\Adopted\CommonGame\CommonGame.uplugin” and “D:\Project\SharedPlugins” being the additional plugin directory:

  • Additional plugin directory is lobbed off, resulting in RemainingPath = “Adopted/CommonGame/CommonGame.uplugin”
  • Then it looks for the first slash, resulting in PluginEndIndex = 7
  • Then it cuts off the first path and assumes it’s the plugin name, giving PluginName = “Adopted”
  • The rest is assumed to be the plugin-relative subpath, giving RemainingPath = “CommonGame/CommonGame.uplugin”
  • The final path is then assembled as “RemappedPlugins/{PluginName}/{RemainingPath}”, which just happens to result in “RemappedPlugins/Adopted/CommonGame/CommonGame.uplugin”

So this just “happens” to work, similar for UnmakeRelativeStagedReference.

ProjectParams::FindPluginRelativePathFromPlatformCookDir does not work like that since it directly grabs the .uplugin filename without extension and slaps it at the end of “RemappedPlugins/”, actually missing the subpath. However this function seems to only be called if cooking DLC, which we’ve never ever done. So I’ll just act as if that is not an issue (see point about limited bandwidth in previous post).

It’s a bit of a mess.

Ciao, Daniel!

[Attachment Removed]

Thanks a lot!

Ultimately I think either way works (stripping out the subdirectories and dropping them directly into RemappedPlugins, or keeping the subdirectories intact); it’s just a choice that should be stuck to uniformly.

Ciao, Daniel!

[Attachment Removed]