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

WIN32: add start menu tiles for Win8 and Win10 #2661

Closed
wants to merge 4 commits into from

Conversation

@SirYodaJedi
Copy link

@SirYodaJedi SirYodaJedi commented Dec 2, 2020

These two commits add the necessary files in order to have a fancy orange background when ScummVM is pinned to the Start Menu/Screen in Windows 8, 8.1, and 10. It also should add them to the installer so that they are actually used; I'm not sure if I did that correctly.

I also created a version for the pre-2.1 icon, which I subjectively think looks better.
ScummVM_Tiles_classic.zip

Example using the old icon (these commits contain the new icon and official current shade of orange):
screenshot

@SirYodaJedi SirYodaJedi marked this pull request as ready for review Dec 2, 2020
@raziel-
Copy link
Contributor

@raziel- raziel- commented Dec 2, 2020

Agreed...that "slimy" look had something distinctive, i loved it, but that's maybe just me :-)

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Dec 2, 2020

Any idea how to refresh the icon cache after copying over those files? It seems that it has no effect at all, at least on my Win10 machine :/

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Dec 2, 2020

On a more constructive side: The changes in the Inno Setup script are correct, so I don't expect any issues here.

However, I recommend adding the new resource files to the scummvm.rc file (see https://github.com/scummvm/scummvm/blob/master/dists/scummvm.rc), so they get bundled right into the executable.

Loading

@SirYodaJedi
Copy link
Author

@SirYodaJedi SirYodaJedi commented Dec 3, 2020

However, I recommend adding the new resource files to the scummvm.rc file (see https://github.com/scummvm/scummvm/blob/master/dists/scummvm.rc), so they get bundled right into the executable.

Good idea, although I'm not sure how to call it. Microsoft's documentation on custom start tiles only says that it is possible to call from a resource, but not how to do so. It's possible to remove the lines with the path and it'll use the default embedded .ico instead, but it'll be smaller and blurrier (that's what Origin is doing in the above screenshot).

Any idea how to refresh the icon cache after copying over those files? It seems that it has no effect at all, at least on my Win10 machine :/

Setting any random custom icon for the .lnk that pops up when you Open File Location worked for me (can't remember if I had to clear the thumbnail cache in Disk Cleanup first).

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Dec 4, 2020

Good idea, although I'm not sure how to call it. Microsoft's documentation on custom start tiles only says that it is possible to call from a resource, but not how to do so. It's possible to remove the lines with the path and it'll use the default embedded .ico instead, but it'll be smaller and blurrier (that's what Origin is doing in the above screenshot).

Okay, I see - it seems that we can't use the XML or MANIFEST types here, so maybe you can simply include them as FILE. This should make the resources accessible directly via the .xml file.

Any idea how to refresh the icon cache after copying over those files? It seems that it has no effect at all, at least on my Win10 machine :/

Setting any random custom icon for the .lnk that pops up when you Open File Location worked for me (can't remember if I had to clear the thumbnail cache in Disk Cleanup first).

Thanks, I'll give this a try.

Loading

dists/scummvm.rc Show resolved Hide resolved
Loading
@SirYodaJedi SirYodaJedi marked this pull request as draft Dec 5, 2020
@lotharsm
Copy link
Member

@lotharsm lotharsm commented Dec 5, 2020

Okay, for me it now kinda works. The icon itself is far bigger now, so that's an improvement. However, I don't get the orange background at all.

Unfortunately, it seems that this is currently far from being an "out of the box" experience, since I indeed had to modify the icon manually to a random icon in order for the manifest to have any effect.

EDIT: Please don't get me wrong! I absolutely like the feature, it just seems that at least on my machine, I have some issues with it.

Loading

@SirYodaJedi
Copy link
Author

@SirYodaJedi SirYodaJedi commented Dec 5, 2020

I'm not sure why it's loading the icons but not the background colour for you, strange. I'm not too familiar with how this feature works (I kinda just copied and modified a template and it worked for me), so I'll just leave this open as a draft Pull Request for now so that others can see and hopefully chime in. I'll also play around with loading from the resource file (and learning how to compile using a makefile), but there's not much I can do if it isn't working on your machine.

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Jan 26, 2021

@SirYodaJedi By accident, I found out why it's not working for me.

Yesterday, I had to remove a Windows update in order to do an in-place update to a later version again (stupid Windows updates breaking again). When I removed the "20H2 Enhancement/Experience Pack" (aka. the 20H2 update), the tile was shown correctly.

Now that I'm on the current patchlevel again, the orange tile background is gone - so it seems to be partially tied to the Windows version - maybe it's relying on some legacy stuff?

Loading

@SirYodaJedi
Copy link
Author

@SirYodaJedi SirYodaJedi commented Jan 26, 2021

Forgot this PR was a thing.

maybe it's relying on some legacy stuff?

Potentially. I haven't been pushed 20H2 yet, so I'm not sure how it would affect things. Had this discussion in the PCGamingWiki Discord a couple of months back; meant to share.
image

Loading

@SirYodaJedi
Copy link
Author

@SirYodaJedi SirYodaJedi commented Feb 28, 2021

Shower thought (or close enough to my shower, anyway; had a conversation about tiles with someone else just now):
If we still wanted the orange background (despite that M$ opts for a just drop shadow now), I'd probably be possible to bake it into the tile PNG, rather than defining it as a hex value. I have a feeling that that's how Spotify and Tobii are doing it in that screenshot of Aemony's screenshot.

Of course, this is assuming Win10 212H doesn't change how things work again 😓

Loading

@digitall
Copy link
Member

@digitall digitall commented Apr 27, 2021

This work looks relatively self-contained adding only a few image files to the Win32 distribution along with an XML manifest so it only affects the WIN32 release version. @ccawley2011 : Any thoughts?

Loading

@sluicebox
Copy link
Member

@sluicebox sluicebox commented Jun 16, 2021

It looks like the icon files are being included in the installer and also embedded in the PE as resources. Shouldn't it be one or the other?

It's been two years since I reported that buildbot duplicates all the PE resources as files in the nightly zip, bloating it significantly, and since then even foone has publicly lambasted us to no avail (I guess we're just powerless to fix this?), so now I'm fighting over every byte =)

Loading

@SirYodaJedi
Copy link
Author

@SirYodaJedi SirYodaJedi commented Jun 17, 2021

It looks like the icon files are being included in the installer and also embedded in the PE as resources. Shouldn't it be one or the other?

I forgot to remove the standalone versions from the .iss after adding them to the .rc; I have just rectified that. Do I also need to update the "path" to the PNGs, either in the .rc or the VisualElementsManifest.xml?

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Jul 4, 2021

It looks like the icon files are being included in the installer and also embedded in the PE as resources. Shouldn't it be one or the other?

It's been two years since I reported that buildbot duplicates all the PE resources as files in the nightly zip, bloating it significantly, and since then even foone has publicly lambasted us to no avail (I guess we're just powerless to fix this?), so now I'm fighting over every byte =)

Indeed, it's been a loooong time. In the meantime, I implemented the --no-builtin-resources flag to configure which does exactly that - exclude the files from being merged into the main executable. This is also a possible way to finally fix the "duplicate everything" issue. Another way would be to modify the build scripts on the buildbot itself. However, especially for the debug builds produced by buildbot it's a nice touch to be able to simply swap out files on-the-fly without having to rebuild the .exe itself.

I forgot to remove the standalone versions from the .iss after adding them to the .rc; I have just rectified that. Do I also need to update the "path" to the PNGs, either in the .rc or the VisualElementsManifest.xml?

I'm not entirely sure about VisualElementsManifest.xml itself. The .rc path is correct as-is.

Loading

@lephilousophe
Copy link
Member

@lephilousophe lephilousophe commented Jul 4, 2021

Indeed, it's been a loooong time. In the meantime, I implemented the --no-builtin-resources flag to configure which does exactly that - exclude the files from being merged into the main executable. This is also a possible way to finally fix the "duplicate everything" issue. Another way would be to modify the build scripts on the buildbot itself. However, especially for the debug builds produced by buildbot it's a nice touch to be able to simply swap out files on-the-fly without having to rebuild the .exe itself.

It's on my TODO, especially since PR #2999. It should be really easy to have a target for the buildbot which wouldn't duplicate every file.

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Nov 1, 2021

I'm afraid we need to close this.

While I appreciate the feature in general, it is basically "dead": I doesn't work on Windows 10 20H2 already and I couldn't get it to work on Windows 11 either.

Loading

@lotharsm lotharsm closed this Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants