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

Added option for custom Log Directory #240

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mrschick
Copy link

@mrschick mrschick commented Mar 16, 2023

Hi, our community noticed the lack of a custom logs directory option, so we added it. First with a dirty override of Logs.prototype.logsPath() and now a slightly cleaner implementation 😄

Copy link
Owner

@Dahlgren Dahlgren left a comment

Choose a reason for hiding this comment

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

I think this breaks reading logs on Windows server. Linux uses this application's log output handler but Windows outputs to the default game logs folder.

if (this.config.type === 'linux') {
return path.join(this.config.path, 'logs')
}
var gameLogFolder = gamesLogFolder[this.config.game]
if (!gameLogFolder) {
return null
}
if (this.config.type === 'windows') {
return userhome('AppData', 'Local', gameLogFolder)
}
if (this.config.type === 'wine') {
var username = process.env.USER
return userhome('.wine', 'drive_c', 'users', username, 'Local Settings', 'Application Data', gameLogFolder)
}
return null

Can you double check that logs works on Windows?

lib/settings.js Outdated Show resolved Hide resolved
config.js.example Outdated Show resolved Hide resolved
@mrschick
Copy link
Author

I think this breaks reading logs on Windows server. Linux uses this application's log output handler but Windows outputs to the default game logs folder.

Can you double check that logs works on Windows?

Huh, odd. I admit that we didn't properly test the actual writing of logs, mainly because our first rough fix (see below) worked just fine on our windows server and the latest also properly returned the custom logs path to the frontend's Settings panel.

Logs.prototype.logsPath = function () {
  return "custom log path";

Thanks for the feedback, we'll see towards making it work.

@mrschick
Copy link
Author

Turns out I misunderstood and miscommunicated what exactly the implementation was intended to do.

On Windows, Logs.prototype.logsPath() only determines the path where logs are read by the WebAdmin's Logs tab, not where Arma actually writes them, which is defined by the -profiles startup parameter (set in the respective server preset).

As such, the current implementation already works for the intended purpose on windows. So if the user uses a custom -profiles path for profiles and logs, they can set the logsPath accordingly to view the correct logs output directory via the Logs tab.
I rewrote the descriptions accordingly.

@mrschick mrschick requested a review from Dahlgren April 16, 2023 20:50
Copy link
Owner

@Dahlgren Dahlgren left a comment

Choose a reason for hiding this comment

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

Sorry for late reply, looks mostly good now!

lib/server.js Outdated Show resolved Hide resolved
lib/server.js Outdated Show resolved Hide resolved
lib/settings.js Outdated Show resolved Hide resolved
lib/settings.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mrschick
Copy link
Author

No problem, thanks for the review!

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.

2 participants