feat(skymp5-client): make server settings more flexible

[nic11]

Important

Introduces SettingsService to centralize server settings management, refactoring related logic from AuthService and LoadOrderVerificationService.

• New Service:
  • Introduces SettingsService in settingsService.ts to manage server settings.
  • Provides methods getServerMasterKey(), getMasterUrl(), getTargetPeer(), and getServerMods().
• Refactoring:
  • Moves getServerMasterKey() and getMasterUrl() from AuthService to SettingsService.
  • Replaces direct calls to these methods in AuthService and LoadOrderVerificationService with calls to SettingsService.
  • Updates SkympClient to use SettingsService for server connection details.
• Behavior:
  • SettingsService handles server mod retrieval and peer targeting, improving flexibility and reducing redundancy.

^{This description was created by for 4d64152. It will automatically update as commits are pushed.}

[Pospelove]

Promises are not yet supported in main menu

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 4d64152 in 2 minutes and 56 seconds

More details

• Looked at 415 lines of code in 5 files
• Skipped 0 files when reviewing.
• Skipped posting 13 drafted comments based on config settings.

1. skymp5-client/src/index.ts:54

• Draft comment:
  New import for SettingsService added. Ensure it’s grouped with other service imports for consistency.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

2. skymp5-client/src/index.ts:66

• Draft comment:
  Instantiating SettingsService as first listener. Confirm dependency order as other services may rely on settings.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

3. skymp5-client/src/services/services/authService.ts:53

• Draft comment:
  Duplicate server settings functions have been removed in favor of SettingsService. Ensure all consumers now use SettingsService.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. skymp5-client/src/services/services/authService.ts:196

• Draft comment:
  Using SettingsService for constructing URLs improves maintainability. Verify the new service reliably returns expected master URL.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

5. skymp5-client/src/services/services/authService.ts:334

• Draft comment:
  readAuthDataFromDisk is now public. Confirm this change is safe and does not expose sensitive data unexpectedly.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

6. skymp5-client/src/services/services/loadOrderVerificationService.ts:25

• Draft comment:
  Mod retrieval now delegates to SettingsService. Ensure that error handling/logging in SettingsService.getServerMods covers all edge cases.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

7. skymp5-client/src/services/services/settingsService.ts:118

• Draft comment:
  Retry loop for getServerMods is clear. Consider adding more detailed logging or error handling if manifest parsing fails.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

8. skymp5-client/src/services/services/skympClient.ts:94

• Draft comment:
  Connection establishment now uses SettingsService.getTargetPeer callback. Verify fallback mechanism (defaultPeer) behaves as expected when the request fails.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

9. skymp5-client/src/index.ts:66

• Draft comment:
  Ensure SettingsService is initialized before dependent services are set up. Its position in the listeners array may affect services (e.g. AuthService) using it.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

10. skymp5-client/src/services/services/authService.ts:196

• Draft comment:
  Multiple lookups for SettingsService are performed. Consider caching the reference in a local variable to improve readability and avoid redundant lookups.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. skymp5-client/src/services/services/authService.ts:334

• Draft comment:
  The method 'readAuthDataFromDisk' was changed from private to public. Confirm if this increased visibility is intended considering potential security concerns.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. skymp5-client/src/services/services/loadOrderVerificationService.ts:30

• Draft comment:
  Using SettingsService.getServerMods() for server mod retrieval is appropriate. Ensure that the server mod list order is consistent with the client mod order for accurate comparisons.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

13. skymp5-client/src/services/services/skympClient.ts:94

• Draft comment:
  In establishConnectionConditional, while using SettingsService.getTargetPeer looks good, consider adding error handling in the callback to handle cases where connection parameters fall back or connection fails.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

Workflow ID: wflow_Ue0bbzzFaxpwaV10

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

The timeout for getTargetPeer is hardcoded to 5000ms. Consider parameterizing this value or documenting its rationale for future configurability.