X2CommunityCore / X2ModBuildCommon

An improved XCOM 2 mod build system
MIT License
5 stars 5 forks source link

Build fails for mods with spaces in their solution name #55

Closed chrishayesmu closed 3 years ago

chrishayesmu commented 3 years ago

This might be a known issue, or maybe the community knows that you shouldn't do this for other reasons, but I stumbled on it for a while.

Problem

In build_common.ps1, the mod name is used both as the actual name of the mod, and to build the path to the mod source files. There is a discrepancy here, because the mod name in XCOM 2 can't include any spaces, but ModBuddy and the default project build logic are perfectly okay with spaces.

For example, suppose your project name is "My Mod". If you set up X2ModBuildCommon with the mod name "My Mod", it won't end up building your script files because make -nopause -mods "My Mod" is invalid. If you use the mod name "MyMod", then X2ModBuildCommon won't be able to find your source directory and .x2proj file at all.

Proposed solution

I've fixed this for myself locally by stripping spaces from the mod name where needed, but I'm not using a bunch of functionality (like shaders) so I can't confirm if my solution works everywhere. Alternatively, if there's other tools that will have problems in this scenario, just mentioning not to have spaces in the README might be good enough.

robojumper commented 3 years ago

I think this behavior is inherited from the original powershell scripts (https://github.com/jammerware/x2mods-dev-scripts/). Spaces aren't the only problematic characters though. Here's what the Firaxis ModBuddy plugin does:

List<char> badFileNameChars = new List<char>();
badFileNameChars.AddRange(Path.GetInvalidFileNameChars());
badFileNameChars.Add(' ');
if (!badFileNameChars.Contains(';'))
{
    badFileNameChars.Add(';');
}
string safeName = GetProjectProperty("Name");
safeName = string.Concat(safeName.Split(badFileNameChars.ToArray(), StringSplitOptions.RemoveEmptyEntries));
// ...
base.BuildProject.SetGlobalProperty("SafeName", safeName);

The project generator in the ModBuddy plugin does the same. This of course also isn't sufficient to deal with all project names users come up with -- a popular candidate is [WOTC] My Mod Name, which will end up with a SafeName of [WOTC]MyModName.

It would be useful to have an overview over where the non-safe name is actually used. To my knowledge, it's:

I think we should recommend that the mod and project name are the same and match ^[A-Za-z][A-Za-z0-9_]*$, but add this sort of canonicalization to the build script and use the full name for these paths only.

robojumper commented 3 years ago

The GetInvalidFileNameChars part doesn't actually make a whole lot of sense to me though, because it's not like we can end up with file or folder names that have any of these characters...