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 host property to ftp config to limit confusion as this is a common configurable property. #346

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

mbartisan
Copy link
Contributor

No description provided.

Added host property to ftp config to limit confusion as this is a common configurable property.
@CodeRedDev
Copy link
Contributor

This might require a ftp:host description in the README.md to be complete.

Even though I'm questioning the sense in this parameter overall. If I'm not mistaken then the log should always be on the server that hosts the game server. So there should not be a parameter to set another ftp:host and this line

host: this.options.ftp.host || this.options.host

should always take this.options.host.

@mbartisan
Copy link
Contributor Author

Specifying the ftp host is required to work with some server providers. See Issue #299 where this feature was implemented. This PR is adding documentation to the ability.

@CodeRedDev
Copy link
Contributor

Ah alright sorry.

My first point should be valid though that the ftp:host should be descibed here

* `ftp:port` - The FTP port of the server. Only required for `ftp` `logReaderMode`.

Maybe also by mentioning #299 👍

@werewolfboy13 werewolfboy13 added plugin bug Bug related to the SquadJS plugins patch Patch Change core bug Bug related to the core SquadJS API and removed plugin bug Bug related to the SquadJS plugins labels Mar 11, 2024
@werewolfboy13 werewolfboy13 merged commit de8219d into Team-Silver-Sphere:master Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core bug Bug related to the core SquadJS API patch Patch Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants