Improved Linux detection fixes #143
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
I've been using this for a few days, seems to be working as intended. |
XenHat
force-pushed
the
improved-detection-fixes
branch
2 times, most recently
from
4f79e57 to
4de0038
Compare
|
|
XenHat
force-pushed
the
improved-detection-fixes
branch
2 times, most recently
from
dfea808 to
10d9750
Compare
|
fixes my issue with running thcrap patched touhou 7, cool pr |
|
|
There was a problem hiding this comment.
Can we refactor this file to something like this?
https://jsfiddle.net/jsq76fyc/ <-- much easier to reason about than quadruple nested if blocks 🙂
Then you can also keep the file minimal and export the methods outside of index.js to keep it sane for the mind.
There was a problem hiding this comment.
This turned out to break detection again, so I'll need more time to fiddle with it.
There was a problem hiding this comment.
@XenHat the approach I'd use for major refactors like this is:
- Cover existing code with tests
- Make sure when you change logic in the code it makes tests red (some tests can be green all the time, that's a useless test 🙂)
- Now, refactor and keep tests green
Much easier in the long run to maintain it this way. Plus you can abstract the tests from actual OS/processes via fakes or mocks.
If I'd have some time I'll give you an example how to write tests for these. But it seems the maintainer didn't add any testing framework for this repo 🙂
So I guess we'd have to ask whether or not we should. E.g. jest/vitest @CanadaHonk
XenHat
force-pushed
the
improved-detection-fixes
branch
4 times, most recently
from
57721dc to
2aba85d
Compare
src/process/index.js
Outdated
| const path = _path.toLowerCase().replaceAll('\\', '/'); | ||
| if (path.startsWith('c:/windows')) continue // system processes (wine) | ||
| if (_path.includes('webhelper')) continue; // CEF Processes | ||
| // TODO: add 'dolphin-emu' to database for linux executable |
There was a problem hiding this comment.
for(var entry of DetectableDB) {
if(entry.id == "356943187589201930") {
entry.executables.push({ is_launcher: false, name: "dolphin-emu", os: "linux" })
break;
}
}There was a problem hiding this comment.
Probably should have a less hacky mechanism to patch existing entries or add/remove them. Vesktop adds one for OBS to enable streamer mode by patching the source as well: https://github.com/Vencord/Vesktop/blob/7b5e1ed4da50c02c9ed3dd976066364d1f19c106/patches/arrpc%403.5.0.patch
XenHat
force-pushed
the
improved-detection-fixes
branch
from
42b80ec to
ddd283e
Compare
Some games are listed in the discord game database with their parent directories but their launched processes on linux don't always have this included in cmdline. This fixes that by checking processes with their working directories as well. For example, this will now detect DOOM Eternal running through proton.
changes: - several performance improvements - match against more (malformed?) game database entries Tested against https://github.com/OpenAsar/arrpc/blob/2234e9c9111f4c42ebcc3aa6a2215bfd979eef77/src/process/detectable.json Signed-off-by: XenHat <me@xenh.at>
it's still slow
Co-authored-by: Marcel Witte <wittemar@gmail.com>
XenHat
force-pushed
the
improved-detection-fixes
branch
from
ddd283e to
88e1b34
Compare
This entry gets detected wrongly, resort to a local fixup
6 participants
This is a re-implementation of #92 with additional fixes for more games, and should be compatible with #130 as I have validated the code against the current upstream database.
The following games have been verified to be correctly detected on Linux:
KNOWN ISSUES:
Main changes:
Additional credit: