Replace MaybeNullWhen(false) with NotNullWhen(true) in AppearanceComponent (#1461)

Fixes #1466

MSBuild/XamlIL.targets

@@ -1,24 +1,60 @@
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="12.0">
+  <PropertyGroup>
+    <RobustUseExternalMSBuild>true</RobustUseExternalMSBuild>
+    <_RobustUseExternalMSBuild>$(RobustUseExternalMSBuild)</_RobustUseExternalMSBuild>
+    <_RobustUseExternalMSBuild Condition="'$(_RobustForceInternalMSBuild)' == 'true'">false</_RobustUseExternalMSBuild>
+  </PropertyGroup>
   <ItemGroup>
     <Compile Update="**\*.xaml.cs">
       <DependentUpon>%(Filename)</DependentUpon>
     </Compile>
-    <EmbeddedResource Include="**\*.xaml" />
-    <AdditionalFiles Include="**\*.xaml" />
+    <EmbeddedResource Include="**\*.xaml"/>
+    <AdditionalFiles Include="**\*.xaml"/>
   </ItemGroup>
 
   <ItemGroup>
-    <ProjectReference Include="$(MSBuildThisFileDirectory)\..\Robust.Client.NameGenerator\Robust.Client.NameGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
-    <ProjectReference Include="$(MSBuildThisFileDirectory)\..\Robust.Client.Injectors\Robust.Client.Injectors.csproj" ReferenceOutputAssembly="false" />
+    <ProjectReference Include="$(MSBuildThisFileDirectory)\..\Robust.Client.NameGenerator\Robust.Client.NameGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false"/>
+    <ProjectReference Include="$(MSBuildThisFileDirectory)\..\Robust.Client.Injectors\Robust.Client.Injectors.csproj" ReferenceOutputAssembly="false"/>
   </ItemGroup>
 
-  <UsingTask TaskName="CompileRobustXamlTask" AssemblyFile="$(MSBuildThisFileDirectory)\..\Robust.Client.Injectors\bin\$(Configuration)\net5.0\Robust.Client.Injectors.dll" />
-  <Target Name="CompileRobustXaml" AfterTargets="AfterCompile">
+  <UsingTask
+    Condition="'$(_RobustUseExternalMSBuild)' != 'true' And $(DesignTimeBuild) != true"
+    TaskName="CompileRobustXamlTask"
+    AssemblyFile="$(MSBuildThisFileDirectory)\..\Robust.Client.Injectors\bin\$(Configuration)\netstandard2.0\Robust.Client.Injectors.dll"/>
+  <Target
+    Name="CompileRobustXaml"
+    Condition="Exists('@(IntermediateAssembly)')"
+    AfterTargets="AfterCompile"
+    Inputs="@(IntermediateAssembly);@(ReferencePathWithRefAssemblies)"
+    Outputs="$(IntermediateOutputPath)XAML/doot">
     <PropertyGroup>
       <RobustXamlReferencesTemporaryFilePath Condition="'$(RobustXamlReferencesTemporaryFilePath)' == ''">$(IntermediateOutputPath)XAML/references</RobustXamlReferencesTemporaryFilePath>
       <RobustXamlOriginalCopyFilePath Condition="'$(RobustXamlOriginalCopyFilePath)' == ''">$(IntermediateOutputPath)XAML/original.dll</RobustXamlOriginalCopyFilePath>
     </PropertyGroup>
-    <WriteLinesToFile File="$(RobustXamlReferencesTemporaryFilePath)" Lines="@(ReferencePathWithRefAssemblies)" Overwrite="true" />
-    <CompileRobustXamlTask AssemblyFile="@(IntermediateAssembly)" ReferencesFilePath="$(RobustXamlReferencesTemporaryFilePath)" OriginalCopyPath="$(RobustXamlOriginalCopyFilePath)" ProjectDirectory="$(MSBuildProjectDirectory)" AssemblyOriginatorKeyFile="$(AssemblyOriginatorKeyFile)" SignAssembly="$(SignAssembly)" DelaySign="$(DelaySign)" />
+    <WriteLinesToFile
+      Condition="'$(_RobustForceInternalMSBuild)' != 'true'"
+      File="$(RobustXamlReferencesTemporaryFilePath)"
+      Lines="@(ReferencePathWithRefAssemblies)"
+      Overwrite="true"/>
+
+    <!--
+    UpdateBuildIndicator is done so that we can use MSBuild Inputs and Outputs on the target
+    to avoid unecessary execution of this target
+    Saves compile time if e.g. ONLY Robust.Client changes (Content.Client doesn't have to re-xaml).
+    -->
+    <CompileRobustXamlTask
+      Condition="'$(_RobustUseExternalMSBuild)' != 'true'"
+      AssemblyFile="@(IntermediateAssembly)"
+      ReferencesFilePath="$(RobustXamlReferencesTemporaryFilePath)"
+      OriginalCopyPath="$(RobustXamlOriginalCopyFilePath)"
+      ProjectDirectory="$(MSBuildProjectDirectory)"
+      AssemblyOriginatorKeyFile="$(AssemblyOriginatorKeyFile)"
+      SignAssembly="$(SignAssembly)"
+      DelaySign="$(DelaySign)"
+      UpdateBuildIndicator="$(IntermediateOutputPath)XAML/doot"/>
+
+    <Exec
+      Condition="'$(_RobustUseExternalMSBuild)' == 'true'"
+      Command="dotnet msbuild /nodereuse:false $(MSBuildProjectFile) /t:CompileRobustXaml /p:_RobustForceInternalMSBuild=true /p:Configuration=$(Configuration) /p:TargetFramework=$(TargetFramework) /p:BuildProjectReferences=false"/>
   </Target>
 </Project>

Robust.Client.Injectors/CompileRobustXamlTask.cs

@@ -1,4 +1,5 @@
-using System.Diagnostics;
+using System;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using Microsoft.Build.Framework;
@@ -48,6 +49,19 @@ namespace Robust.Build.Tasks
                 if(File.Exists(inputPdb))
                     File.Copy(inputPdb, outputPdb, true);
             }
+
+            if (!string.IsNullOrEmpty(UpdateBuildIndicator))
+            {
+                if (!File.Exists(UpdateBuildIndicator))
+                {
+                    File.Create(UpdateBuildIndicator).Dispose();
+                }
+                else
+                {
+                    File.SetLastWriteTime(UpdateBuildIndicator, DateTime.Now);
+                }
+            }
+
             return true;
         }
 
@@ -64,6 +78,7 @@ namespace Robust.Build.Tasks
         public string OriginalCopyPath { get; set; }
 
         public string OutputPath { get; set; }
+        public string UpdateBuildIndicator { get; set; }
 
         public string AssemblyOriginatorKeyFile { get; set; }
         public bool SignAssembly { get; set; }

Robust.Client.Injectors/Robust.Client.Injectors.csproj

@@ -1,8 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
     <PropertyGroup>
-        <OutputType>Exe</OutputType>
         <TargetFramework>netstandard2.0</TargetFramework>
+        <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
     </PropertyGroup>
 
     <ItemGroup>

Robust.Client.Injectors/XamlCompiler.cs

@@ -31,6 +31,16 @@ namespace Robust.Build.Tasks
 
             var asm = typeSystem.TargetAssemblyDefinition;
 
+            if (asm.MainModule.GetType("CompiledRobustXaml", "XamlIlContext") != null)
+            {
+                // If this type exists, the assembly has already been processed by us.
+                // Do not run again, it would corrupt the file.
+                // This *shouldn't* be possible due to Inputs/Outputs dependencies in the build system,
+                // but better safe than sorry eh?
+                engine.LogWarningEvent(new BuildWarningEventArgs("XAMLIL", "", "", 0, 0, 0, 0, "Ran twice on same assembly file; ignoring.", "", ""));
+                return (true, false);
+            }
+
             var compileRes = CompileCore(engine, typeSystem);
             if (compileRes == null)
                 return (true, false);

Robust.Client.NameGenerator/Robust.Client.NameGenerator.csproj

@@ -5,8 +5,8 @@
     </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="3.8.0-3.final" PrivateAssets="all" />
-    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.1" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="3.8.0" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.2" PrivateAssets="all" />
   </ItemGroup>
 
   <ItemGroup>

Robust.Client/Audio/Midi/MidiManager.cs

@@ -104,7 +104,7 @@ namespace Robust.Client.Audio.Midi
         private const string OsxSoundfont =
             "/System/Library/Components/CoreAudio.component/Contents/Resources/gs_instruments.dls";
 
-        private const string FallbackSoundfont = "/Resources/Midi/fallback.sf2";
+        private const string FallbackSoundfont = "/Midi/fallback.sf2";
 
         private readonly ResourceLoaderCallbacks _soundfontLoaderCallbacks = new();
 
@@ -207,13 +207,10 @@ namespace Robust.Client.Audio.Midi
 
                 var renderer = new MidiRenderer(_settings!, soundfontLoader);
 
-                foreach (var file in _resourceManager.ContentFindFiles(new ResourcePath("/Audio/MidiCustom/")))
+                foreach (var file in _resourceManager.ContentFindFiles(("/Audio/MidiCustom/")))
                 {
                     if (file.Extension != "sf2" && file.Extension != "dls") continue;
-                    if (_resourceManager.TryGetDiskFilePath(file, out var path))
-                    {
-                        renderer.LoadSoundfont(path);
-                    }
+                    renderer.LoadSoundfont(file.ToString());
                 }
 
                 // Since the last loaded soundfont takes priority, we load the fallback soundfont before the soundfont.
@@ -383,9 +380,10 @@ namespace Robust.Client.Audio.Midi
             public override IntPtr Open(string filename)
             {
                 Stream? stream;
-                if (filename.StartsWith("/Resources/"))
+                var resourceCache = IoCManager.Resolve<IResourceCache>();
+                if (resourceCache.ContentFileExists(filename))
                 {
-                    if (!IoCManager.Resolve<IResourceCache>().TryContentFileRead(filename.Substring(10), out stream))
+                    if (!resourceCache.TryContentFileRead(filename, out stream))
                         return IntPtr.Zero;
                 }
                 else if (File.Exists(filename))

Robust.Client/GameObjects/Components/Appearance/AppearanceComponent.cs

@@ -54,17 +54,17 @@ namespace Robust.Client.GameObjects
             return (T) data[key];
         }
 
-        public override bool TryGetData<T>(Enum key, [MaybeNullWhen(false)] out T data)
+        public override bool TryGetData<T>(Enum key, [NotNullWhen(true)] out T data)
         {
             return TryGetData(key, out data);
         }
 
-        public override bool TryGetData<T>(string key, [MaybeNullWhen(false)] out T data)
+        public override bool TryGetData<T>(string key, [NotNullWhen(true)] out T data)
         {
             return TryGetData(key, out data);
         }
 
-        internal bool TryGetData<T>(object key, [MaybeNullWhen(false)] out T data)
+        internal bool TryGetData<T>(object key, [NotNullWhen(true)] out T data)
         {
             if (this.data.TryGetValue(key, out var dat))
             {
@@ -72,7 +72,7 @@ namespace Robust.Client.GameObjects
                 return true;
             }
 
-            data = default;
+            data = default!;
             return false;
         }
 

Robust.Client/Graphics/Clyde/Audio/Clyde.AudioFormats.OggVorbis.cs

@@ -18,7 +18,7 @@ namespace Robust.Client.Graphics.Clyde
 
                 while (readSamples < totalSamples)
                 {
-                    var read = vorbis.ReadSamples(buffer, readSamples * channels, (int)totalSamples - readSamples);
+                    var read = vorbis.ReadSamples(buffer, readSamples * channels, buffer.Length - readSamples);
                     if (read == 0)
                     {
                         break;

Robust.Client/UserInterface/RichTextEntry.cs

@@ -4,6 +4,7 @@ using JetBrains.Annotations;
 using Robust.Client.Graphics;
 using Robust.Client.Graphics.Drawing;
 using Robust.Client.UserInterface.Controls;
+using Robust.Shared.Log;
 using Robust.Shared.Maths;
 using Robust.Shared.Utility;
 
@@ -172,8 +173,29 @@ namespace Robust.Client.UserInterface
             // This needs to happen because word wrapping doesn't get checked for the last word.
             if (posX > maxSizeX)
             {
-                DebugTools.Assert(wordStartBreakIndex.HasValue,
-                    "wordStartBreakIndex can only be null if the word begins at a new line, in which case this branch shouldn't be reached as the word would be split due to being longer than a single line.");
+                if (!wordStartBreakIndex.HasValue)
+                {
+                    Logger.Error(
+                        "Assert fail inside RichTextEntry.Update, " +
+                        "wordStartBreakIndex is null on method end w/ word wrap required. " +
+                        "Dumping relevant stuff. Send this to PJB.");
+                    Logger.Error($"Message: {Message}");
+                    Logger.Error($"maxSizeX: {maxSizeX}");
+                    Logger.Error($"maxUsedWidth: {maxUsedWidth}");
+                    Logger.Error($"breakIndexCounter: {breakIndexCounter}");
+                    Logger.Error("wordStartBreakIndex: null (duh)");
+                    Logger.Error($"wordSizePixels: {wordSizePixels}");
+                    Logger.Error($"posX: {posX}");
+                    Logger.Error($"lastChar: {lastChar}");
+                    Logger.Error($"forceSplitData: {forceSplitData}");
+                    Logger.Error($"LineBreaks: {string.Join(", ", LineBreaks)}");
+
+                    throw new Exception(
+                        "wordStartBreakIndex can only be null if the word begins at a new line," +
+                        "in which case this branch shouldn't be reached as" +
+                        "the word would be split due to being longer than a single line.");
+                }
+
                 LineBreaks.Add(wordStartBreakIndex!.Value.index);
                 Height += font.GetLineHeight(uiScale);
                 maxUsedWidth = Math.Max(maxUsedWidth, wordStartBreakIndex.Value.lineSize);

Robust.Server/GameObjects/Components/Appearance/AppearanceComponent.cs

@@ -40,17 +40,17 @@ namespace Robust.Server.GameObjects
             return (T)data[key];
         }
 
-        public override bool TryGetData<T>(Enum key, [MaybeNullWhen(false)] out T data)
+        public override bool TryGetData<T>(Enum key, [NotNullWhen(true)] out T data)
         {
             return TryGetData(key, out data);
         }
 
-        public override bool TryGetData<T>(string key, [MaybeNullWhen(false)] out T data)
+        public override bool TryGetData<T>(string key, [NotNullWhen(true)] out T data)
         {
             return TryGetData(key, out data);
         }
 
-        bool TryGetData<T>(object key, [MaybeNullWhen(false)] out T data)
+        private bool TryGetData<T>(object key, [NotNullWhen(true)] out T data)
         {
             if (this.data.TryGetValue(key, out var dat))
             {

Robust.Shared/GameObjects/Components/Appearance/SharedAppearanceComponent.cs

@@ -22,8 +22,8 @@ namespace Robust.Shared.GameObjects.Components.Appearance
         public abstract T GetData<T>(string key);
         public abstract T GetData<T>(Enum key);
 
-        public abstract bool TryGetData<T>(string key, [MaybeNullWhen(false)] out T data);
-        public abstract bool TryGetData<T>(Enum key, [MaybeNullWhen(false)] out T data);
+        public abstract bool TryGetData<T>(string key, [NotNullWhen(true)] out T data);
+        public abstract bool TryGetData<T>(Enum key, [NotNullWhen(true)] out T data);
 
         [Serializable, NetSerializable]
         protected class AppearanceComponentState : ComponentState

nuget.config

@@ -1,6 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <configuration>
   <packageSources>
+  	<add key="nuget" value="https://api.nuget.org/v3/index.json" />
     <add key="dotnet-eng" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json" />
   </packageSources>
 </configuration>