Prototype Standardize a method to gracefully handle missing files

This is more of an epic, and surely will be a recurring theme throughout the project...

Given how data-driven and customizable this game is intended to be, we can expect that files will sometimes be missing, even if they're part of our own distribution. For example I ran into an issue when switching branches where some fonts or at least their import files were missing, and this brought down half of the render loop. Whenever we read a file we should assume that it may fail, and try to deal with it. Depending on the file, this may include any combination of:

explicitly check the result before continuing
use the null conditional operator ?. to access it
LOG IT at a high level (this should eventually be shown in-game)
default to a placeholder/empty reference at the lowest level possible
disable the feature
if it's absolutely critical, abort back to the nearest safe state

Nov 20 '24 05:11 WildWeazel

I was looking into this a bit. What is the intended scope of this issue? Is it for files (I see references to new FileIniDataParser().ReadFile(SETTINGS_FILE_NAME)), fonts (I see references to ResourceLoader.Load<FontFile>(...) , etc.?

What is the desired end-state here? Is it to have one file reader class that delegates to interface implementations for different file types? e.g. an ini implementation, a font implementation, an image implementation, etc. with methods for handling missing files that can be used for other core logic? Does it include updating all the places that depend on file reading or can they be refactored out over time?

Sep 05 '25 18:09 mwhiter

Here's my interpretation of the requirements:

We want a file reader that is both safe, and generic. In the event of a failure, default to some value passed in, and inform the client of the error so they can potentially handle it.
Unsure how we'd have a generic function handle "disable feature" or "if it's absolutely critical, abort back to the nearest safe state", because this is highly context specific, so it's probably best to delegate to the clients on how to handle these failures appropriately.

Here is an implementation I whipped up that is extensible to more file types and handles the requirements to the best of my knowledge. However, given C#'s explicit typing requirements, I'd find it difficult to make this smarter than it currently is, but open to suggestions.

This is untested, looking for feedback on approach.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Serilog;

namespace C7Engine.FileReader {
	public class C7FileReader {
		private readonly List<IC7FileReader> _fileReaders = new();
		private ILogger log = Log.ForContext<C7FileReader>();

		public C7FileReader() {
			_fileReaders.Add(new IniFileReader());
		}

		public FileReadResult<T> ReadFile<T>(
			string fileName,
			Func<T> defaultFactory
		) {
			if (!File.Exists(fileName)) {
				log.Error($"File {fileName} does not exist");
				return new FileReadResult<T>(defaultFactory(), FileReadResultType.FileDoesNotExist);
			}

			IC7FileReader fileReader = DetermineFileReader(fileName);
			if (fileReader == null) {
				log.Error($"No valid file readers available for {fileName}!");
				IEnumerable<string> readers = _fileReaders.Select(reader => reader.GetFileType());
				log.Error($"Allowed values are: {string.Join(", ", readers)}");
				return new FileReadResult<T>(defaultFactory(), FileReadResultType.FileReaderNotDefined);
			}

			if (fileReader.ValidReturnType() != typeof(T)) {
				log.Error($"Return Type {typeof(T)} not valid for {fileName} (expected: {fileReader.ValidReturnType()})!");
				return new FileReadResult<T>(defaultFactory(), FileReadResultType.InvalidClassCast);
			}

			try {
				T file = fileReader.ReadFile<T>(fileName);
				return new FileReadResult<T>(file, FileReadResultType.Success);
			} catch (Exception ex) {
				log.Error($"Error reading file {fileName}: {ex.Message}");
				return new FileReadResult<T>(defaultFactory(), FileReadResultType.FailedToRead);
			}
		}

		private IC7FileReader DetermineFileReader(string fileName) {
			return _fileReaders.FirstOrDefault(fileReader => fileReader.CanReadFile(fileName));
		}
	}
}

using System;

namespace C7Engine.FileReader;

public interface IC7FileReader {
	public string GetFileType();
	bool CanReadFile(string fileName);
	Type ValidReturnType();
	T ReadFile<T>(string fileName);
}

using System;
using System.IO;
using IniParser;
using IniParser.Model;

namespace C7Engine.FileReader;

public class IniFileReader : IC7FileReader {
	private readonly FileIniDataParser _dataParser = new();

	public string GetFileType() {
		return ".ini";
	}

	public bool CanReadFile(string fileName) {
		return Path.GetExtension(fileName).Equals(".ini", StringComparison.OrdinalIgnoreCase);
	}

	public Type ValidReturnType() {
		return typeof(IniData);
	}

	public T ReadFile<T>(string fileName) {
		IniData data = _dataParser.ReadFile(fileName);
		if (data is T typed) return typed;
		throw new InvalidCastException($"Expected to cast to {ValidReturnType()}, but was {typeof(T)} instead.");
	}
}

namespace C7Engine.FileReader;

public record FileReadResult<T>(T Value, FileReadResultType ResultType);

public enum FileReadResultType {
	Success,
	InvalidClassCast,
	FileReaderNotDefined,
	FileDoesNotExist,
	FailedToRead,

}

Plugging that code above could do this: Old code

public static Dictionary<string, string> getINIAnimationsInfo(string iniPath) {
			FileIniDataParser UnitIniFile = new FileIniDataParser();
			IniData UnitIniData = UnitIniFile.ReadFile(iniPath);
			var tr = new Dictionary<string, string>();
			foreach (UnitAction actn in Enum.GetValues(typeof(UnitAction))) {
				var fileName = UnitIniData["Animations"][actn.ToString()];
				if ((fileName != null) && (fileName != ""))
					tr[actn.ToString()] = fileName;
			}
			return tr;
		}

New code

public static Dictionary<string, string> getINIAnimationsInfo(string iniPath) {
			C7FileReader fileReader = new();
			IniData UnitIniData = fileReader.ReadFile(iniPath, () => new IniData()).Value;
			var tr = new Dictionary<string, string>();
			foreach (UnitAction actn in Enum.GetValues(typeof(UnitAction))) {
				var fileName = UnitIniData["Animations"][actn.ToString()];
				if ((fileName != null) && (fileName != ""))
					tr[actn.ToString()] = fileName;
			}
			return tr;
		}

Some advantages I can think of:

Single source of truth for file-reading logic. Can just search for C7FileReader and get all file reads in the application.
Extensible to generic file types, with implementations that can do anything (look at the resource loader, or use the native File API)
if the client really needs to know why something failed, they can using FileReadResult.ResultType.
Implementations of file-validation logic can be more complex either now or later (example, can rely on validating magic bytes for .png and use the interface to implement)

Sep 05 '25 20:09 mwhiter

@WildWeazel or @esbudylin can correct me here, but I wonder if this might make more sense to do closer to the level of the lua configs.

For example, many of the textures are loaded here: https://github.com/C7-Game/Prototype/blob/8b7643a1641ac43ca25ec0d6b1efc09df16c4d78/C7/TextureLoader.cs#L122, or at some of the other Load* methods in that file.

These are mostly the things that can fail to load, like a missing unit, or a missing texture.

If it was closer to the file loading, the actual error we usually get is in this logic, where we decide how to resolve a relative path: https://github.com/C7-Game/Prototype/blob/8b7643a1641ac43ca25ec0d6b1efc09df16c4d78/C7/Util.cs#L111

Sep 08 '25 01:09 TomWerner