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

Avoid deleting eldenring.exe while it is running #78

Merged
merged 3 commits into from
Jul 18, 2024
Merged

Conversation

metinc
Copy link
Contributor

@metinc metinc commented Jul 7, 2024

If you pass an --executable arg like when playing the Seamless Co-op mod Elden Ring does crash. The reason is subprocess.run() waits for the passed executable like ersc_launcher.exe to finish but it does not wait for eldenring.exe to finish. The Python script is unaware of eldenring.exe since it was executed by ersc_launcher.exe. Then rmtree() is called and deletes the patched eldenring.exe while it is running.
This PR solves this by checking if eldenring.exe can be accessed. If it is locked it won't be deleted. The behavior remains unchanged if --executable is not passed.
I tested this on Windows 11. Would be great if someone can test it on Linux aswell.

Fixes #54
Fixes #60
Fixes #61

@metinc metinc changed the title Avoid deleting eldenring.exe while it is locked Avoid deleting eldenring.exe while it is running Jul 7, 2024
@gurrgur
Copy link
Owner

gurrgur commented Jul 8, 2024

Thanks for your work on this.

The solution looks good but I'm worried about potentially leaving the temp dir as it would lead to steam uninstallation not working properly. We could try an approach where cleanup polls the locked state of eldenring.exe (e.g. every 3 seconds) and proceeds when the lock has ceased.

In any case, I'd like to have this tested on Linux before merging.

@metinc
Copy link
Contributor Author

metinc commented Jul 14, 2024

@gurrgur I understand your concerns regarding the er-patcher-tmp folder not being deleted. I wasn't aware that this issue was causing the Steam uninstall to no longer work. I have now adjusted my pull request accordingly. If os.remove() fails because eldenring.exe has not been unlocked yet, a retry will be executed every 3 seconds. If a different error occurs, no retry will be performed. In any case, rmtree() will be executed afterward.

Additionally, I have now installed Arch Linux in a dual boot setup. I will test the changes under Linux and provide feedback.

@gurrgur
Copy link
Owner

gurrgur commented Jul 18, 2024

Thanks for the update! LGTM

@gurrgur
Copy link
Owner

gurrgur commented Jul 18, 2024

Oh and I just tested running seamless coop on linux and it starts up just fine!

env WINE_FULLSCREEN_FSR=1 python er-patcher -ucvs --rate 120 -x ersc_launcher.exe -- %command%

@anthonybaldwin anthonybaldwin mentioned this pull request Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants