Bannerlord.Module.Template icon indicating copy to clipboard operation
Bannerlord.Module.Template copied to clipboard

Add support for vscode

Open Mrcubix opened this issue 2 years ago • 5 comments

Premises

vscode cannot make use of the Properties/launchSettings.json file as it uses an unsupported commandName, only project is supported in vscode.

What is this PR for

This PR aim to add launch & debug support of modules in vscode.

Necessary files

To achieve this, 3 files are necessary:

  • launch.json
  • settings.json
  • tasks.json

Only a single of those files, launch.json, requires the use of variables to make a newly created module, work properly. those variables are stored in the settings.json files and is used by vscode to feed the appropriate values.

the 2 variables that are required are:

  • The game binary folder (by default, Win64_Shipping_Client)
  • The name of the module to debug

Unfortunately, the addition of 3 template symbols, including a single parameter are required.

Template symbols

-c or --code will need to be used if the user intend to use vscode as their IDE.

Generated type symbols are used to replace the placeholders present within settings.json initially.

  1. gameBinariesFolder:

If the gameWindows parameter is true, gameBinariesFolder's value is the Win64_Shipping_Client folder, if not then, If the gameWindowsStore parameter is true , gameBinariesFolder's value is the Gaming.Desktop.x64_Shipping folder.

  1. displayName:

a separate symbol to moduleName that control its presence, in the case that it is not, sourceName will be used (the name symbol represent that value)

Folder exclusion

In the case that the user specify the use of vscode, the Properties folder will be excluded from the template. The opposite, will be true if the user does not specify any values or false, .vscode will be excluded.

What could be done on top of what is currently provided

If necessary, i can add the publish and watch tasks back to tasks.json

Mrcubix avatar Jul 26 '23 20:07 Mrcubix

This adds some concerns if we're implementing it this way

  • There are two independent ways for configuration - MSBuild and VSCode
  • Hardcoded gameBinariesFolder - if you change the BANNERLORD_GAME_DIR env, you'll also need to change the variable

I think that adding a new VSCode specific templates for Standard/Sdk would be better - we could move all of the dynamic variables stored in MSBuild to settings.json. When building, MSBuild will look into settings.json for the values. I should be able to adapt the changes as a new project if you're not able. It kinda doubles the codebase, but it should provide a better user experience

Also, as a comment, I think it would be better to file a bug report to VSCode/ C# Dev Kit extension for the lack of executable support in commandName. Looks like someone reported an IIS error and it's being fixed

Aragas avatar Jul 27 '23 06:07 Aragas

  • There are two independent ways for configuration - MSBuild and VSCode
  • I think it would be better to file a bug report to VSCode/ C# Dev Kit extension for the lack of executable support in commandName. Looks like someone reported an IIS error and it's being fixed
  • I think that adding a new VSCode specific templates for Standard/Sdk would be better

https://code.visualstudio.com/docs/csharp/debugger-settings#_launchsettingsjson-support

Seems like vscode originally planned it to be this way, i agree that mixing both configs isn't the best of ideas, that would reduce the complexity of templates, however, that will make maintaining the templates more difficult.

  • Hardcoded gameBinariesFolder - if you change the BANNERLORD_GAME_DIR env, you'll also need to change the variable

I don't really see what your concern is about here, this would mostly be an issue if the BANNERLORD_GAME_DIR variable isn't set properly, or if you don't restart vscode after changing the variable. If you move the game to another location on steam or the store, the binary folder will stay the same.

  • we could move all of the dynamic variables stored in MSBuild to settings.json. When building, MSBuild will look into settings.json for the values.

This would require the use of an msbuild task just for that purpose, which is somewhat confusing to use, even with the documentation provided. I will try playing around with it since, at least, it has a more proper documentation than templates does.

EDIT: i may have misunderstood what you meant for that last part.

Mrcubix avatar Jul 27 '23 22:07 Mrcubix

I've added my own interpretation of VSCode, but as you said, MSBuild code is a mess, especially the way to read JSON without any JSON dependency https://github.com/BUTR/Bannerlord.Module.Template/tree/Mrcubix_master_BUTR

Aragas avatar Jul 29 '23 18:07 Aragas

you should be able to make use of System.Text.Json just for this task, it shouldn't pose any issues, or worst case scenario, you can make use of the game's newtonsoft dependency.

Mrcubix avatar Jul 29 '23 19:07 Mrcubix

you should be able to make use of System.Text.Json just for this task, it shouldn't pose any issues, or worst case scenario, you can make use of the game's newtonsoft dependency.

Surprisingly, System.Text.Json isn't available in MSBuild because the target is netstandard2.0, as I understand

Reusage of Newtonsoft could work when the game folder is available, and reference assemblies are not set, but there are other issues We need Newtonsoft.Json to get the Game folder where Newtonsoft.Json is, so that's a paradox. It won't also work with having Newtonsoft.Json as a NuGet package, because we need to resolve the game folder before the project dependencies are resolved. So we need some built-in mechanism to read Json without external deps, where System.Text.Json should have helped

Aragas avatar Jul 29 '23 19:07 Aragas

We should be able to revisit this, looks like we could potentially reuse launchSettings.json

Aragas avatar Jun 28 '24 14:06 Aragas

So it's marked as merged, but it not "really" merged. @Mrcubix if you're still modding Bannerlord, can you create a standard SDK/non SKD template and check whether such support for VSCode is correct and sufficient?

Aragas avatar Jun 28 '24 20:06 Aragas

I have moved on from bannerlord modding but i can check tomorrow, still useful for other templates i might work on.

Mrcubix avatar Jun 29 '24 04:06 Mrcubix

So there are multiple issues with the SDK template:

  • The project generated from the template is created in another folder than the current folder, resulting in VScode being unable to find the launch.json file. Usually, templates just dump their content in the current folder.
  • No default profile is being specified, which could lead to some errors,
  • If i open the generated directory instead in vscode, and try to debug the module, then i get the following error:
[...]\Bannerlord\Testing\Template-testing\Bannerlord.Template.Test\Bannerlord.Template.Test.csproj' is not an executable project.

Update: Noticed again that only Profiles of type Project are compatible with vscode, so these templates won't work, see: https://code.visualstudio.com/docs/csharp/debugger-settings#:~:text=Properties/launchSettings.json%22-,Restrictions The same applies to dotnet CLI.

Mrcubix avatar Jun 29 '24 15:06 Mrcubix

So there are multiple issues with the SDK template:

  • The project generated from the template is created in another folder than the current folder, resulting in VScode being unable to find the launch.json file. Usually, templates just dump their content in the current folder.
  • No default profile is being specified, which could lead to some errors,
  • If i open the generated directory instead in vscode, and try to debug the module, then i get the following error:
[...]\Bannerlord\Testing\Template-testing\Bannerlord.Template.Test\Bannerlord.Template.Test.csproj' is not an executable project.

Update: Noticed again that only Profiles of type Project are compatible with vscode, so these templates won't work, see: https://code.visualstudio.com/docs/csharp/debugger-settings#:~:text=Properties/launchSettings.json%22-,Restrictions The same applies to dotnet CLI.

Not sure I can reproduce the issues. So I've create the template with two methods

  • dotnet new blmodfx - will treat the current directory as the project name
  • `dotnet new blmodfx --name "MyModule" - will create a new folder MyModule with the content

Do you mean the VSCode profile or the profile from launchSettings? I'm able to select Start Debugging [net472] (Steam/GOG/Epic) and it starts the game as it should - with correct args, so it looks like Executable is working correctly in VSCode

Aragas avatar Jun 29 '24 16:06 Aragas

This is all using blmodsdk.

Do you mean the VSCode profile or the profile from launchSettings?

launchSettings.json

I'm able to select Start Debugging [net472] (Steam/GOG/Epic) and it starts the game as it should - with correct args, so it looks like Executable is working correctly in VSCode

I tried multiple times, none of which works, also tried on another mostly clean machine just to make sure it wasn't a setup issue on my side.

Mrcubix avatar Jun 29 '24 16:06 Mrcubix

Now, with blmodfx:

  1. dotnet new blmodfx
  2. Try to debug
  3. Get the error '[...]\Bannerlord\Testing\template_test\template_test.csproj' is not an executable project.

Mrcubix avatar Jun 29 '24 16:06 Mrcubix

Now, with blmodfx:

  1. dotnet new blmodfx
  2. Try to debug
  3. Get the error '[...]\Bannerlord\Testing\template_test\template_test.csproj' is not an executable project.

Just to be sure, both ms-dotnettools.csharp and ms-dotnettools.csdevkit are installed?

Aragas avatar Jun 29 '24 17:06 Aragas

I can confirm that without ms-dotnettools.csharp it will tell that dotnet is not supported and without ms-dotnettools.csdevkit it states 'c:\Git\f\f.csproj' is not an executable project.

Aragas avatar Jun 29 '24 17:06 Aragas

I definitely need to add extensions.json with their recommendations

{
    "recommendations": [
        "ms-dotnettools.csharp",
        "ms-dotnettools.csdevkit"
    ]
}

Aragas avatar Jun 29 '24 17:06 Aragas

ms-dotnettools.csdevkit is not installed as it is not Open source, has a very restrictive license, has telemetry parts that cannot be opted out from & is not necessary for C# dev.

Mrcubix avatar Jun 29 '24 17:06 Mrcubix

Okay, then we'll need a non proprietary solution still, but we don't have yet an alternative solution which would not require a new template. But even with the template those issues still persist

Aragas avatar Jun 29 '24 17:06 Aragas