-
Notifications
You must be signed in to change notification settings - Fork 156
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
Migrate all ctypes code to use pywin32 instead, if possible #1808
Conversation
It's using the win32/user32 GetWindowTextLengthW() so name it appropriately.
After over an hour's struggle trying to get the win32/user32 `GetWindowTextW()` prototyped properly, and running into problem after problem: 1. Once you properly prototype this, including defining the 2nd `lpString` parameter as 'out', ctypes wants to just *return* that. 2. This leads to then implementing a `.errcheck` property so that you can *also* check the INT result for errors. 3. But then you just run into another error. I finally found <https://stackoverflow.com/a/10905155> which pointed out you can simply use `win32gui.GetWindowText()` from `pywin32`. Tested, it simply works, no errors/exceptions.
* All of the implementation of the EnumWindows callback. The documentation for pywin32 isn't great, in terms of finding the call convention and necessary form of parameters, but once you do this is *much* cleaner than using ctypes directly. I'm not 100% certain that `win32api.OpenProcess()` has the same semantic as the previous `GetProcessHandleFromHwnd()`, i.e. only getting the `handle` back if the process is owned by the current user. If the user has Windows Administrator rights they can probably kill other users' processes ?
NB: I'm not 100% sure that: handle = win32api.OpenProcess(win32con.PROCESS_TERMINATE, False, process_id) will definitely only return a handle for the current user's processes. It might do so for other users' processes if the current one is a Windows Administrator ? There's an alternative method of getting process info using the |
Not that it matters, because it turns out the old code comment is wrong. Running So, although we'd actually like to have the check be 100% correct, only being positive in the "game is running as this user" case... not doing so won't actually be a regression. |
I also note that the whole of the old code only finds the The same as also true of the new code in this PR. |
Both the current `develop` code and this PR's new code share this behaviour. What the code does is look for windows/processes *in the current Windows session*, no matter if they're owned by the session user or not. Neither version will find a process, that would match, if it's running in another session, i.e. you'd need to switch to it via the windows Login screen.
I double checked both old and new code through |
This is something the old code *didn't actually do*. Tested with a 'pseudo ed' process (same MainWindowTitle) running as myself (test matches), and via `runas` as another user (doesn't match).
At this time this errors, but that might be due to mixing ctypes and pywin32 code.
Somehow I didn't run into this with testing *this* invocation of `win32gui.EnumWindows()`, but then I did over in EDMarketConnector.py. So, as I had to handle the exception there, let's also do it here.
I currently can't find where pywin32 defines COINIT_APARTMENTTHREADED and COINIT_DISABLE_OLE1DDE so am still having to hard-define those.
This was to be sure *that* bit of code was being hit and working without issues. It is.
* Also actually define the 'mapping' constants, with URL reference.
This started out as properly prototyping the ctypes code in
monitor.py
, so that nothing gets confused by return types on 64-bit python/windows...... but after struggling for far too long with
GetWindowTextW()
, due to its second (out of three) parameter being 'out'... and realising that ctypes 'helpfully' returns all 'out' as the function return, and that means you need to define a function to assign to the.errcheck
property in order to actually check result (return) ... I came across how to do the same thing with pywin32 modules instead.And now I've managed to eliminate all the ctypes code from
monitor.py
. I'll look into all the other ctypes code as well.This does mean we'll start having
pywin32
as a PIP-level dependency ... and I am wondering if py2exe will do the right thing with all the separate little modules it provides. They are explicit imports in our code, so here's hoping.Close #1805