Code Style & Commit Guidelines - Tkachov/Overstrike GitHub Wiki

Commit messages

There are no strict rules here. Just use common sense.

Some recommendations, mostly to keep messages more or less consistent with existing ones:

  • start with lowercase;
  • use present tense ("fix bug", not "fixed bug");
  • don't write extended message if first line explains it clearly enough already;
  • for changes in Modding Tool or Localization Tool, use short prefixes (like "MT: bump to 1.3.0" or "LT: fix bug"). No prefix needed for changes in Overstrike and common changes (DAT1, GDeflateWrapper, or related changes that are in Overstrike and other tools at the same time).

Commit contents

  • use common sense when deciding what to commit;
  • try to make commits atomic: relatively small and only containing related changes;
  • if you change a lot of formatting, don't mix that with functionality changes (changing/fixing a couple of places related to functionality change is OK);
  • test your changes before commit and try not to leave things broken.

Code Style

In general, just try to be consistent with existing code.
If you see something not following the style and you don't think it was done intentionally (it looks wrong), feel free to fix.

There's an .editorconfig generated from the auto-formatting settings I'm using, but it doesn't match the style described ideally.

Spacing

Tabs to indent. I have them set to be 4 spaces long, but that's not required as long as those are tabs.

1TBS braces. Opening on the same line, closing on a separate, aligned with opening indent.

if (something) {
	DoThis();
} else {
	DoThat();
}

 
Braces could be left out if the body is return;, continue; or break;:

foreach (var element in collection) {
	if (!SomeCondition(element)) continue;
	// ...
}

 
Closing brace could be on the same line in Properties or if the body is empty:

uint Size { get; set; }
string SizeFormatted { get => SizeFormat.FormatSize(Size); }
virtual void Method() {} // default (base) implementation does nothing
try {
	// ...
} catch {} // there has to be no space between { and } in such cases

 
One space after keywords.
One space between ) and {.
No space after ( and no space before ).

while (true) {}
foreach (var item in collection) {}

 
One space around operators.
No space before , and ; and one after.
No space between function name and arguments.

if (a && b) return;
for (int i = 0; i < 10; ++i) {}
f(1, 2, 3);

 
Parentheses around ternary operator, with spaces around ? and :.

(condition ? a : b)

 
No space before : and one space after in all other cases.

class DerivedClass: ClassBase {
	public DerivedClass(string a): base(a) {}
}

 
No space after cast.

var icon = (IconItem)item;

 
Use vertical space to group related pieces of code and separate unrelated.
In this example, there are three clear blocks: where variables are declared, where some optional logic happens, and where goes the final action:

var basename = Path.GetFileName(filename);
var cwd = Directory.GetCurrentDirectory();
var path = Path.Combine(cwd, "Mods Library", basename);

if (renameInsteadOverwriting) {
	// some logic changing `path` here
}

File.Copy(filename, path, true);

 
Indent case in switch once, and code within it again.
Don't use a break if code ends in return.
Put code on the same line with case if it's short in all the cases.

switch (mod.Type) {
	case ModType.SMPC:
	case ModType.MMPC:
		return new SMPCModInstaller();

	default:
		return null;
}
switch (id) {
	case GameMSMR.ID: return GameMSMR.Instance;
	case GameMM.ID: return GameMM.Instance;
	case GameRCRA.ID: return GameRCRA.Instance;
	default: return null;
}

Naming

Use PascalCase for classes', methods' and namespaces' names.

namespace Overstrike.Data {
	public class Profile {
		public bool Save() {

 
Use camelCase for local variables' names.

void TryDetectGame(string name, string path) {
	if (AlreadyAdded(path)) return;

	var game = GameBase.DetectGameInstallation(path);
	if (game != null) {
		AddToProfilesList(new Profile(name, game, path));
	}
}

 
Use camelCase with underscore in the beginning for private/protected class' fields' names.

protected string _gamePath;

 
Use PascalCase for public fields' and properties' names.

public class SuitSlot {
	public string SuitId { get; set; }
	public bool MarkedToDelete;
	public float DisplayOpacity => (MarkedToDelete ? 0.3f : 1.0f);
}

 
Use UPPERCASE for constants, readonly fields and enum values.

const ulong LOCALIZATION_AID = 0xBE55D94F171BF8DE;
private static readonly Dictionary<string, string> USERFRIENDLY_LANGUAGE_NAMES = new() {
	// ...
};
public enum ArchiveAddingImpl {
	DEFAULT,
	SMPCTOOL,
	SUITTOOL
}

Structure

First, define constants and enums.
Then, the fields and properties.
Then, constructors.
Public methods.
Private/protected methods.

internal class MSMRSuitInstaller: SuitInstallerBase {
	public static readonly Dictionary<string, byte> LANGUAGES = new() { /* ... */ };

	private string _language;

	public MSMRSuitInstaller(TOC_I20 toc, string gamePath, string language = "") : base(toc, gamePath) {  /* ... */ }

	public override void Install(ModEntry mod, int index) { /* ... */ }

	private void WriteAssetsFile(ZipArchive zip, string suitsPath, string id) { /* ... */ }

 
If a field is only used within a few next methods, it can be defined right before them instead of being above.

private bool _reactToItemGeneratorStatusChange = false;

protected void RefreshSelectedSuit(SuitSlot selectedSuit) {
	_reactToItemGeneratorStatusChange = true;
	// ...
}

protected void SuitsSlots_ItemGeneratorStatusChanged(object? sender, EventArgs e) {
	if (!_reactToItemGeneratorStatusChange) return;
	// ...
}

 
That's also true for underlying fields for the properties:

private string _currentFile;
public string CurrentFile { get => _currentFile; }

 
In bigger classes, group things with #region.
Use regions within regions if needed.
Have a blank line after #region and before #endregion.
You can also have regions grouped together (no blank line between #endregion and next #region).

#region state

protected bool _loaded = false;
protected bool _hasChanges = false;

#endregion
#region observable collections

protected ObservableCollection<IconItem> _iconItems = new();
protected ObservableCollection<IconItem> _bigIconItems = new();

#endregion

#region start

public void Init(Action<Thread> addTaskThread, Action<string, string> setOverlayLabels) { /* ... */ }
public void OnOpen() { /* ... */ }

#endregion
#region loading

#region - thread starting

private void StartLoadSuitsThread() { /* ... */ }
private void StartReloadProfileThread(string previouslySelectedSuitId) { /* ... */ }

#endregion
#region - thread logic

private void LoadSuits() { /* ... */ }
private void ReloadProfileSuits(string previouslySelectedSuitId) { /* ... */ }

#endregion

#endregion

Language features

Use C# features if it makes the code simpler.
Try not to use too C#-specific syntax (if it doesn't look obvious for non-C#-programmers).
Try not to use LINQ too much.
 
Properties set on construction:

_loadouts.Add(new LoadoutItem() { Path = path, Name = GetFriendlyLoadoutName(path) });

_cache[tocPath] = new JObject() {
	["config"] = config,
	["timestamp"] = timestamp
};

 
Lambdas:

var thread = new Thread(() => InstallMods(modsToInstall, game, gamePath, uninstalling));
thread.Start();

 
Format line:

ErrorLogger.WriteInfo($"Installing '{mod.Name}'...");

 
Collections range indexing:

if (name.StartsWith("loadout_")) {
	name = name[8..];
}

if (name.EndsWith(".config")) {
	name = name[..^7];
}

 
Checking for null and calling methods if it's not:

item?.Focus();

 
Scope-limited using:

using var f = File.OpenRead(filename);
using var r = new BinaryReader(f);

 
Use var for local variables when possible.

var suitsOrder = new Dictionary<string, int>();
var order = modifications.SuitsOrder;
foreach (var element in collection) {

 
Use new() when possible.

protected List<string> _iconsPaths = new();
⚠️ **GitHub.com Fallback** ⚠️