New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary loading mod exceptions #13367

Merged
merged 1 commit into from Jun 29, 2017
Jump to file or symbol
Failed to load files and symbols.
+32 −20
Diff settings

Always

Just for now

@@ -80,7 +80,7 @@ void LoadMod(MiniYaml yaml)
mod.Icon = sheetBuilder.Add(bitmap);
}
mods.Add(ExternalMod.MakeKey(mod), mod);
mods[ExternalMod.MakeKey(mod)] = mod;
}
internal void Register(Manifest mod)
View
@@ -375,7 +375,10 @@ public static void InitializeMod(string mod, Arguments args)
ModData = null;
if (mod == null)
throw new InvalidOperationException("Game.Mod argument missing or mod could not be found.");
throw new InvalidOperationException("Game.Mod argument missing.");
if (!Mods.ContainsKey(mod))
throw new InvalidOperationException("Unknown or invalid mod '{0}'.".F(mod));
Console.WriteLine("Loading mod: {0}", mod);
@@ -69,26 +69,38 @@ Manifest LoadMod(string id, string path)
try
{
if (!Directory.Exists(path))
throw new InvalidDataException(path + " is not a valid mod package");
{
Log.Write("debug", path + " is not a valid mod package");
return null;
}
package = new Folder(path);
if (!package.Contains("mod.yaml"))
throw new InvalidDataException(path + " is not a valid mod package");
using (var stream = package.GetStream("icon.png"))
if (stream != null)
using (var bitmap = new Bitmap(stream))
icons[id] = sheetBuilder.Add(bitmap);
return new Manifest(id, package);
if (package.Contains("mod.yaml"))

This comment has been minimized.

@pchote

pchote May 29, 2017

Member
if (!package.Contains("mod.yaml"))
	return null;

would be more concise.

You can put the package disposal inside a finally block to make sure it gets cleaned up on these early returns.

@pchote

pchote May 29, 2017

Member
if (!package.Contains("mod.yaml"))
	return null;

would be more concise.

You can put the package disposal inside a finally block to make sure it gets cleaned up on these early returns.

This comment has been minimized.

@rob-v

rob-v May 29, 2017

Contributor

This change + move disposal to finally isn't sufficient, because we want disposal on error, missing mod.yaml, but not on valid package so it would need more refactoring and the end result might be less concise than now.

@rob-v

rob-v May 29, 2017

Contributor

This change + move disposal to finally isn't sufficient, because we want disposal on error, missing mod.yaml, but not on valid package so it would need more refactoring and the end result might be less concise than now.

{
var manifest = new Manifest(id, package);
if (package.Contains("icon.png"))
{
using (var stream = package.GetStream("icon.png"))
if (stream != null)
using (var bitmap = new Bitmap(stream))
icons[id] = sheetBuilder.Add(bitmap);
}
else if (!manifest.Metadata.Hidden)
Log.Write("debug", "Mod '{0}' is missing 'icon.png'.".F(path));
return manifest;
}
}
catch (Exception)
catch (Exception e)
{
if (package != null)
package.Dispose();
return null;
Log.Write("debug", "Load mod '{0}': {1}".F(path, e));
}
if (package != null)
package.Dispose();
return null;
}
Dictionary<string, Manifest> GetInstalledMods(IEnumerable<string> searchPaths, IEnumerable<string> explicitPaths)
@@ -100,9 +112,6 @@ Manifest LoadMod(string id, string path)
foreach (var pair in candidates)
{
var mod = LoadMod(pair.First, pair.Second);
// Mods in the support directory and oramod packages (which are listed later
// in the CandidateMods list) override mods in the main install.
if (mod != null)
ret[pair.First] = mod;
}
ProTip! Use n and p to navigate between commits in a pull request.