C7-Game / Prototype

An early-stage, open-source 4X strategy game
https://c7-game.github.io/
MIT License
34 stars 9 forks source link

Apply code formatting config to all source files #406

Open WildWeazel opened 1 year ago

WildWeazel commented 1 year ago

163 introduced a .editorconfig with code formatting rules. Reformat all .cs (and .gd?) files at once. Going forward devs should use this file in their IDE if possible, and format changes accordingly. But first let's settle on what the formatting rules should be.

I recommend we follow Godot's own C# style guide, and Microsoft's as a fallback. Discuss formatting here.

pcen commented 1 year ago

https://github.com/C7-Game/Prototype/issues/143 falls under this issue

pcen commented 1 year ago

I need to revisit the .editorconfig, recently my vscode C# got updated (I assume) because it's giving new suggestions that aren't in the editor config and were not previously defaults:

remove "this" qualifier on member variables/methods/properties when they are not shadowing a local non-member identifier

personally, I don't like this default, I prefer members to be qualified with "this" because it makes code easier to read at the expense of being slightly more verbose to type

use explicit type instead of var

I believe this has been our recommended coding style, but I don't think my editor used to complain

use lowercase identifier for builtin types instead of capitalised (ie. double instead of Double)

new expression can be simplified

instead of Label foo = new Label(); this one recommends Label foo = new(); Personally I like this one, since we're opting to not use var on the lhs, I think this is a nice tradeoff of making code less verbose without, imo, reducing clarity

object initialisation can be simplified

here's a real world example from our codebase: before:

TerrainType c7Terrain = new TerrainType();
c7Terrain.Key = civTerrainKeyLookup[civ3Index];
c7Terrain.DisplayName = civ3Terrain.Name;
c7Terrain.baseFoodProduction = civ3Terrain.Food;
c7Terrain.baseShieldProduction = civ3Terrain.Shields;
c7Terrain.baseCommerceProduction = civ3Terrain.Commerce;
c7Terrain.movementCost = civ3Terrain.MovementCost;
c7Terrain.allowCities = civ3Terrain.AllowCities != 0;
c7Terrain.defenseBonus = new StrengthBonus {
      description = civ3Terrain.Name,
      amount = civ3Terrain.DefenseBonus / 100.0
};

after:

TerrainType c7Terrain = new TerrainType {
  Key = civTerrainKeyLookup[civ3Index],
  DisplayName = civ3Terrain.Name,
  baseFoodProduction = civ3Terrain.Food,
  baseShieldProduction = civ3Terrain.Shields,
  baseCommerceProduction = civ3Terrain.Commerce,
  movementCost = civ3Terrain.MovementCost,
  allowCities = civ3Terrain.AllowCities != 0,
  defenseBonus = new StrengthBonus {
      description = civ3Terrain.Name,
      amount = civ3Terrain.DefenseBonus / 100.0
  }
};

The indentation is messed up here but the idea is to assign fields values inside a member initialiser list that's tacked on to the end of the constructor (in this case, it's the default ctor, but this syntax also works for something like new Foo(buz){Bar = bar};

WildWeazel commented 11 months ago

I definitely agree with using 'this' by default. It also helps with autocomplete when typing. Using 'new()' instead of 'var' seems fair.

Object initialization seems cool but I haven't used it enough to have much of an opinion. It does make it easier to hide setters.