Visual Studio error list spammed with warnings

[Pheubel]

For programming the error list is useful to sniff out compilation errors ahead of time, before switching back to the unity editor. But as it stands, a lot of warnings are generated from the Ink package.

One of the biggest sources are naming rule violations, making up 300+ warnings on their own. Public facing members starting with a lower case it violates the naming conventions and thus generates a warning.

If we take IncludedFile.cs as example:

namespace Ink.Parsed
{
    public class IncludedFile : Parsed.Object
    {
        public Parsed.Story includedStory { get; private set; }

        public IncludedFile (Parsed.Story includedStory)
        {
            this.includedStory = includedStory;
        }

        public override Runtime.Object GenerateRuntimeObject ()
        {
            // Left to the main story to process
            return null;
        }
    }
}

it will show: IDE1006 Naming rule violation: These words must begin with upper case characters: includedStory

In order to remove these warnings for end users there would be 2 options:

1. Refactor the code to comply with the naming conventions. This would be seen as the "correct" thing to do, the downside to this change would be that it would remove backwards compatibility, not so great.

2. Disable warnings for the relevant API's. By doing it like this the end user's API does not change, but will get rid of the warnings. With the example above it would look something like this:

   #pragma warning disable IDE1006
   namespace Ink.Parsed
   {
   public class IncludedFile : Parsed.Object
   {
       public Parsed.Story includedStory { get; private set; }
   
       public IncludedFile (Parsed.Story includedStory)
       {
           this.includedStory = includedStory;
       }
   
       public override Runtime.Object GenerateRuntimeObject ()
       {
           // Left to the main story to process
           return null;
       }
   }
   }
   #pragma warning restore IDE1006

Another area causing warnings is the analyzer picking up on unnecessary assignments, places where readonly can be placed safely and the likes. This is mostly just coding style that can be swapped out without much risk.

Finally we have warnings for using new features, like pattern matching, to clean up the code a bit. If you are always targeting the newest version of C#, then it should be just as easy to refactor as the area above. However, when targeting lower versions they might not have access to the same list of features. A good effort can be made by using #if CSHARP_7_3_OR_NEWER toensure that the newer features can be used. The downside to this approach would be that two versions of the same procedure will have to be maintained.

I wouldn't mind helping out with this stuff, as long as i know which direction to take.

[tomkail]

Adding warnings sounds best! I’m aware that capitals are considered “correct” in .net but I’ve not worked with many people who follow that convention. Feel free to submit a pull request. On Mon, 30 Aug 2021 at 00:05, Pheubel @.***> wrote:

For programming the error list is useful to sniff out compilation errors ahead of time, before switching back to the unity editor. But as it stands, a lot of warnings are generated from the Ink package.

One of the biggest sources are naming rule violations, making up 300+ warnings on their own. Public facing members starting with a lower case it violates the naming conventions https://docs.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names#naming-conventions and thus generates a warning.

If we take IncludedFile.cs as example:

namespace Ink.Parsed { public class IncludedFile : Parsed.Object { public Parsed.Story includedStory { get; private set; }

    public IncludedFile (Parsed.Story includedStory)
    {
        this.includedStory = includedStory;
    }

    public override Runtime.Object GenerateRuntimeObject ()
    {
        // Left to the main story to process
        return null;
    }
}

}

it will show: IDE1006 Naming rule violation: These words must begin with upper case characters: includedStory

In order to remove these warnings for end users there would be 2 options:

1. Refactor the code to comply with the naming conventions. This would be seen as the "correct" thing to do, the downside to this change would be that it would remove backwards compatibility, not so great.
2. Disable warnings for the relevant API's. By doing it like this the end user's API does not change, but will get rid of the warnings. With the example above it would look something like this:

pragma warning disable IDE1006namespace Ink.Parsed

{ public class IncludedFile : Parsed.Object { public Parsed.Story includedStory { get; private set; }

   public IncludedFile (Parsed.Story includedStory)
   {
       this.includedStory = includedStory;
   }

   public override Runtime.Object GenerateRuntimeObject ()
   {
       // Left to the main story to process
       return null;
   }

} }

pragma warning restore IDE1006

Another area causing warnings is the analyzer picking up on unnecessary assignments, places where readonly can be placed safely and the likes. This is mostly just coding style that can be swapped out without much risk.

Finally we have warnings for using new features, like pattern matching, to clean up the code a bit. If you are always targeting the newest version of C#, then it should be just as easy to refactor as the area above. However, when targeting lower versions they might not have access to the same list of features. A good effort can be made by using #if CSHARP_7_3_OR_NEWER toensure that the newer features can be used. The downside to this approach would be that two versions of the same procedure will have to be maintained.

I wouldn't mind helping out with this stuff, as long as i know which direction to take.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/inkle/ink-unity-integration/issues/157, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJR3UDV2PCNVN76AYHPBZDT7K4JXANCNFSM5DAWY7RA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[tomkail]

Not heard back on this, closing!