Pupix / rift-explorer

🛠 Explore the API of the League of Legends client
MIT License
555 stars 73 forks source link

Replace deprecated WMIC #156

Closed Nerdsie closed 2 years ago

Nerdsie commented 2 years ago

WMIC is deprecated, even removed in Windows 11 dev builds. Replace its usage with powershell Get-CimInstance

Morilli commented 2 years ago

Replacing WMIC is definitely a good thing; I wasn't aware that it was actively removed from Win11+.

However, your current method seems rather complicated in terms of getting the actual commandline. I'm not too versed with all the possible windows commands, but how about just using something like this?

(Get-Process RiotClientServices)[0].CommandLine
Nerdsie commented 2 years ago

I think if all we're using is the path we could use (Get-Process RiotClientServices)[0].Path

Nerdsie commented 2 years ago

Actually, we're using --launch-product and --launch-patchline command line arguments, which aren't included in .Path, so my proposed solution above won't work.

As far as I can tell, despite how complicated the command to get commandline seems, that is the way it must be done. Get-Process doesn't seem to include CommandLine.

The best I could do is replace
Get-CimInstance -Query "SELECT * from Win32_Process WHERE name LIKE '${RIOTCLIENT_PROCESS}'" | Select-Object CommandLine | fl with
Get-CimInstance Win32_Process -Filter "name = 'RiotClientServices.exe'" | select CommandLine | fl

which does look a little cleaner but is still a bit long.

Morilli commented 2 years ago

As far as I can tell, despite how complicated the command to get commandline seems, that is the way it must be done. Get-Process doesn't seem to include CommandLine.

Did you read my proposed command above? (Get-Process RiotClientServices)[0].CommandLine does include the entire commandline of the process.

Nerdsie commented 2 years ago

image

Doesn't seem to work for me. What powershell version do you have? get-host | select-object version I'm on 5.1.19041.1320

Morilli commented 2 years ago

Hmm, my default powershell version is 5.1.22494.1000, and indeed, it doesn't work there. I usually use the cross-platform version which is currently version 7.2.0.

To ensure the command will work correctly, it's probably best to use one that is guaranteed to work on all systems that have powershell.

Nerdsie commented 2 years ago

Okay, pushed a simpler command that also simplified the required normalization.

Morilli commented 2 years ago

While some of the logic surrounding this still seems a little complex, this is not your fault, so I think this is fine to merge (I also cannot test on mac to confirm changes would work there)