Skip to content

Commit

Permalink
Merge pull request #29992 from smoogipoo/fix-ios-realm-crashes
Browse files Browse the repository at this point in the history
Fix reflection-related iOS crashes
  • Loading branch information
peppy authored Sep 26, 2024
2 parents 3111d6a + 89e8baf commit 78c1426
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 14 deletions.
52 changes: 52 additions & 0 deletions osu.Game.Tests/Utils/BindableValueAccessorTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using NUnit.Framework;
using osu.Framework.Bindables;
using osu.Game.Utils;

namespace osu.Game.Tests.Utils
{
[TestFixture]
public class BindableValueAccessorTest
{
[Test]
public void GetValue()
{
const int value = 1337;

BindableInt bindable = new BindableInt(value);
Assert.That(BindableValueAccessor.GetValue(bindable), Is.EqualTo(value));
}

[Test]
public void SetValue()
{
const int value = 1337;

BindableInt bindable = new BindableInt();
BindableValueAccessor.SetValue(bindable, value);

Assert.That(bindable.Value, Is.EqualTo(value));
}

[Test]
public void GetInvalidBindable()
{
BindableList<object> list = new BindableList<object>();
Assert.That(BindableValueAccessor.GetValue(list), Is.EqualTo(list));
}

[Test]
public void SetInvalidBindable()
{
const int value = 1337;

BindableList<int> list = new BindableList<int> { value };
BindableValueAccessor.SetValue(list, 2);

Assert.That(list, Has.Exactly(1).Items);
Assert.That(list[0], Is.EqualTo(value));
}
}
}
5 changes: 4 additions & 1 deletion osu.Game/Beatmaps/BeatmapImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,11 @@ protected override void PreImport(BeatmapSetInfo beatmapSet, Realm realm)

if (beatmapSet.OnlineID > 0)
{
// Required local for iOS. Will cause runtime crash if inlined.
int onlineId = beatmapSet.OnlineID;

// OnlineID should really be unique, but to avoid catastrophic failure let's iterate just to be sure.
foreach (var existingSetWithSameOnlineID in realm.All<BeatmapSetInfo>().Where(b => b.OnlineID == beatmapSet.OnlineID))
foreach (var existingSetWithSameOnlineID in realm.All<BeatmapSetInfo>().Where(b => b.OnlineID == onlineId))
{
existingSetWithSameOnlineID.DeletePending = true;
existingSetWithSameOnlineID.OnlineID = -1;
Expand Down
7 changes: 2 additions & 5 deletions osu.Game/Configuration/SettingSourceAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
Expand All @@ -15,6 +14,7 @@
using osu.Framework.Localisation;
using osu.Game.Graphics.UserInterface;
using osu.Game.Overlays.Settings;
using osu.Game.Utils;

namespace osu.Game.Configuration
{
Expand Down Expand Up @@ -228,10 +228,7 @@ public static object GetUnderlyingSettingValue(this object setting)
return b.Value;

case IBindable u:
// An unknown (e.g. enum) generic type.
var valueMethod = u.GetType().GetProperty(nameof(IBindable<int>.Value));
Debug.Assert(valueMethod != null);
return valueMethod.GetValue(u)!;
return BindableValueAccessor.GetValue(u);

default:
// fall back for non-bindable cases.
Expand Down
5 changes: 4 additions & 1 deletion osu.Game/Online/BeatmapDownloadTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ protected override void LoadComplete()
// Used to interact with manager classes that don't support interface types. Will eventually be replaced.
var beatmapSetInfo = new BeatmapSetInfo { OnlineID = TrackedItem.OnlineID };

realmSubscription = realm.RegisterForNotifications(r => r.All<BeatmapSetInfo>().Where(s => s.OnlineID == TrackedItem.OnlineID && !s.DeletePending), (items, _) =>
// Required local for iOS. Will cause runtime crash if inlined.
int onlineId = TrackedItem.OnlineID;

realmSubscription = realm.RegisterForNotifications(r => r.All<BeatmapSetInfo>().Where(s => s.OnlineID == onlineId && !s.DeletePending), (items, _) =>
{
if (items.Any())
Schedule(() => UpdateState(DownloadState.LocallyAvailable));
Expand Down
11 changes: 8 additions & 3 deletions osu.Game/Online/ScoreDownloadTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,15 @@ protected override void LoadComplete()
Downloader.DownloadBegan += downloadBegan;
Downloader.DownloadFailed += downloadFailed;

// Required local for iOS. Will cause runtime crash if inlined.
long onlineId = TrackedItem.OnlineID;
long legacyOnlineId = TrackedItem.LegacyOnlineID;
string hash = TrackedItem.Hash;

realmSubscription = realm.RegisterForNotifications(r => r.All<ScoreInfo>().Where(s =>
((s.OnlineID > 0 && s.OnlineID == TrackedItem.OnlineID)
|| (s.LegacyOnlineID > 0 && s.LegacyOnlineID == TrackedItem.LegacyOnlineID)
|| (!string.IsNullOrEmpty(s.Hash) && s.Hash == TrackedItem.Hash))
((s.OnlineID > 0 && s.OnlineID == onlineId)
|| (s.LegacyOnlineID > 0 && s.LegacyOnlineID == legacyOnlineId)
|| (!string.IsNullOrEmpty(s.Hash) && s.Hash == hash))
&& !s.DeletePending), (items, _) =>
{
if (items.Any())
Expand Down
3 changes: 1 addition & 2 deletions osu.Game/Rulesets/Mods/Mod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,7 @@ public void CopyCommonSettingsFrom(Mod source)

// TODO: special case for handling number types

PropertyInfo property = targetSetting.GetType().GetProperty(nameof(Bindable<bool>.Value))!;
property.SetValue(targetSetting, property.GetValue(sourceSetting));
BindableValueAccessor.SetValue(targetSetting, BindableValueAccessor.GetValue(sourceSetting));
}
}

Expand Down
5 changes: 4 additions & 1 deletion osu.Game/Skinning/RealmBackedResourceStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ public RealmBackedResourceStore(Live<T> source, IResourceStore<byte[]> underlyin
invalidateCache();
Debug.Assert(fileToStoragePathMapping != null);

realmSubscription = realm?.RegisterForNotifications(r => r.All<T>().Where(s => s.ID == source.ID), skinChanged);
// Required local for iOS. Will cause runtime crash if inlined.
Guid id = source.ID;

realmSubscription = realm?.RegisterForNotifications(r => r.All<T>().Where(s => s.ID == id), skinChanged);
}

protected override void Dispose(bool disposing)
Expand Down
5 changes: 4 additions & 1 deletion osu.Game/Skinning/SkinManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,12 @@ public void SelectRandomSkin()
{
Realm.Run(r =>
{
// Required local for iOS. Will cause runtime crash if inlined.
Guid currentSkinId = CurrentSkinInfo.Value.ID;

// choose from only user skins, removing the current selection to ensure a new one is chosen.
var randomChoices = r.All<SkinInfo>()
.Where(s => !s.DeletePending && s.ID != CurrentSkinInfo.Value.ID)
.Where(s => !s.DeletePending && s.ID != currentSkinId)
.ToArray();

if (randomChoices.Length == 0)
Expand Down
39 changes: 39 additions & 0 deletions osu.Game/Utils/BindableValueAccessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Linq;
using System.Reflection;
using osu.Framework.Bindables;
using osu.Framework.Extensions.TypeExtensions;

namespace osu.Game.Utils
{
internal static class BindableValueAccessor
{
private static readonly MethodInfo get_method = typeof(BindableValueAccessor).GetMethod(nameof(getValue), BindingFlags.Static | BindingFlags.NonPublic)!;
private static readonly MethodInfo set_method = typeof(BindableValueAccessor).GetMethod(nameof(setValue), BindingFlags.Static | BindingFlags.NonPublic)!;

public static object GetValue(IBindable bindable)
{
Type? bindableWithValueType = bindable.GetType().GetInterfaces().FirstOrDefault(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(IBindable<>));
if (bindableWithValueType == null)
return bindable;

return get_method.MakeGenericMethod(bindableWithValueType.GenericTypeArguments[0]).Invoke(null, [bindable])!;
}

public static void SetValue(IBindable bindable, object value)
{
Type? bindableWithValueType = bindable.GetType().EnumerateBaseTypes().SingleOrDefault(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(Bindable<>));
if (bindableWithValueType == null)
return;

set_method.MakeGenericMethod(bindableWithValueType.GenericTypeArguments[0]).Invoke(null, [bindable, value]);
}

private static object getValue<T>(object bindable) => ((IBindable<T>)bindable).Value!;

private static object setValue<T>(object bindable, object value) => ((Bindable<T>)bindable).Value = (T)value;
}
}

0 comments on commit 78c1426

Please sign in to comment.