From f52f7ca36f2ecf3d4478c5bc1e9cd95e5ff53929 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 19:32:00 -0400 Subject: add mod code analyzers to detect implicit net field conversion issues (#471) --- docs/mod-build-config.md | 63 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 7 deletions(-) (limited to 'docs/mod-build-config.md') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 0d72e4d9..71a2518a 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -3,15 +3,16 @@ for SMAPI mods. The package... -* lets your code compile on any computer (Linux/Mac/Windows) without needing to change the assembly - references or game path. -* packages the mod into the game's `Mods` folder when you rebuild the code (configurable). -* configures Visual Studio so you can debug into the mod code when the game is running (_Windows - only_). +* detects your game install path; +* adds the assembly references you need (with automatic support for Linux/Mac/Windows); +* packages the mod into your `Mods` folder when you rebuild the code (configurable); +* configures Visual Studio to enable debugging into the code when the game is running (_Windows only_); +* adds C# analyzers to warn for Stardew Valley-specific issues. ## Contents * [Install](#install) * [Configure](#configure) +* [Code analysis warnings](#code-analysis-warnings) * [Troubleshoot](#troubleshoot) * [Release notes](#release-notes) @@ -121,7 +122,7 @@ The configuration will check your custom path first, then fall back to the defau still compile on a different computer). ### Unit test projects -**(upcoming in 2.0.3)** +**(upcoming in 2.1)** You can use the package in unit test projects too. Its optional unit test mode... @@ -134,15 +135,63 @@ To enable it, add this above the first `` in your `.csproj`: True ``` +## Code warnings +### Overview +The NuGet package adds code warnings in Visual Studio specific to Stardew Valley. For example: +![](screenshots/code-analyzer-example.png) + +You can hide the warnings... +* [for specific code](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives/preprocessor-pragma-warning); +* for a method using this attribute: + ```cs + [System.Diagnostics.CodeAnalysis.SuppressMessage("SMAPI.CommonErrors", "SMAPI001")] // implicit net field conversion + ``` +* for an entire project: + 1. Expand the _References_ node for the project in Visual Studio. + 2. Right-click on _Analyzers_ and choose _Open Active Rule Set_. + 4. Expand _StardewModdingAPI.ModBuildConfig.Analyzer_ and uncheck the warnings you want to hide. + +See below for help with each specific warning. + +### SMAPI001 +**Implicit net field conversion:** +> This implicitly converts '{{expression}}' from {{net type}} to {{other type}}, but +> {{net type}} has unintuitive implicit conversion rules. Consider comparing against the actual +> value instead to avoid bugs. + +Stardew Valley uses a set of net fields (like `NetBool` or `NetInt`) to handle multiplayer sync. +These types can implicitly convert to their equivalent normal values (like `bool x = new NetBool()`), +but their conversion rules can be unintuitive and error-prone. For example, +`item?.category == null && item?.category != null` can both be true at once, and +`building.indoors != null` will be true for a null value in some cases. + +Suggested fix: +* For a reference type (i.e. one that can contain `null`), you can use the `.Value` property: + ```c# + if (building.indoors.Value == null) + ``` + Or convert the value before comparison: + ```c# + GameLocation indoors = building.indoors; + if(indoors == null) + // ... + ``` +* For a value type (i.e. one that can't contain `null`), make sure to check for null first if + applicable: + ```cs + if (item != null && item.category == 0) + ``` + ## Troubleshoot ### "Failed to find the game install path" That error means the package couldn't find your game. You can specify the game path yourself; see _[Game path](#game-path)_ above. ## Release notes -### 2.0.3 alpha +### 2.1 alpha * Added support for Stardew Valley 1.3. * Added support for unit test projects. +* Added C# analyzers to warn about implicit conversions of Netcode fields in Stardew Valley 1.3. ### 2.0.2 * Fixed compatibility issue on Linux. -- cgit From 4f5f463bd227a81c26224a81b178e2221946b9b2 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 22:33:45 -0400 Subject: warn when directly using a net field that has a non-net wrapper (#471) --- docs/mod-build-config.md | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'docs/mod-build-config.md') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 71a2518a..44242160 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -182,6 +182,15 @@ Suggested fix: if (item != null && item.category == 0) ``` +### SMAPI002 +**Avoid net fields when possible:** +> '{{expression}}' is a {{net type}} field; consider using the {{property name}} property instead. + +Your code accesses a net field, which has some unusual behavior (see [SMAPI001](#SMAPI001)). This +field has an equivalent non-net property that avoids those issues. + +Suggested fix: access the suggested property name instead. + ## Troubleshoot ### "Failed to find the game install path" That error means the package couldn't find your game. You can specify the game path yourself; see -- cgit From c6c2302baf830a441b269ef7402068bd9dece22e Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:39 -0400 Subject: tweak analyzer code & documentation (#471) --- docs/mod-build-config.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'docs/mod-build-config.md') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 44242160..00f9a356 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -159,13 +159,16 @@ See below for help with each specific warning. > {{net type}} has unintuitive implicit conversion rules. Consider comparing against the actual > value instead to avoid bugs. -Stardew Valley uses a set of net fields (like `NetBool` or `NetInt`) to handle multiplayer sync. -These types can implicitly convert to their equivalent normal values (like `bool x = new NetBool()`), -but their conversion rules can be unintuitive and error-prone. For example, +Stardew Valley uses net types (like `NetBool` and `NetInt`) to handle multiplayer sync. These types +can implicitly convert to their equivalent normal values (like `bool x = new NetBool()`), but their +conversion rules are unintuitive and error-prone. For example, `item?.category == null && item?.category != null` can both be true at once, and `building.indoors != null` will be true for a null value in some cases. Suggested fix: +* Some net fields have an equivalent non-net property like `monster.Health` (`int`) instead of + `monster.health` (`NetInt`). The package will add a separate [SMAPI002](#smapi002) warning for + these. Use the suggested property instead. * For a reference type (i.e. one that can contain `null`), you can use the `.Value` property: ```c# if (building.indoors.Value == null) @@ -176,17 +179,17 @@ Suggested fix: if(indoors == null) // ... ``` -* For a value type (i.e. one that can't contain `null`), make sure to check for null first if - applicable: +* For a value type (i.e. one that can't contain `null`), check if the object is null (if applicable) + and compare with `.Value`: ```cs - if (item != null && item.category == 0) + if (item != null && item.category.Value == 0) ``` ### SMAPI002 **Avoid net fields when possible:** > '{{expression}}' is a {{net type}} field; consider using the {{property name}} property instead. -Your code accesses a net field, which has some unusual behavior (see [SMAPI001](#SMAPI001)). This +Your code accesses a net field, which has some unusual behavior (see [SMAPI001](#smapi001)). This field has an equivalent non-net property that avoids those issues. Suggested fix: access the suggested property name instead. -- cgit From 13f31e8b725e46ca8442a943a5675723d22b4fdc Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:57 -0400 Subject: warn for fields which no longer work (#471) --- docs/mod-build-config.md | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'docs/mod-build-config.md') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 00f9a356..99a567f2 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -194,6 +194,12 @@ field has an equivalent non-net property that avoids those issues. Suggested fix: access the suggested property name instead. +### SMAPI003 +**Avoid obsolete fields:** +> The '{{old field}}' field is obsolete and should be replaced with '{{new field}}'. + +Your code accesses a field which is obsolete or no longer works. Use the suggested field instead. + ## Troubleshoot ### "Failed to find the game install path" That error means the package couldn't find your game. You can specify the game path yourself; see -- cgit