DolphinQT: Gives option to add desktop shortcut #9438
Conversation
When a game is selected, the option to add a shortcut of the game to the desktop is given. Uses native Windows API since Qt lacks support for adding shortcuts.
shuffle2
force-pushed
the
add-shortcut-to-desktop
branch
from
b3d24f9
to
23e565d
Compare
| #ifdef _WIN32 | ||
| bool GameList::AddShortcutToDesktop() | ||
| { | ||
| auto init = wil::CoInitializeEx_failfast(COINIT_APARTMENTTHREADED); |
There was a problem hiding this comment.
Not sure if failfast is desirable here - COM may already be initialized in a different threading model, and I don't think this code must use an apartment threaded model?
There was a problem hiding this comment.
That is (I think?) the only wrapper provided by WIL. Multithreaded would not work here as it's on a GUI thread. Specifying multithreaded resulted in an assert, so I think it's working as intended? However, I didn't investigate further to find out what is calling CoInitialize on this thread beforehand.
All CoInitialize* calls on a specific thread should specify the same threading model, even though it's possible to specify multiple threading models (and ignore the returned error code). I think it's better to be explicit about the expectation (which is probably why WIL only exposes a _failfast version of the API).
There was a problem hiding this comment.
It's possible - but if you don't want to make a strong assumption like this, you could. My WASAPI PR realizes this by calling CoInitializeEx "manually" and then activating/deactivating the RAII CoUnitialize call basing on the result.
There was a problem hiding this comment.
I noticed cubeb has something similar. I'd rather use what's provided by WIL, and I don't think making a strong assumption is a problem here. Ideally, all usages of CoInitialize would make strong assumptions (therefor if they trigger fastfail, it's a good indication some code is actually being used incorrectly).
If you want to make a wrapper like cubeb's, maybe consider moving it to some more common location in dolphin?
|
ah, this looks much nicer than the non-WIL version! |
When a game is selected, the option to add a shortcut of the game to the desktop is given. Uses native Windows API since Qt lacks support for adding shortcuts.
This is a modification of #9365 , using WIL and handling all error/memory leak cases correctly (hopefully🙂 )