style: wrap at 190 chars, enrich code style, apply code style (shpaas…

…s#270)

Main changes:
* Added blank lines around keywords like `if`, `while`, and some others.
* Wrapped lines that went beyond 190 characters.

Then the review went beyond that, and we fixed a ton of adjacent stuff too.

.editorconfig

@@ -102,6 +102,8 @@ csharp_style_expression_bodied_operators = true:suggestion
 csharp_style_expression_bodied_properties = true:suggestion
 csharp_style_expression_bodied_indexers = true:suggestion
 csharp_style_expression_bodied_accessors = true:suggestion
+csharp_style_expression_bodied_lambdas = true:suggestion
+csharp_style_expression_bodied_local_functions = true:suggestion
 # Pattern matching preferences
 csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion
 csharp_style_pattern_matching_over_as_with_null_check = true:suggestion

CommandLineToolExample/Program.cs

@@ -44,8 +44,6 @@ public static void Main(string[] args) {
     }
 
     private class ConsoleProgressReport : IProgress<(string, string)> {
-        public void Report((string, string) value) {
-            Console.WriteLine(value.Item1 + "  -  " + value.Item2);
-        }
+        public void Report((string, string) value) => Console.WriteLine(value.Item1 + "  -  " + value.Item2);
     }
 }

Docs/CodeStyle.md

@@ -18,7 +18,6 @@ The main idea is to keep the code maintainable and readable.
 
 ### Code
 * In the programmers' haven, there is always a free spot for those who write tests.
-* Please try to keep the lines shorter than 190 characters so they fit on most monitors.
 * If you add a TODO, then please describe the details in the commit message, or ideally in a github issue. That increases the chances of the TODOs being addressed.
 
 #### Difference with C# conventions
@@ -52,3 +51,24 @@ private void One(<a long description
     <more calls and assignments> // An empty line to clearly separate the definition and the start of the function
 }
 ```
+
+### Line wrap
+Please try to keep the lines shorter than 190 characters, so they fit on most monitors.
+
+When wrapping a line, you can use the following example as a guideline for which wrapping is preferred:
+```
+if (recipe.subgroup == null
+    // (1) Add breaks at operators before adding them within the expressions passed to those operators
+    && imgui.BuildRedButton("Delete recipe")
+        // (2) Add breaks before .MethodName before adding breaks between method parameters
+        .WithTooltip(imgui,
+            // (3) Add breaks between method parameters before adding breaks within method parameters
+            "Shortcut: right-click")
+    // (1)
+    && imgui.CloseDropdown()) {
+```
+
+Most of the operators like `=>` `.` `+` `&&` go to the next line.  
+The notable operators that stay on the same line are `=> {` and `,`.
+
+The wrapping of arguments in constructors and method-definitions is up to you.

Yafc.Model.Tests/Analysis/Milestones.cs

@@ -3,6 +3,7 @@
 
 #pragma warning disable CA1861 // "CA1861: Avoid constant arrays as arguments." Disabled because it tried to fix constant arrays in InlineData.
 namespace Yafc.Model.Tests;
+
 public class MilestonesTests {
     private static Bits createBits(ulong value) {
         var bitsType = typeof(Bits);
@@ -23,7 +24,6 @@ private static Milestones setupMilestones(ulong result, ulong mask, out Factorio
             [factorioObj] = createBits(result)
         };
 
-
         var milestonesType = typeof(Milestones);
         var milestonesLockedMask = milestonesType.GetProperty("lockedMask");
         var milestoneResultField = milestonesType.GetField("milestoneResult", BindingFlags.NonPublic | BindingFlags.Instance);
@@ -72,8 +72,8 @@ public void GetLockedMaskFromProject_ShouldCalculateMask(bool unlocked, int[] bi
         var projectField = milestonesType.GetField("project", BindingFlags.NonPublic | BindingFlags.Instance);
 
         var milestones = setupMilestones(0, 0, out FactorioObject factorioObj);
-
         Project project = new Project();
+
         if (unlocked) {
             // Can't use SetFlag() as it uses the Undo system, which requires SDL
             var flags = project.settings.itemFlags;
@@ -87,11 +87,12 @@ public void GetLockedMaskFromProject_ShouldCalculateMask(bool unlocked, int[] bi
 
         _ = getLockedMaskFromProject.Invoke(milestones, null);
         var lockedBits = milestones.lockedMask;
-
         int index = 0;
+
         for (int i = 0; i < lockedBits.length; i++) {
             bool expectSet = index == bitsCleared.Length || bitsCleared[index] != i;
             Assert.True(expectSet == lockedBits[i], "bit " + i + " is expected to be " + (expectSet ? "set" : "cleared"));
+
             if (index < bitsCleared.Length && bitsCleared[index] == i) {
                 index++;
             }

Yafc.Model.Tests/Data/DataUtils.cs

@@ -20,6 +20,7 @@ public void TryParseAmount_IsInverseOfFormatValue() {
             for (UnitOfMeasure unit = 0; unit < UnitOfMeasure.Celsius; unit++) {
                 float value;
                 int count = 1;
+
                 do {
                     r.NextBytes(bytes);
                     value = BitConverter.ToSingle(bytes, 0);
@@ -92,14 +93,17 @@ public void TryParseAmount_IsInverseOfFormatValue_WithBeltsAndPipes() {
 
     [Theory]
     [MemberData(nameof(TryParseAmount_TestData))]
-    public void TryParseAmount_WhenGivenInputs_ShouldProduceCorrectValues(string input, UnitOfMeasure unitOfMeasure, bool expectedReturn, float expectedOutput, int time, int itemUnit, int fluidUnit, int defaultBeltSpeed) {
+    public void TryParseAmount_WhenGivenInputs_ShouldProduceCorrectValues(string input, UnitOfMeasure unitOfMeasure, bool expectedReturn, float expectedOutput, int time,
+        int itemUnit, int fluidUnit, int defaultBeltSpeed) {
+
         Project.current.preferences.time = time;
         Project.current.preferences.itemUnit = itemUnit;
         Project.current.preferences.fluidUnit = fluidUnit;
         Project.current.preferences.defaultBelt ??= new();
         typeof(EntityBelt).GetProperty(nameof(EntityBelt.beltItemsPerSecond)).SetValue(Project.current.preferences.defaultBelt, defaultBeltSpeed);
 
         Assert.Equal(expectedReturn, DataUtils.TryParseAmount(input, out float result, unitOfMeasure));
+
         if (expectedReturn) {
             double error = (result - expectedOutput) / (double)expectedOutput;
             Assert.True(Math.Abs(error) < .00001, $"Parsing {input} produced {result}, which differs from the expected {expectedOutput} by {error:0.00%}.");

Yafc.Model.Tests/LuaDependentTestHelper.cs

@@ -35,10 +35,13 @@ static LuaDependentTestHelper() {
     internal static Project GetProjectForLua(string targetStreamName = null) {
         // Verify correct non-parallel declaration for tests, to accomodate the singleton analyses.
         StackTrace stack = new();
+
         for (int i = 1; i < stack.FrameCount; i++) {
+
             // Search up the stack until we find a method with [Fact] or [Theory].
             MethodBase method = stack.GetFrame(i).GetMethod();
             if (method.GetCustomAttribute<FactAttribute>() != null || method.GetCustomAttribute<TheoryAttribute>() != null) {
+
                 targetStreamName ??= method.DeclaringType.FullName + '.' + method.Name + ".lua";
 
                 // CollectionAttribute doesn't store its constructor argument, so we have to read the attribute data instead of the constructed attribute.
@@ -48,6 +51,7 @@ internal static Project GetProjectForLua(string targetStreamName = null) {
                     // A second test can replace the analysis results while the first is still running.
                     Assert.Fail($"Test classes that call {nameof(LuaDependentTestHelper)}.{nameof(GetProjectForLua)} must be annotated with [Collection(\"LuaDependentTests\")].");
                 }
+
                 break;
             }
         }

Yafc.Model.Tests/Math/Bits.cs

@@ -1,7 +1,10 @@
 using System.Reflection;
 using Xunit;
 
+#pragma warning disable CA1861 // "CA1861: Avoid constant arrays as arguments." Disabled because it tried to fix constant arrays in InlineData.
+
 namespace Yafc.Model.Tests;
+
 public class BitsTests {
     [Fact]
     public void New_WhenTrueIsProvided_ShouldHaveBit0Set() {
@@ -20,6 +23,7 @@ public void SetBit_WhenGivenABit_ShouldReturnSetBit(int bit) {
         bits[bit] = true;
 
         Assert.True(bits[bit], "Set bit should be set");
+
         for (int i = 0; i <= 128; i++) {
             if (i != bit) {
                 Assert.False(bits[i], "Other bits should be clear");
@@ -354,7 +358,6 @@ public void UnequalOperator_WithSameValueDifferentLengths_ShouldReturnCorrectRes
         Assert.False(a != b);
     }
 
-
     [Fact]
     public void UnequalOperator_WithDefault_ShouldReturnFalse() {
         Bits a = default;
@@ -374,14 +377,12 @@ public void SubtractOperator_WithLongInputBits_ShouldReturnCorrectResult() {
 
         var result = a - 1;
 
-
         ulong[] resultValue = (ulong[])bitsData.GetValue(result);
 
         Assert.Equal(~0ul, resultValue[0]);
         Assert.Equal(2ul, resultValue[1]);
     }
 
-
     [Theory]
     [InlineData(1, 1)]
     [InlineData(2, 1)]

Yafc.Model.Tests/Model/ProductionTableContentTests.cs

@@ -29,11 +29,17 @@ public void ChangeFuelEntityModules_ShouldPreserveFixedAmount() {
         static void testCombinations(RecipeRow row, ProductionTable table, Action assert) {
             foreach (EntityCrafter crafter in Database.allCrafters) {
                 row.entity = crafter;
+
                 foreach (Goods fuel in crafter.energy.fuels) {
                     row.fuel = fuel;
+
                     foreach (Module module in Database.allModules.Concat([null])) {
                         ModuleTemplateBuilder builder = new();
-                        if (module != null) { builder.list.Add((module, 0)); }
+
+                        if (module != null) {
+                            builder.list.Add((module, 0));
+                        }
+
                         row.modules = builder.Build(row);
                         table.Solve((ProjectPage)table.owner).Wait();
                         assert();
@@ -65,15 +71,17 @@ public void ChangeProductionTableModuleConfig_ShouldPreserveFixedAmount() {
         void testCombinations(RecipeRow row, ProductionTable table, Action assert) {
             foreach (EntityCrafter crafter in Database.allCrafters) {
                 row.entity = crafter;
+
                 foreach (Goods fuel in crafter.energy.fuels) {
                     row.fuel = fuel;
+
                     foreach (Module module in modules) {
                         for (int beaconCount = 0; beaconCount < 13; beaconCount++) {
                             for (float payback = 1; payback < float.MaxValue; payback *= 16) {
                                 if (table.GetType().GetProperty("modules").SetMethod is MethodInfo method) {
                                     // Pre-emptive code for if ProductionTable.modules is made writable.
                                     // The ProductionTable.modules setter must notify all relevant recipes if it is added.
-                                    method.Invoke(table, [new ModuleFillerParameters(table) {
+                                    _ = method.Invoke(table, [new ModuleFillerParameters(table) {
                                         beacon = beacon,
                                         beaconModule = module,
                                         beaconsPerBuilding = beaconCount,

Yafc.Model/Analysis/Analysis.cs

@@ -3,16 +3,16 @@
 using System.Linq;
 
 namespace Yafc.Model;
+
 public abstract class Analysis {
     internal readonly HashSet<FactorioObject> excludedObjects = [];
 
     public abstract void Compute(Project project, ErrorCollector warnings);
 
     private static readonly List<Analysis> analyses = [];
-    public static void RegisterAnalysis(Analysis analysis, params Analysis[] dependencies) // TODO don't ignore dependencies
-    {
-        analyses.Add(analysis);
-    }
+
+    // TODO don't ignore dependencies
+    public static void RegisterAnalysis(Analysis analysis, params Analysis[] dependencies) => analyses.Add(analysis);
 
     public static void ProcessAnalyses(IProgress<(string, string)> progress, Project project, ErrorCollector errors) {
         foreach (var analysis in analyses) {
@@ -44,41 +44,23 @@ public static void ExcludeFromAnalysis<T>(FactorioObject obj) where T : Analysis
 }
 
 public static class AnalysisExtensions {
-    public static bool IsAccessible(this FactorioObject obj) {
-        return Milestones.Instance.GetMilestoneResult(obj) != 0;
-    }
+    public static bool IsAccessible(this FactorioObject obj) => Milestones.Instance.GetMilestoneResult(obj) != 0;
 
-    public static bool IsAccessibleWithCurrentMilestones(this FactorioObject obj) {
-        return Milestones.Instance.IsAccessibleWithCurrentMilestones(obj);
-    }
+    public static bool IsAccessibleWithCurrentMilestones(this FactorioObject obj) => Milestones.Instance.IsAccessibleWithCurrentMilestones(obj);
 
-    public static bool IsAutomatable(this FactorioObject obj) {
-        return AutomationAnalysis.Instance.automatable[obj] != AutomationStatus.NotAutomatable;
-    }
+    public static bool IsAutomatable(this FactorioObject obj) => AutomationAnalysis.Instance.automatable[obj] != AutomationStatus.NotAutomatable;
 
-    public static bool IsAutomatableWithCurrentMilestones(this FactorioObject obj) {
-        return AutomationAnalysis.Instance.automatable[obj] == AutomationStatus.AutomatableNow;
-    }
+    public static bool IsAutomatableWithCurrentMilestones(this FactorioObject obj) => AutomationAnalysis.Instance.automatable[obj] == AutomationStatus.AutomatableNow;
 
-    public static float Cost(this FactorioObject goods, bool atCurrentMilestones = false) {
-        return CostAnalysis.Get(atCurrentMilestones).cost[goods];
-    }
+    public static float Cost(this FactorioObject goods, bool atCurrentMilestones = false) => CostAnalysis.Get(atCurrentMilestones).cost[goods];
 
-    public static float ApproximateFlow(this FactorioObject recipe, bool atCurrentMilestones = false) {
-        return CostAnalysis.Get(atCurrentMilestones).flow[recipe];
-    }
+    public static float ApproximateFlow(this FactorioObject recipe, bool atCurrentMilestones = false) => CostAnalysis.Get(atCurrentMilestones).flow[recipe];
 
-    public static float ProductCost(this Recipe recipe, bool atCurrentMilestones = false) {
-        return CostAnalysis.Get(atCurrentMilestones).recipeProductCost[recipe];
-    }
+    public static float ProductCost(this Recipe recipe, bool atCurrentMilestones = false) => CostAnalysis.Get(atCurrentMilestones).recipeProductCost[recipe];
 
-    public static float RecipeWaste(this Recipe recipe, bool atCurrentMilestones = false) {
-        return CostAnalysis.Get(atCurrentMilestones).recipeWastePercentage[recipe];
-    }
+    public static float RecipeWaste(this Recipe recipe, bool atCurrentMilestones = false) => CostAnalysis.Get(atCurrentMilestones).recipeWastePercentage[recipe];
 
-    public static float RecipeBaseCost(this Recipe recipe, bool atCurrentMilestones = false) {
-        return CostAnalysis.Get(atCurrentMilestones).recipeCost[recipe];
-    }
+    public static float RecipeBaseCost(this Recipe recipe, bool atCurrentMilestones = false) => CostAnalysis.Get(atCurrentMilestones).recipeCost[recipe];
 
     /// <summary>
     /// Filters a list of <see cref="FactorioObject"/>s down to those that were not excluded by the specified <see cref="Analysis"/>

Yafc.Model/Analysis/AutomationAnalysis.cs

@@ -4,6 +4,7 @@
 using Yafc.UI;
 
 namespace Yafc.Model;
+
 public enum AutomationStatus : sbyte {
     NotAutomatable = -1, AutomatableLater = 2, AutomatableNow = 3,
 }
@@ -22,8 +23,10 @@ public override void Compute(Project project, ErrorCollector warnings) {
         state[Database.voidEnergy] = AutomationStatus.AutomatableNow;
         Queue<FactorioId> processingQueue = new Queue<FactorioId>(Database.objects.count);
         int unknowns = 0;
+
         foreach (Recipe recipe in Database.recipes.all.ExceptExcluded(this)) {
             bool hasAutomatableCrafter = false;
+
             foreach (var crafter in recipe.crafters) {
                 if (crafter != Database.character && crafter.IsAccessible()) {
                     hasAutomatableCrafter = true;
@@ -49,6 +52,7 @@ public override void Compute(Project project, ErrorCollector warnings) {
             var index = processingQueue.Dequeue();
             var dependencies = Dependencies.dependencyList[index];
             var automationState = Milestones.Instance.IsAccessibleWithCurrentMilestones(index) ? AutomationStatus.AutomatableNow : AutomationStatus.AutomatableLater;
+
             foreach (var depGroup in dependencies) {
                 if (!depGroup.flags.HasFlags(DependencyList.Flags.OneTimeInvestment)) {
                     if (depGroup.flags.HasFlags(DependencyList.Flags.RequireEverything)) {
@@ -60,6 +64,7 @@ public override void Compute(Project project, ErrorCollector warnings) {
                     }
                     else {
                         var localHighest = AutomationStatus.NotAutomatable;
+
                         foreach (var element in depGroup.elements) {
                             if (state[element] > localHighest) {
                                 localHighest = state[element];
@@ -74,6 +79,7 @@ public override void Compute(Project project, ErrorCollector warnings) {
                 else if (automationState == AutomationStatus.AutomatableNow && depGroup.flags == DependencyList.Flags.CraftingEntity) {
                     // If only character is accessible at current milestones as a crafting entity, don't count the object as currently automatable
                     bool hasMachine = false;
+
                     foreach (var element in depGroup.elements) {
                         if (element != Database.character?.id && Milestones.Instance.IsAccessibleWithCurrentMilestones(element)) {
                             hasMachine = true;
@@ -94,8 +100,10 @@ public override void Compute(Project project, ErrorCollector warnings) {
             state[index] = automationState;
             if (automationState != Unknown) {
                 unknowns--;
+
                 foreach (var revDep in Dependencies.reverseDependencies[index]) {
                     var oldState = state[revDep];
+
                     if (oldState == Unknown || (oldState == AutomationStatus.AutomatableLater && automationState == AutomationStatus.AutomatableNow)) {
                         if (oldState == AutomationStatus.AutomatableLater) {
                             unknowns++;