qb-0 / pyMeow

Python Game Hacking Library
MIT License
355 stars 39 forks source link

Add process path to Windows process structure #49

Closed Hypnootika closed 6 months ago

Hypnootika commented 7 months ago

The code has been updated to include the process path in the Windows process structure. This was achieved by modifying the process initialization section of the Windows code block. Since im unsure if the Linux part of it works, I just added it to Windows.

qb-0 commented 7 months ago

Thank you for the pr.

Why is the process path relevant in the process structure? Why did get_process_path need to be changed? And yes, the Linux version works. We should produce as little 'Windows only' stuff as possible. pyMeow should still be cross-platform. I'd rather have less functionality in pyMeow than only partly cross-platform stuff.

Hypnootika commented 7 months ago

It needed to get changed because - atleast in my tests - it didn't work (returned an empty String).

Its rarely needed, thats true. But since the information is avaliable on Linux and Windows, i thought it might be a good addition. On Windows you sometimes need the full path for DLL related functions.

No, relatable, thats why its not a Windows only PR. But since its just a matter of moving/adding the path, its not really a Windows only PR.

If you think, that pyMeow doesn't need it, thats no problem. I mainly did it because i integrated it in my Framework and didn't see a reason why pyMeow shouldn't have a working solution aswell

qb-0 commented 7 months ago

I see. Well, get_process_path needs to function properly, so the pull request is completely acceptable. And yes, it's a valuable addition if there's a need for it. However, if we're going to include it, I want it to be functional on both platforms. The only reason I see for not adding it is if get_process_path might fail, which would cause open_process to fail as well.

I'm not sure what happens if QueryFullProcessImageNameW or readLink fails since both return values are getting discarded. Just returning an empty string?

So, if there's a need for including it in the structure, we must ensure it works on both platforms and is failsafe.

Hypnootika commented 7 months ago

If it fails, atleast on Windows, it returns an empty String. Not sure what it does on Linux.

We could either go for the Options module or the path remains split from the Process object. The use case is rare enough to have it seperate from the object. I guess keeping it as it is and only fix the Win function would be a good point to agree on?

qb-0 commented 6 months ago

I guess keeping it as it is and only fix the Win function would be a good point to agree on?

:+1:

Hypnootika commented 6 months ago

All right, ill change it. Won't make it till tuesday though.

Hypnootika commented 6 months ago

I removed the automatic path insertion and the path from the Process-object. It is now a plain fix for the getProcessPath proc for Windows. (the issue the fix is intended to solve was an empty string returned by the previous proc for - for me - unknown reasons (it looked correct but apparently wasn't).)

qb-0 commented 6 months ago

As usual: Thanks for your support frog ;)

Hypnootika commented 6 months ago

Welcome