From 81033e1fdf4fccf8e5f5947d79c4660477ed3822 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2019 18:52:07 +0900 Subject: [PATCH 1/4] Add extra logging --- osu.Game/Beatmaps/BeatmapManager.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 6e485f642a..7a616ed8f2 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -129,9 +129,12 @@ namespace osu.Game.Beatmaps { var beatmapIds = beatmapSet.Beatmaps.Where(b => b.OnlineBeatmapID.HasValue).Select(b => b.OnlineBeatmapID).ToList(); + LogForModel(beatmapSet, "Validating online IDs..."); + // ensure all IDs are unique if (beatmapIds.GroupBy(b => b).Any(g => g.Count() > 1)) { + LogForModel(beatmapSet, "Found non-unique IDs, resetting..."); resetIds(); return; } @@ -144,8 +147,12 @@ namespace osu.Game.Beatmaps // reset the import ids (to force a re-fetch) *unless* they match the candidate CheckForExisting set. // we can ignore the case where the new ids are contained by the CheckForExisting set as it will either be used (import skipped) or deleted. var existing = CheckForExisting(beatmapSet); + if (existing == null || existingBeatmaps.Any(b => !existing.Beatmaps.Contains(b))) + { + LogForModel(beatmapSet, "Found existing import with IDs already, resetting..."); resetIds(); + } } void resetIds() => beatmapSet.Beatmaps.ForEach(b => b.OnlineBeatmapID = null); From 1bc0eae2a6fa08edf390f47a4f40cc80f71af3f2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2019 18:53:22 +0900 Subject: [PATCH 2/4] Fix beatmap online retrieval response running incorrectly scheduled --- osu.Game/Beatmaps/BeatmapManager.cs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 7a616ed8f2..80d41c5267 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -387,22 +387,21 @@ namespace osu.Game.Beatmaps var req = new GetBeatmapRequest(beatmap); - req.Success += res => - { - LogForModel(set, $"Online retrieval mapped {beatmap} to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}."); - - beatmap.Status = res.Status; - beatmap.BeatmapSet.Status = res.BeatmapSet.Status; - beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; - beatmap.OnlineBeatmapID = res.OnlineBeatmapID; - }; - req.Failure += e => { LogForModel(set, $"Online retrieval failed for {beatmap} ({e.Message})"); }; try { // intentionally blocking to limit web request concurrency req.Perform(api); + + var res = req.Result; + + beatmap.Status = res.Status; + beatmap.BeatmapSet.Status = res.BeatmapSet.Status; + beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; + beatmap.OnlineBeatmapID = res.OnlineBeatmapID; + + LogForModel(set, $"Online retrieval mapped {beatmap} to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}."); } catch (Exception e) { From fbf81207d4a634bb4213d9794cdd72ed491c1ab5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2019 18:54:03 +0900 Subject: [PATCH 3/4] Don't assign server-fetched online id if it was assigned elsewhere --- osu.Game/Beatmaps/BeatmapManager.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 80d41c5267..7057bd46bb 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -399,7 +399,11 @@ namespace osu.Game.Beatmaps beatmap.Status = res.Status; beatmap.BeatmapSet.Status = res.BeatmapSet.Status; beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; - beatmap.OnlineBeatmapID = res.OnlineBeatmapID; + + // note that this check only needs to be here if two identical hashed beatmaps exist int he same import. + // probably fine to leave it for safety. + if (set.Beatmaps.All(b => b.OnlineBeatmapID != res.OnlineBeatmapID)) + beatmap.OnlineBeatmapID = res.OnlineBeatmapID; LogForModel(set, $"Online retrieval mapped {beatmap} to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}."); } From 6d5484646297c146f5202cae667552416d5edac1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Nov 2019 10:18:47 +0900 Subject: [PATCH 4/4] Null online id on lookup failure --- osu.Game/Beatmaps/BeatmapManager.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 7057bd46bb..77fcf44a9b 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -387,7 +387,7 @@ namespace osu.Game.Beatmaps var req = new GetBeatmapRequest(beatmap); - req.Failure += e => { LogForModel(set, $"Online retrieval failed for {beatmap} ({e.Message})"); }; + req.Failure += fail; try { @@ -399,16 +399,18 @@ namespace osu.Game.Beatmaps beatmap.Status = res.Status; beatmap.BeatmapSet.Status = res.BeatmapSet.Status; beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; - - // note that this check only needs to be here if two identical hashed beatmaps exist int he same import. - // probably fine to leave it for safety. - if (set.Beatmaps.All(b => b.OnlineBeatmapID != res.OnlineBeatmapID)) - beatmap.OnlineBeatmapID = res.OnlineBeatmapID; + beatmap.OnlineBeatmapID = res.OnlineBeatmapID; LogForModel(set, $"Online retrieval mapped {beatmap} to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}."); } catch (Exception e) { + fail(e); + } + + void fail(Exception e) + { + beatmap.OnlineBeatmapID = null; LogForModel(set, $"Online retrieval failed for {beatmap} ({e.Message})"); } }