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

Fix reflection-related iOS crashes #29992

Merged
merged 4 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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));
}
}
}
4 changes: 3 additions & 1 deletion osu.Game/Beatmaps/BeatmapImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,10 @@ protected override void PreImport(BeatmapSetInfo beatmapSet, Realm realm)

if (beatmapSet.OnlineID > 0)
{
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
4 changes: 3 additions & 1 deletion osu.Game/Online/BeatmapDownloadTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ 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, _) =>
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
10 changes: 7 additions & 3 deletions osu.Game/Online/ScoreDownloadTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,14 @@ protected override void LoadComplete()
Downloader.DownloadBegan += downloadBegan;
Downloader.DownloadFailed += downloadFailed;

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
4 changes: 3 additions & 1 deletion osu.Game/Skinning/RealmBackedResourceStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ 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);
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
4 changes: 3 additions & 1 deletion osu.Game/Skinning/SkinManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,11 @@ public void SelectRandomSkin()
{
Realm.Run(r =>
{
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
44 changes: 44 additions & 0 deletions osu.Game/Utils/BindableValueAccessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// 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(isBindableT);
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().FirstOrDefault(isBindableT);
if (bindableWithValueType == null)
return;

set_method.MakeGenericMethod(bindableWithValueType.GenericTypeArguments[0]).Invoke(null, [bindable, value]);
bdach marked this conversation as resolved.
Show resolved Hide resolved
}

private static bool isBindableT(Type type)
=> type.IsGenericType
&& (type.GetGenericTypeDefinition() == typeof(Bindable<>)
|| type.GetGenericTypeDefinition() == typeof(IBindable<>));
bdach marked this conversation as resolved.
Show resolved Hide resolved

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;
}
}
Loading