Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diagnostics for incompatible SMAPI mods #1129

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

erri120
Copy link
Member

@erri120 erri120 commented Mar 26, 2024

Resolves #1119.

This PR adds two new diagnostics for Stardew Valley:

2024-03-26_12-06-56
2024-03-26_12-07-10

The data for these diagnostics are sourced from the internal "mod database" file located at smapi-internal/metadata.json. SMAPI itself reads this file at startup to issue warnings and errors, we mirror this behavior in the App with diagnostics.

Other changes:

  • Upgrades the SMAPI submodule from 3.18.6 to 4.0.2.
  • Fixes the diagnostic source generator when using raw string literals, multi-line strings, or escaped strings.
  • Made the DiagnosticManager more robust when one emitter throws an exception.
@erri120 erri120 added the Epic: Diagnostics This is related to the Diagnostic System label Mar 26, 2024
@erri120 erri120 added this to the v0.4 milestone Mar 26, 2024
@erri120 erri120 requested a review from a team March 26, 2024 11:11
@erri120 erri120 self-assigned this Mar 26, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 5.73770% with 230 lines in your changes are missing coverage. Please review.

Project coverage is 52.47%. Comparing base (38bb865) to head (7491737).
Report is 4 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1129      +/-   ##
==========================================
- Coverage   53.52%   52.47%   -1.06%     
==========================================
  Files         676      678       +2     
  Lines       22525    22735     +210     
  Branches     1726     1743      +17     
==========================================
- Hits        12057    11930     -127     
- Misses      10073    10406     +333     
- Partials      395      399       +4     
Flag Coverage Δ
Linux 52.41% <5.73%> (-0.43%) ⬇️
Windows ?
clean_environment_tests 52.47% <5.73%> (-1.05%) ⬇️
macOS 51.87% <5.73%> (-0.45%) ⬇️
network_tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...rc/Games/NexusMods.Games.StardewValley/Services.cs 100.00% <100.00%> (ø)
.../Games/NexusMods.Games.StardewValley/TypeFinder.cs 100.00% <100.00%> (ø)
...ames.StardewValley/Installers/SMAPIModInstaller.cs 0.00% <0.00%> (ø)
...mes.StardewValley/Models/SMAPIModDatabaseMarker.cs 0.00% <0.00%> (ø)
...cs/DiagnosticTemplateIncrementalSourceGenerator.cs 84.66% <66.66%> (ø)
...mes/NexusMods.Games.StardewValley/StardewValley.cs 13.55% <0.00%> (-3.69%) ⬇️
...rdewValley/Emitters/DependencyDiagnosticEmitter.cs 0.00% <0.00%> (ø)
...exusMods.Games.StardewValley/WebAPI/SMAPIWebApi.cs 0.00% <0.00%> (ø)
...ns/NexusMods.Abstractions.Installers/Extensions.cs 47.36% <9.09%> (-52.64%) ⬇️
...s.Games.StardewValley/Installers/SMAPIInstaller.cs 0.00% <0.00%> (ø)
... and 4 more

... and 32 files with indirect coverage changes

@Al12rs Al12rs self-requested a review March 26, 2024 13:21
Comment on lines -34 to -40
/// <summary>
/// Creates a new instance of <see cref="SMAPIModInstaller"/>.
/// </summary>
/// <param name="serviceProvider"></param>
/// <returns></returns>
public static SMAPIModInstaller Create(IServiceProvider serviceProvider) => new(serviceProvider);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this simply unused?

Copy link
Member Author

@erri120 erri120 Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was used, but it's a weird pattern, so I removed it. The installer is now registered in DI.

@erri120 erri120 merged commit 8b66b04 into Nexus-Mods:main Mar 26, 2024
7 checks passed
@erri120 erri120 deleted the feat/1119-obsolete-smapi-mods branch March 26, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic: Diagnostics This is related to the Diagnostic System
3 participants