Skip to content
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

fix: Ignore NS dedicated server for game running check #562

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Ombrezz
Copy link
Contributor

@Ombrezz Ombrezz commented Sep 13, 2023

check_northstar_running() only returns true if Northstar wasn't launched with the -dedicated flag, untested on Windows.

Closes #284

Only count Northstar as running if it's not a dedicated server.
@GeckoEidechse GeckoEidechse changed the title Fix #284 fix: Ignore NS dedicated server for game running check Sep 13, 2023
Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for the PR <3

I enabled CI and it failed clippy check. The fix should be explained in CI logs.

If you have any questions, LMK ^^

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in quick testing on Windows, this cause non-dedicated-Northstar to be detected as running.

Both when starting first a dedi server and then client, and when only starting a client.

Not quite sure what's causing that cause looking at the code it should be fine?

@Ombrezz
Copy link
Contributor Author

Ombrezz commented Sep 14, 2023

The sysinfo crate suggests calling refresh_all() on a new System struct to get all the information up to date, maybe that will fix it?

@GeckoEidechse
Copy link
Member

Nope, looks like it still has the same issue... :/

image

Did it work for you locally?

@Ombrezz
Copy link
Contributor Author

Ombrezz commented Sep 18, 2023

Works fine for me on Linux, could you run a quick test to see what sysinfo returns for the name(), exe(), and cmd() functions on the Northstar and/or TF2 processes on Windows?

let mut s = sysinfo::System::new_all();
s.refresh_all();
for process in s.processes().values() {
if (process.name().ends_with("Titanfall2.exe") || process.name().ends_with("Northstar.exe"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to #562 (comment) I think, process name was changed in this PR while it shouldn't have been modified:

Suggested change
if (process.name().ends_with("Titanfall2.exe") || process.name().ends_with("Northstar.exe"))
if (process.name().ends_with("Titanfall2.exe") || process.name().ends_with("NorthstarLauncher.exe"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't say "Game is running" if it's Northstar dedicated server
3 participants