Chizaruu / com.tsk.ide.vscode

Enhance your Unity development workflow with seamless code editor integration for VSCode.
https://openupm.com/packages/com.tsk.ide.vscode/
181 stars 5 forks source link

Add NuGet Package Import Support #39

Open Chizaruu opened 1 year ago

Chizaruu commented 1 year ago

Describe what feature you'd like. Pseudo-code, mockups, or screenshots of similar solutions are encouraged!

It should be a simple process of adding names and versions.

What type of pull request would this be?

New Feature

Any links to similar examples or other references we should review?

No.

NoTuxNoBux commented 1 year ago

Will support for other Roslyn analyzers such as Roslynator tie into this? I'm happy to create a new ticket for it if not, I guess it depends a bit on how you're planning to do this.

Currently with the vscode package from Unity (and the other fork from wackoisgod) Unity looks for Roslyn analyzer assemblies inside the project folder with a specific asset tag, passes them to the VSCode extension, which in turn generates entries like this:

<ItemGroup>
    <Analyzer Include="/home/user/project/Assets/RoslynAnalyzers/roslynator.analyzers/4.2.0/analyzers/dotnet/cs/Roslynator_Analyzers_Roslynator.Core.dll" />
    ...
</ItemGroup>

I have a bunch of those right now, but they don't appear to be added to the generated Scripts.csproj.

If it's not yet implemented: the Unity approach is to ask you to drop your analyzers as DLLs inside the project and tag them, but installing through NuGet is much better since you can effectively version properly and don't need to push dependencies to your repository. This package may be able to generate the entries just for the C# project files and ignore Unity altogether.

The downside to ignoring Unity is that you won't get the analyzer messages inside the Unity editor, but the upside is that analyzer errors won't block play mode (and you can't turn Roslyn analyzers off in Unity 2022 any more, whilst you could in 2021) and the analyzers won't slow down editor iteration times if all you want is to see them with OmniSharp, your IDE, and/or use them in CI.

EDIT: I just realized I can try creating a OriginalName.csproj.user file to add the additional Roslyn analyzers, since the Microsoft Unity analyzer is already in here and works at build time. The Microsoft Unity analyzer is also not working for me with OmniSharp, though; possibly because a reference to it is missing in the C# project file.

Chizaruu commented 1 year ago

The analyzer works. The problems just aren't showing up :c

I posted a picture in https://github.com/Chizaruu/com.tsk.ide.vscode/issues/48

NoTuxNoBux commented 1 year ago

I'm not sure that's the problem, though (I'm the one that reported it, by the way :smile:), since it didn't happen on Linux and is Windows-specific (I'm on Linux).

I'll try to dig in a bit more to see what the problem is :slightly_smiling_face: .

Chizaruu commented 1 year ago

I'm not sure that's the problem, though (I'm the one that reported it, by the way 😄), since it didn't happen on Linux and is Windows-specific (I'm on Linux).

I'll try to dig in a bit more to see what the problem is 🙂 .

I should really invest some time into creating a new VM for specific Linux edge cases xD

Chizaruu commented 1 year ago

Btw, if you have time, please let me know if the new dev tool extension works, I'm currently locked behind a path issue 😢

I want to know if it solves the analyzer problems ✌️

NoTuxNoBux commented 1 year ago

Btw, if you have time, let me know if the new dev tool extension works, I'm currently locked behind a https://github.com/dotnet/vscode-csharp/issues/5713

It's working for me in ordinary .NET projects and in Unity projects (using the wackoisgod fork that generates SDK-style projects). With this package I'm currently hitting #54, but it still works, probably because the projects are already properly generated (not getting any analyzer messages to show up in VSCode, though, but I'll dig into that later).

EDIT: But, as mentioned, I am on Linux, so I might not be suffering from the same bug :smile:.

Chizaruu commented 1 year ago

Btw, if you have time, let me know if the new dev tool extension works, I'm currently locked behind a dotnet/vscode-csharp#5713

It's working for me in ordinary .NET projects and in Unity projects (using the wackoisgod fork that generates SDK-style projects). With this package I'm currently hitting #54, but it still works, probably because the projects are already properly generated (not getting any analyzer messages to show up in VSCode, though, but I'll dig into that later).

EDIT: But, as mentioned, I am on Linux, so I might not be suffering from the same bug 😄.

It will generate the projects as intended, but your issue stops dotnet from running restore/build 😅

Hopefully I can patch it soon

NoTuxNoBux commented 1 year ago

I fixed my problem with the analyzers not working: it was some shenanigans with indeed not having run dotnet restore in the correct folder :tada: .

FWIW, a heads-up from someone who looked into getting NuGet packages into Unity using standard .NET tooling and built a mostly-working-home-baked-that-probably-doesn't-work-in-all-situations approach for the company he works for before you start on this :smile: : it's unfortunately not just a matter of dotnet restore-ing some additional packages like in ordinary .NET and having Unity pick those up; the process is made much more complex because of Unity. Here are a few problems you will hit:

  1. dotnet restore installs special 'meta packages' such as System.Buffers to the dependency chain so different NuGet packages can depend on different versions of them and have conflict resolution happen correctly.
    • Most of these packages have special _._ assembly files (i.e. none at all) probably because they are built into .NET itself and don't need installing, and sometimes they don't, e.g. shims for System.Text.Json for older versions of .NET.
    • Unity ships its own custom versions of these assemblies in folders such as Unity/2022.3.1f1/Editor/Data/NetStandard/compat/2.1.0/shims/netstandard and - you guessed it - these have different versions that aren't always compatible. These don't appear often for me, and often you can get away with resolving the conflicts manually in .csproj and it will just "work" during builds, but it's hoping for the best.
  2. Unity validates assembly versions to be compatible. If one package wants 4.1 and another wants 4.2 of package A, Unity will complain about that, even though they are obviously compatible according to semver. You can disable this in the Unity project settings.
  3. .NET packages often package the same assembly DLL multiple times: one for .NET standard 2.0, one for .NET 5, and so on; Unity doesn't care and fails hard because you are trying to install the same assembly multiple times - you will need to filter out the appropriate one based on the Unity version and project settings.
  4. Similarly, .NET packages sometimes package native (unmanaged) DLLs: one for Windows x86-64, one for Windows ARM64 (UWP, HoloLens 2), one for Linux ARM64, and so on. In Unity these need to be marked as included only for the platform they target.
  5. Some .NET packages ship their own Roslyn analyzers. If you want these to work inside the Unity Editor itself, they need to be tagged appropriately (see the documentation) or Unity will not pick them up.

If you look at the codebase of e.g. NuGetForUnity, they also have various workarounds for these things. We started out using that and ended up rolling our own in-house thing because it still fails to install e.g. System.Text.Json and couldn't handle Roslyn analyzers properly.

(Off-topic about item 5: starting with Unity 2022 you can't disable Roslyn analyzers inside the Unity Editor any more, and Roslyn analyzer options specified through .editorconfig (required by e.g. Roslynator) aren't passed along by Unity and they don't plan to fix it, so you will get the wrong errors and slower iteration times in the Unity Editor if you take that path. It might be easier to just do what this package currently does and ignore Unity entirely, so you can just run the analyzers in your IDE and CI/CD, leaving Unity behind.)

Chizaruu commented 1 year ago

Thanks for the great advice; I'll see what I can do in my free time. 😄

NoTuxNoBux commented 1 year ago

To answer my own question from before about Roslyn analyzers: I got this working with how this package works right now by making a custom .csproj with the necessary analyzers in, and creating a Scripts.csproj.user (or whatever project you want it in; I have a Unity asmdef for Scripts, hence the name) that just links to that project additionally.

This pulls in the necessary analyzers I want, which works for OmniSharp and dotnet build :tada: .