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.

Sign up for GitHub

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

Jump to bottom

Fix 39 non-obsolete warnings #33794

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix 39 non-obsolete warnings #33794

wants to merge 8 commits into from

Conversation

SpaceManiac
Copy link
Contributor

@SpaceManiac SpaceManiac commented Dec 9, 2024 •
edited
Loading

About the PR

Pursuant to #33279.

Technical details

Fix:

  • Missing base call in ClientGameTicker.Initialize by calling base since it seems harmless
  • Missing base call in CableVisualizerSystem.Initialize by detaching it from VisualizerSystem entirely
  • Unused private variables
  • Private/internal starts-with-underscore naming rule violations
  • Duplicate dependency on IAdminLogManager in GhostSystem
  • Remove unused redeclaration without use of new in InstrumentBoundUserInterface
  • Remove never-emitted events in DockObject UI and their never-called listeners
  • Await non-async Tasks in a try-catch and log errors
  • Unused usings, but only in files already touched

Silence:

  • Missing base call in DoAfterSystem.Update since it seems intentional

This doesn't fix any warning with "obsolete" in the name, RA0026, or these monsters:

.../space-station-14/BuildChecker/BuildChecker.csproj : warning NU1503: Skipping restore for project '.../space-station-14/BuildChecker/BuildChecker.csproj'. The project file may be invalid or missing targets required for restore. [.../space-station-14/SpaceStation14.sln]

/usr/share/dotnet/sdk/8.0.110/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.targets(284,5): warning NETSDK1206: Found version-specific or distribution-specific runtime identifier(s): debian-x64, fedora-x64, opensuse-x64, rhel-x64, win7-arm, win7-x64, win7-x86. Affected libraries: Libuv. In .NET 8.0 and higher, assets for version-specific and distribution-specific runtime identifiers will not be found by default. See https://aka.ms/dotnet/rid-usage for details. [.../space-station-14/Pow3r/Pow3r.csproj]

CSC : warning AD0001: Analyzer 'NUnit.Analyzers.DisposeFieldsInTearDown.DisposeFieldsAndPropertiesInTearDownAnalyzer' threw an exception of type 'System.ArgumentException' with message 'Syntax node is not within syntax tree'. [.../space-station-14/Content.IntegrationTests/Content.IntegrationTests.csproj]

Requirements

@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted Changes: UI Changes: Might require knowledge of UI design or code. size/L Denotes a PR that changes 100-1000 lines. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Dec 9, 2024
@ScarKy0 ScarKy0 added P3: Standard Priority: Default priority for repository items. T: Cleanup Type: Code clean-up, without being a full refactor or feature D3: Low Difficulty: Some codebase knowledge required. A: Core Tech Area: Underlying core tech for the game and the Github repository. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Dec 9, 2024
@PJB3005 PJB3005 self-assigned this Dec 14, 2024
PJB3005
PJB3005 reviewed Dec 14, 2024
View reviewed changes
Content.Client/Instruments/UI/InstrumentBoundUserInterface.cs Outdated Show resolved Hide resolved
Content.Client/GameTicking/Managers/ClientGameTicker.cs Show resolved Hide resolved
Content.Client/Shuttles/UI/DockObject.xaml.cs Outdated Show resolved Hide resolved
Content.Server/Salvage/SalvageSystem.Magnet.cs Outdated
@@ -45,7 +45,7 @@ private void OnMagnetClaim(EntityUid uid, SalvageMagnetComponent component, ref
return;
}

TakeMagnetOffer((station.Value, dataComp), args.Index, (uid, component));
_ = TakeMagnetOffer((station.Value, dataComp), args.Index, (uid, component));
Copy link
Member

Choose a reason for hiding this comment

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

Discarding this task is not the right choice, it'll cause the error result to be poorly handled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would catch-all-and-log suffice? Seems like it's too late to do anything else of value.

Content.Server/Speech/EntitySystems/ReplacementAccentSystem.cs Outdated Show resolved Hide resolved
Content.Shared/Chat/SharedChatSystem.cs Outdated Show resolved Hide resolved
@PJB3005 PJB3005 added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Dec 14, 2024
@SpaceManiac SpaceManiac changed the title Fix 41 non-obsolete warnings Fix 39 non-obsolete warnings Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PJB3005 PJB3005 PJB3005 left review comments

@metalgearsloth metalgearsloth Awaiting requested review from metalgearsloth metalgearsloth is a code owner

@Partmedia Partmedia Awaiting requested review from Partmedia Partmedia is a code owner

@ficcialfaint ficcialfaint Awaiting requested review from ficcialfaint ficcialfaint is a code owner

@TheShuEd TheShuEd Awaiting requested review from TheShuEd TheShuEd is a code owner

Assignees

@PJB3005 PJB3005

Labels
A: Core Tech Area: Underlying core tech for the game and the Github repository. Changes: UI Changes: Might require knowledge of UI design or code. D3: Low Difficulty: Some codebase knowledge required. P3: Standard Priority: Default priority for repository items. S: Awaiting Changes Status: Changes are required before another review can happen size/L Denotes a PR that changes 100-1000 lines. T: Cleanup Type: Code clean-up, without being a full refactor or feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SpaceManiac @PJB3005 @ScarKy0