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

Update c# language version to 9 #18830

Merged
merged 10 commits into from
Jun 25, 2022
Merged

Update c# language version to 9 #18830

merged 10 commits into from
Jun 25, 2022

Conversation

peppy
Copy link
Member

@peppy peppy commented Jun 24, 2022

No description provided.

@frenzibyte frenzibyte changed the title Update c# language version to 10 Update c# language version to 9 Jun 24, 2022
@peppy
Copy link
Member Author

peppy commented Jun 24, 2022

I've applied remaining changes in line with what was done on the framework side, plus bumped the framework here (will require a fix, nuget not working for me right now so can't immediately address, feel free to fix anyone else).

@bdach
Copy link
Collaborator

bdach commented Jun 24, 2022

Build failures cannot be easily resolved, due to the following TextureLoaderStore usage in example ruleset:

public override Drawable CreateIcon() => new Sprite
{
Texture = new TextureStore(new TextureLoaderStore(CreateResourceStore()), false).Get("Textures/coin"),
};

The ctor in question was internalised framework-side in ppy/osu-framework#5240 and now there is no good way to access GameHost from the ruleset to create a texture loader store.

Related: #11240

May want to revert that particular breaking change framework-side for now.

@peppy
Copy link
Member Author

peppy commented Jun 24, 2022

Painful. Wanna make a PR reverting that if you're able to?

@bdach
Copy link
Collaborator

bdach commented Jun 24, 2022

Done (ppy/osu-framework#5266).

@bdach
Copy link
Collaborator

bdach commented Jun 24, 2022

Bumped framework again (manually, so buyer beware), as well as fixed the failures from the framework breaking change to texture stores in a83c45b. @frenzibyte would appreciate a double-check of the latter if/when able, if it matches the changes you had queued.

smoogipoo
smoogipoo previously approved these changes Jun 24, 2022
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeRedundantParentheses/@EntryIndexedValue">WARNING</s:String>
<s:Boolean x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeRedundantParentheses/@EntryIndexRemoved">True</s:Boolean>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeTypeMemberModifiers/@EntryIndexedValue">WARNING</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeTypeModifiers/@EntryIndexedValue">WARNING</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=AssignedValueIsNeverUsed/@EntryIndexedValue">HINT</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=AssignmentIsFullyDiscarded/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=AssignNullToNotNullAttribute/@EntryIndexedValue">WARNING</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=AsyncVoidMethod/@EntryIndexedValue">WARNING</s:String>
Copy link
Collaborator

Choose a reason for hiding this comment

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

How were these dotsettings changes made? Looks like the file may have been copy-pasted verbatim from the framework directory to the game's, but in the meantime this subtly changed the set of enabled inspections (like the "async void" one above).

Copy link
Member Author

Choose a reason for hiding this comment

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

Overwrote the file with framework version and undid anything that looked out of place (or caused changes in warnings/errors).

Welcome to remove these changes if you want to look into it further, I was just making sure it didn't affect already existing code, mostly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have changes queued that revert all inspection downgrades that happened as a result of the procedure you described, just want to run them past inspectcode again to make sure I haven't missed anything.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pushed changes in e0c2228. Now the diff of osu.sln.DotSettings only contains additions of new rules (and one whitespace fix).

@peppy peppy merged commit f15698d into ppy:master Jun 25, 2022
@peppy peppy deleted the c-sharp-10 branch June 27, 2022 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants