Fix preference loading bugs (#27742)

First bug: if an error occured during pref loading code, it would fail. If the person then readied up, it would likely cause the round to fail to start.

Why could they ready up? The code only checks that the prefs finished loading, not that they finished loading *successfully*. Whoops.

Anyways, now people get kicked if their prefs fail to load. And I improved the error handling.

Second bug: if a user disconnected while their prefs were loading, it would cause an exception. This exception would go unobserved on lobby servers or raise through gameticker on non-lobby servers.

This happened even on a live server once and then triggered the first bug, but idk how.

Fixed this by properly plumbing through cancellation into the preferences loading code. The stuff is now cancelled properly.

Third bug: if somebody has a loadout item with a playtime requirement active, load-time sanitization of player prefs could run into a race condition because the sanitization can happen *before* play time was loaded.

Fixed by moving pref sanitizations to a later stage in the load process.
This commit is contained in:
Pieter-Jan Briers
2024-05-07 06:21:03 +02:00
committed by GitHub
parent 61c1aeddf3
commit 7a38b22ddb
9 changed files with 154 additions and 54 deletions

View File

@@ -13,6 +13,7 @@ using Robust.Shared.Configuration;
using Robust.Shared.Network;
using Robust.Shared.Player;
using Robust.Shared.Prototypes;
using Robust.Shared.Utility;
namespace Content.Server.Preferences.Managers
@@ -27,6 +28,7 @@ namespace Content.Server.Preferences.Managers
[Dependency] private readonly IConfigurationManager _cfg = default!;
[Dependency] private readonly IServerDbManager _db = default!;
[Dependency] private readonly IPlayerManager _playerManager = default!;
[Dependency] private readonly IDependencyCollection _dependencies = default!;
[Dependency] private readonly IPrototypeManager _protos = default!;
// Cache player prefs on the server so we don't need as much async hell related to them.
@@ -101,9 +103,8 @@ namespace Content.Server.Preferences.Managers
var curPrefs = prefsData.Prefs!;
var session = _playerManager.GetSessionById(userId);
var collection = IoCManager.Instance!;
profile.EnsureValid(session, collection);
profile.EnsureValid(session, _dependencies);
var profiles = new Dictionary<int, ICharacterProfile>(curPrefs.Characters)
{
@@ -196,7 +197,7 @@ namespace Content.Server.Preferences.Managers
async Task LoadPrefs()
{
var prefs = await GetOrCreatePreferencesAsync(session.UserId);
var prefs = await GetOrCreatePreferencesAsync(session.UserId, cancel);
prefsData.Prefs = prefs;
prefsData.PrefsLoaded = true;
@@ -211,6 +212,16 @@ namespace Content.Server.Preferences.Managers
}
}
public void SanitizeData(ICommonSession session)
{
// This is a separate step from the actual database load.
// Sanitizing preferences requires play time info due to loadouts.
// And play time info is loaded concurrently from the DB with preferences.
var data = _cachedPlayerPrefs[session.UserId];
DebugTools.Assert(data.Prefs != null);
data.Prefs = SanitizePreferences(session, data.Prefs, _dependencies);
}
public void OnClientDisconnected(ICommonSession session)
{
_cachedPlayerPrefs.Remove(session.UserId);
@@ -270,18 +281,15 @@ namespace Content.Server.Preferences.Managers
return null;
}
private async Task<PlayerPreferences> GetOrCreatePreferencesAsync(NetUserId userId)
private async Task<PlayerPreferences> GetOrCreatePreferencesAsync(NetUserId userId, CancellationToken cancel)
{
var prefs = await _db.GetPlayerPreferencesAsync(userId);
var prefs = await _db.GetPlayerPreferencesAsync(userId, cancel);
if (prefs is null)
{
return await _db.InitPrefsAsync(userId, HumanoidCharacterProfile.Random());
return await _db.InitPrefsAsync(userId, HumanoidCharacterProfile.Random(), cancel);
}
var session = _playerManager.GetSessionById(userId);
var collection = IoCManager.Instance!;
return SanitizePreferences(session, prefs, collection);
return prefs;
}
private PlayerPreferences SanitizePreferences(ICommonSession session, PlayerPreferences prefs, IDependencyCollection collection)