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

Add infrastructure for embedding Win32 icons #308

Closed
wants to merge 1 commit into from
Closed

Add infrastructure for embedding Win32 icons #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added osu.Game.Resources/Icons/101-Lazer.ico
View file
Open in desktop
Binary file not shown.
7 changes: 7 additions & 0 deletions osu.Game.Resources/Icons/Icons.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace osu.Game.Resources.Icons
{
public static class Icons
{
public static Win32Icon Lazer => new Win32Icon(101);
}
}
4 changes: 4 additions & 0 deletions osu.Game.Resources/Icons/Icons.rc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#include <winnt.rh>
LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL

101 ICON "101-Lazer.ico"
Binary file added osu.Game.Resources/Icons/Icons.res
View file
Open in desktop
Binary file not shown.
12 changes: 12 additions & 0 deletions osu.Game.Resources/Icons/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# How to generate `Icons.res`

1. Open `Developer PowerShell for VS 2022`
2. `cd` into this directory
3. Run `RC.exe /v Icons.rc`

# Adding new icons

1. Put the `.ico` file with a reasonable name into this folder
2. Open `Icons.rc` as a text file and add the relevant lines
3. Add the C# definition to `Icons.cs`
4. Regenerate `Icons.res`
29 changes: 29 additions & 0 deletions osu.Game.Resources/Icons/Win32Icon.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using System.Reflection;

namespace osu.Game.Resources.Icons
{
public class Win32Icon
{
public readonly Assembly Assembly;
public readonly int Id;

/// <summary>
/// String that can be used with the <c>DefaultIcon</c> key in the registry to point to this icon.
/// </summary>
// The icon id needs to be negative, see https://devblogs.microsoft.com/oldnewthing/20100505-00/?p=14153
public string RegistryString => $"{Assembly.Location},{-Id}";
Copy link
Collaborator

Choose a reason for hiding this comment

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

So my primary question here would be.......... can we skip all of this legacy nonsense and just use plain ICO files?

I'd prefer that we didn't have to use some win32 resource incantations and some executables that are only available on windows and also only if you have visual studio enabled.

I would much rather put the .ico assets directly in osu.Game.Desktop and point the registry entries at a plain ICO file rather than deal with whatever this thing is. The fact that this is having to cite a Raymond Chen blogpost is a red flag in my eyes in that this is underdocumented legacy cruft, because if that is the best available citation then oh boy.

Copy link
Collaborator

@bdach bdach Feb 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In fact as far as I can tell even microsoft doesn't wanna deal with this stuff anymore:

image

The above is a VS Code .md file association. It points at a plain .ico.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would much rather put the .ico assets directly in osu.Game.Desktop and point the registry entries at a plain ICO file

I agree, would make things simpler.

I tried including the icons as Content and it seems to work well. I just tested this with velopack and a sample app, and the icons are available in the install directory.

<ItemGroup>
    <Content Include="Icons\*.ico"
             CopyToPublishDirectory="PreserveNewest"
             CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
C:\Users\...\AppData\Local\VPTest> tree /F
│   Update.exe
│
├───current
│   │   Microsoft.Extensions.DependencyInjection.Abstractions.dll
│   │   Microsoft.Extensions.Logging.Abstractions.dll
│   │   NuGet.Versioning.dll
│   │   sq.version
│   │   Velopack.dll
│   │   VelopackTest.deps.json
│   │   VelopackTest.dll
│   │   VelopackTest.exe
│   │   VelopackTest.runtimeconfig.json
│   │
│   └───Icons
│           lazer.ico
│
└───packages
        VPTest-1.0.0-full.nupkg


public override string ToString() => RegistryString;

public Win32Icon(Assembly assembly, int id)
{
Assembly = assembly;
Id = id;
}

internal Win32Icon(int id)
: this(OsuResources.ResourceAssembly, id)
{
}
}
}
1 change: 1 addition & 0 deletions osu.Game.Resources/osu.Game.Resources.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<PackageReleaseNotes>Automated release.</PackageReleaseNotes>
<copyright>Copyright (c) 2019 ppy Pty Ltd</copyright>
<PackageTags>osu game resources</PackageTags>
<Win32Resource>Icons/Icons.res</Win32Resource>
</PropertyGroup>
<ItemGroup>
<EmbeddedResource Include="Beatmaps\**\*" />
Expand Down