MonoGame.Extended icon indicating copy to clipboard operation
MonoGame.Extended copied to clipboard

Initial Coding Guidelines

Open AristurtleDev opened this issue 7 months ago • 6 comments

I have drafted an initial set of coding guidelines to use. Some of these are based on general .NET best practices for libraries, others are more targeted since this is a library meant to be used specifically with MonoGame.

AristurtleDev avatar May 20 '25 19:05 AristurtleDev

Anyway we can create a linter or pre-check script so people don't have to memorize these?

I haven't read it over completely but I'm just recalling rubocop for ruby.

Too many rules remember so someone wrote a Pre check tool.

kaltinril avatar May 21 '25 23:05 kaltinril

@kaltinril Yes, there are built-in Microsoft analysis rules, off-the shelf ones, and the ability to write custom ones using Rosyln tooling. See https://learn.microsoft.com/en-us/visualstudio/code-quality/roslyn-analyzers-overview

For Microsoft's extended built in code analysis, it can be turned on via .csproj or a shared Directory.Build.props file. Note that your milage and MonoGame.Extended project may wish to it differently but the jist is here to "turn up the analysis to maximum".

<PropertyGroup>
    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
    <EnableNETAnalyzers>true</EnableNETAnalyzers>
    <AnalysisMode>all</AnalysisMode>
    <AnalysisLevel>latest</AnalysisLevel>
    <RunAnalyzersDuringBuild>true</RunAnalyzersDuringBuild>
    <RunAnalyzersDuringLiveAnalysis>true</RunAnalyzersDuringLiveAnalysis>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
  </PropertyGroup>

There exists off the shelf analysis tools that can be added as "development" NuGet packages only (not added to the final build). See https://github.com/cybermaxs/awesome-analyzers.

If none of that is enough, there is documentation/tutorials on how to write custom analyzers that hook into the .NET build process. See link above.

lithiumtoast avatar May 21 '25 23:05 lithiumtoast

Anyway we can create a linter or pre-check script so people don't have to memorize these?

These can all be addressed using an editorconfig.

There is code already in the project file that uses the editor config to treat warnings as errors to catch this in a linting perspective. It's just a matter of updating the editor config once everything is finalized.

AristurtleDev avatar May 21 '25 23:05 AristurtleDev

Great thank you both, that is my only concern. As reading through it I saw so many rules that I know I would never remember them all.

I also code in 5 or more different languages depending on the project at work. Each having their own style guidelines. It becomes a chore just to figure out how to style something sometimes.

kaltinril avatar May 21 '25 23:05 kaltinril

@kaltinril Yes of course. The most useful analyzers are also ones that also provide a "quick fix" where a developer can have the analyzer fix the code directly by a click of a button. This is possible leveraging Roslyn's API for both syntax and semantic analysis.

lithiumtoast avatar May 21 '25 23:05 lithiumtoast

Per discussion in #988 I have updated the extension method section guidelines to the following to prefer static methods in types we control the source for rather than creating an extension class for it.


Extension Methods

Preferred Approach for MonoGame.Extended Types:

  • For types that are part of MonoGame.Extended (where we control the source code), prefer static methods within the type itself rather than extension methods
  • Extension methods should primarily be used for extending types from MonoGame core or external libraries where we don't control the source

When Extension Methods Are Necessary:

  • Extension Class Naming: {TypeName}Extensions (e.g., ColorExtensions, Vector2Extensions)
  • One Extension Class Per Type: Each type being extended should have its own extension class
  • No Mixed Extensions: Don't extend multiple types in a single extension class

AristurtleDev avatar May 21 '25 23:05 AristurtleDev

Given there hasn't been any further discussion or changes requested to the guidelines, I'm going to merge these in now.

AristurtleDev avatar Jul 24 '25 19:07 AristurtleDev