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.

Sign up for GitHub

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

Deprecate {IsValid*, Try*} APIs. #510

Merged
merged 2 commits into from
Dec 28, 2024
Merged

Deprecate {IsValid*, Try*} APIs. #510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5198aeb
Deprecated `Promise(<T>).Deferred.{IsValid*, Try*}` APIs.
timcassell Dec 28, 2024
5930503
Fixed tests.
timcassell Dec 28, 2024
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
10 changes: 10 additions & 0 deletions Package/Core/InternalShared/DebugInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ internal static void ValidateArgument<TArg>(TArg arg, string argName, int skipFr
}
internal static void ValidateArgument(Promise arg, string argName, int skipFrames)
{
#pragma warning disable CS0618 // Type or member is obsolete
if (!arg.IsValid)
#pragma warning restore CS0618 // Type or member is obsolete
{
throw new InvalidArgumentException(argName,
"Promise is invalid. Call `GetRetainer()` if you intend to await multiple times.",
Expand Down Expand Up @@ -252,7 +254,9 @@ private IEnumerable<StackTrace> GetStackTraces()

internal static void ValidateOperation(Promise promise, int skipFrames)
{
#pragma warning disable CS0618 // Type or member is obsolete
if (!promise.IsValid)
#pragma warning restore CS0618 // Type or member is obsolete
{
throw new InvalidOperationException("Promise is invalid." +
" Call `GetRetainer()` if you intend to await multiple times.",
Expand All @@ -270,7 +274,9 @@ partial void ValidateAwait(PromiseRefBase other, short promiseId)

private void ValidateAwait(PromiseRefBase other, short promiseId, bool awaited)
{
#pragma warning disable CS0618 // Type or member is obsolete
if (new Promise(other, promiseId).IsValid == false)
#pragma warning restore CS0618 // Type or member is obsolete
{
// Awaiting or returning an invalid from the callback is not allowed.
if (awaited)
Expand Down Expand Up @@ -492,7 +498,9 @@ static partial void ValidateArgument(Promise arg, string argName, int skipFrames

static partial void ValidateElement(Promise promise, string argName, int skipFrames)
{
#pragma warning disable CS0618 // Type or member is obsolete
if (!promise.IsValid)
#pragma warning restore CS0618 // Type or member is obsolete
{
throw new InvalidElementException(argName,
$"A promise is invalid in {argName}." +
Expand Down Expand Up @@ -522,7 +530,9 @@ static partial void ValidateArgument(Promise<T> arg, string argName, int skipFra

static partial void ValidateElement(Promise<T> promise, string argName, int skipFrames)
{
#pragma warning disable CS0618 // Type or member is obsolete
if (!promise.IsValid)
#pragma warning restore CS0618 // Type or member is obsolete
{
throw new InvalidElementException(argName,
$"A promise is invalid in {argName}." +
Expand Down
104 changes: 72 additions & 32 deletions Package/Core/Promises/Deferred.cs
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 0 additions & 22 deletions Package/Core/Promises/Internal/DeferredInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,28 +100,6 @@ internal static DeferredPromise<TResult> GetOrCreate()
return promise;
}

[MethodImpl(InlineOption)]
internal static bool TryResolve(DeferredPromise<TResult> _this, int deferredId, in TResult value)
{
if (_this?.TryIncrementDeferredId(deferredId) == true)
{
_this.ResolveDirect(value);
return true;
}
return false;
}

[MethodImpl(InlineOption)]
internal static bool TryResolveVoid(DeferredPromise<TResult> _this, int deferredId)
{
if (_this?.TryIncrementDeferredId(deferredId) == true)
{
_this.ResolveDirectVoid();
return true;
}
return false;
}

[MethodImpl(InlineOption)]
internal void ResolveDirect(in TResult value)
{
Expand Down
7 changes: 0 additions & 7 deletions Package/Core/Promises/Promise.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,6 @@ public static implicit operator System.Threading.Tasks.ValueTask(in Promise rhs)
=> rhs.AsValueTask();
#endif // UNITY_2021_2_OR_NEWER || !UNITY_2018_3_OR_NEWER

/// <summary>
/// Gets whether this instance is valid to be awaited.
/// </summary>
public bool IsValid
// I would prefer to have a null ref only valid if the promise was created from Promise.Resolved, but it's more efficient to allow default values to be valid.
=> _ref?.GetIsValid(_id) != false;

/// <summary>
/// Mark this as awaited and prevent any further awaits or callbacks on this.
/// <para/>NOTE: It is imperative to terminate your promise chains with Forget so that any uncaught rejections will be reported and objects repooled (if pooling is enabled).
Expand Down
7 changes: 0 additions & 7 deletions Package/Core/Promises/PromiseT.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,6 @@ public static implicit operator System.Threading.Tasks.ValueTask(in Promise<T> r
=> rhs.AsValueTaskVoid();
#endif // UNITY_2021_2_OR_NEWER || !UNITY_2018_3_OR_NEWER

/// <summary>
/// Gets whether this instance is valid to be awaited.
/// </summary>
public bool IsValid
// I would prefer to have a null ref only valid if the promise was created from Promise.Resolved, but it's more efficient to allow default values to be valid.
=> _ref?.GetIsValid(_id) != false;

/// <summary>
/// Cast to <see cref="Promise"/>.
/// </summary>
Expand Down
8 changes: 8 additions & 0 deletions Package/Core/Promises/PromiseT_Deprecated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ namespace Proto.Promises
/// </summary>
public readonly partial struct Promise<T> : IEquatable<Promise<T>>
{
/// <summary>
/// Gets whether this instance is valid to be awaited.
/// </summary>
[Obsolete("Due to object pooling, this property is inherently unsafe. This will be removed in a future version.", false), EditorBrowsable(EditorBrowsableState.Never)]
public bool IsValid
// I would prefer to have a null ref only valid if the promise was created from Promise.Resolved, but it's more efficient to allow default values to be valid.
=> _ref?.GetIsValid(_id) != false;

/// <summary>
/// Mark this as awaited and get a new <see cref="Promise{T}"/> of <typeparamref name="T"/> that inherits the state of this and can be awaited multiple times until <see cref="Forget"/> is called on it.
/// <para/><see cref="Forget"/> must be called when you are finished with it.
Expand Down
8 changes: 8 additions & 0 deletions Package/Core/Promises/Promise_Deprecated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ namespace Proto.Promises
{
partial struct Promise
{
/// <summary>
/// Gets whether this instance is valid to be awaited.
/// </summary>
[Obsolete("Due to object pooling, this property is inherently unsafe. This will be removed in a future version.", false), EditorBrowsable(EditorBrowsableState.Never)]
public bool IsValid
// I would prefer to have a null ref only valid if the promise was created from Promise.Resolved, but it's more efficient to allow default values to be valid.
=> _ref?.GetIsValid(_id) != false;

/// <summary>
/// Mark this as awaited and get a new <see cref="Promise"/> that inherits the state of this and can be awaited multiple times until <see cref="Forget"/> is called on it.
/// <para/><see cref="Forget"/> must be called when you are finished with it.
Expand Down
45 changes: 11 additions & 34 deletions Package/Tests/CoreTests/APIs/APlus_2_1_PromiseStates.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,24 @@ public void _2_1_1_1_MayTransitionToEitherTheFulfilledOrRejectedState_void()
string state = null;

var deferred = Promise.NewDeferred();
Assert.IsTrue(deferred.IsValidAndPending);

deferred.Promise
.Then(() => state = Resolved, () => state = Rejected)
.Forget();
Assert.IsNull(state);

deferred.Resolve();
Assert.IsFalse(deferred.IsValidAndPending);

Assert.AreEqual(Resolved, state);

state = null;
deferred = Promise.NewDeferred();
Assert.IsTrue(deferred.IsValidAndPending);

deferred.Promise
.Then(() => state = Resolved, () => state = Rejected)
.Forget();
Assert.IsNull(state);

deferred.Reject("Fail Value");
Assert.IsFalse(deferred.IsValidAndPending);

Assert.AreEqual(Rejected, state);
}

Expand All @@ -62,30 +56,24 @@ public void _2_1_1_1_MayTransitionToEitherTheFulfilledOrRejectedState_T()
string state = null;

var deferred = Promise.NewDeferred<int>();
Assert.IsTrue(deferred.IsValidAndPending);

deferred.Promise
.Then(v => state = Resolved, () => state = Rejected)
.Forget();
Assert.IsNull(state);

deferred.Resolve(1);
Assert.IsFalse(deferred.IsValidAndPending);

Assert.AreEqual(Resolved, state);

state = null;
deferred = Promise.NewDeferred<int>();
Assert.IsTrue(deferred.IsValidAndPending);

deferred.Promise
.Then(v => state = Resolved, () => state = Rejected)
.Forget();
Assert.IsNull(state);

deferred.Reject("Fail Value");
Assert.IsFalse(deferred.IsValidAndPending);

Assert.AreEqual(Rejected, state);
}
}
Expand Down Expand Up @@ -121,10 +109,8 @@ public void _2_1_2_1_MustNotTransitionToAnyOtherState_void()
Assert.IsTrue(voidResolved);
Assert.IsFalse(voidRejected);

Assert.IsFalse(deferred.TryResolve());
Assert.Throws<InvalidOperationException>(() => deferred.Resolve());
Assert.IsFalse(deferred.TryReject(RejectValue));
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
Assert.Catch<InvalidOperationException>(() => deferred.Resolve());
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));

promiseRetainer.WaitAsync()
.Then(() => voidResolved = true, () => voidRejected = true)
Expand All @@ -151,10 +137,8 @@ public void _2_1_2_1_MustNotTransitionToAnyOtherState_T()
Assert.IsTrue(intResolved);
Assert.IsFalse(intRejected);

Assert.IsFalse(deferred.TryResolve(1));
Assert.Throws<InvalidOperationException>(() => deferred.Resolve(1));
Assert.IsFalse(deferred.TryReject(RejectValue));
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
Assert.Catch<InvalidOperationException>(() => deferred.Resolve(1));
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));

promiseRetainer.WaitAsync()
.Then(_ => intResolved = true, () => intRejected = true)
Expand Down Expand Up @@ -187,8 +171,7 @@ public void _2_1_2_2_MustHaveAValueWhichMustNotChange()
onReject: s => Assert.Fail("Promise was rejected when it should have been resolved."),
onUnknownRejection: () => Assert.Fail("Promise was rejected when it should have been resolved.")
);
Assert.IsFalse(deferred.TryResolve(100));
Assert.Throws<InvalidOperationException>(() => deferred.Resolve(100));
Assert.Catch<InvalidOperationException>(() => deferred.Resolve(100));

Assert.AreEqual(expected, result);
}
Expand Down Expand Up @@ -226,10 +209,8 @@ public void _2_1_3_1_MustNotTransitionToAnyOtherState_void()
Assert.IsFalse(voidResolved);
Assert.IsTrue(voidRejected);

Assert.IsFalse(deferred.TryResolve());
Assert.Throws<InvalidOperationException>(() => deferred.Resolve());
Assert.IsFalse(deferred.TryReject(RejectValue));
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
Assert.Catch<InvalidOperationException>(() => deferred.Resolve());
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));

promiseRetainer.WaitAsync()
.Then(() => voidResolved = true, () => voidRejected = true)
Expand All @@ -256,10 +237,8 @@ public void _2_1_3_1_MustNotTransitionToAnyOtherState_T()
Assert.IsFalse(intResolved);
Assert.IsTrue(intRejected);

Assert.IsFalse(deferred.TryResolve(1));
Assert.Throws<InvalidOperationException>(() => deferred.Resolve(1));
Assert.IsFalse(deferred.TryReject(RejectValue));
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
Assert.Catch<InvalidOperationException>(() => deferred.Resolve(1));
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));

promiseRetainer.WaitAsync()
.Then(_ => intResolved = true, () => intRejected = true)
Expand Down Expand Up @@ -288,8 +267,7 @@ public void _2_1_3_2_MustHaveAReasonWhichMustNotChange_void()

Assert.AreEqual(expected, rejection);

Assert.IsFalse(deferred.TryReject("Different Fail Value"));
Assert.Throws<InvalidOperationException>(() => deferred.Reject("Different Fail Value"));
Assert.Catch<InvalidOperationException>(() => deferred.Reject("Different Fail Value"));
TestHelper.AddCallbacks<int, string, string>(promiseRetainer.WaitAsync(),
onResolve: () => Assert.Fail("Promise was resolved when it should have been rejected."),
onReject: failValue =>
Expand Down Expand Up @@ -321,8 +299,7 @@ public void _2_1_3_2_MustHaveAReasonWhichMustNotChange_T()

Assert.AreEqual(expected, rejection);

Assert.IsFalse(deferred.TryReject("Different Fail Value"));
Assert.Throws<InvalidOperationException>(() => deferred.Reject("Different Fail Value"));
Assert.Catch<InvalidOperationException>(() => deferred.Reject("Different Fail Value"));
TestHelper.AddCallbacks<int, bool, string, string>(promiseRetainer.WaitAsync(),
onResolve: v => Assert.Fail("Promise was resolved when it should have been rejected."),
onReject: failValue =>
Expand Down
12 changes: 4 additions & 8 deletions Package/Tests/CoreTests/APIs/APlus_2_2_TheThenMethod.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,7 @@ public void _2_2_2_3_ItMustNotBeCalledMoreThanOnce_void()
);
deferred.Resolve();

Assert.IsFalse(deferred.TryResolve());
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Resolve());
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Resolve());

Assert.AreEqual(
(TestHelper.resolveVoidVoidCallbacks + TestHelper.resolveVoidConvertCallbacks +
Expand All @@ -337,8 +336,7 @@ public void _2_2_2_3_ItMustNotBeCalledMoreThanOnce_T()
);
deferred.Resolve(1);

Assert.IsFalse(deferred.TryResolve(1));
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Resolve(100));
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Resolve(100));

Assert.AreEqual(
(TestHelper.resolveTVoidCallbacks + TestHelper.resolveTConvertCallbacks +
Expand Down Expand Up @@ -452,8 +450,7 @@ public void _2_2_3_3_ItMustNotBeCalledMoreThanOnce_void()
);
deferred.Reject("Fail value");

Assert.IsFalse(deferred.TryReject("Fail value"));
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));

Assert.AreEqual(
(TestHelper.rejectVoidVoidCallbacks + TestHelper.rejectVoidConvertCallbacks +
Expand All @@ -475,8 +472,7 @@ public void _2_2_3_3_ItMustNotBeCalledMoreThanOnce_T()
);
deferred.Reject("Fail value");

Assert.IsFalse(deferred.TryReject("Fail value"));
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));

Assert.AreEqual(
(TestHelper.rejectTVoidCallbacks + TestHelper.rejectTConvertCallbacks + TestHelper.rejectTTCallbacks +
Expand Down
10 changes: 8 additions & 2 deletions Package/Tests/CoreTests/APIs/AsyncFunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,10 @@ public void AsyncLocalWorks_void([Values] bool isPending)
var deferred = isPending ? Promise.NewDeferred() : default(Promise.Deferred);
_promise = isPending ? deferred.Promise : Promise.Resolved();
FuncVoid().Forget();
deferred.TryResolve();
if (isPending)
{
deferred.Resolve();
}
}

private async Promise FuncVoid()
Expand Down Expand Up @@ -936,7 +939,10 @@ public void AsyncLocalWorks_T([Values] bool isPending)
var deferred = isPending ? Promise.NewDeferred() : default(Promise.Deferred);
_promise = isPending ? deferred.Promise : Promise.Resolved();
FuncT().Forget();
deferred.TryResolve();
if (isPending)
{
deferred.Resolve();
}
}

private async Promise<int> FuncT()
Expand Down
20 changes: 16 additions & 4 deletions Package/Tests/CoreTests/APIs/AwaitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,10 @@ public void PromiseToTaskIsResolvedProperly_void([Values] bool isPending)
bool completed = false;

Func();
deferred.TryResolve();
if (isPending)
{
deferred.Resolve();
}

Assert.IsTrue(completed);

Expand All @@ -923,7 +926,10 @@ public void PromiseToTaskIsResolvedProperly_T([Values] bool isPending)
bool completed = false;

Func();
deferred.TryResolve(1);
if (isPending)
{
deferred.Resolve(1);
}

Assert.IsTrue(completed);

Expand Down Expand Up @@ -992,7 +998,10 @@ public void PromiseAsValueTaskIsResolvedProperly_void([Values] bool isPending)
bool completed = false;

Func();
deferred.TryResolve();
if (isPending)
{
deferred.Resolve();
}
TestHelper.ExecuteForegroundCallbacksAndWaitForThreadsToComplete();

Assert.IsTrue(completed);
Expand All @@ -1013,7 +1022,10 @@ public void PromiseAsValueTaskIsResolvedProperly_T([Values] bool isPending)
bool completed = false;

Func();
deferred.TryResolve(1);
if (isPending)
{
deferred.Resolve(1);
}
TestHelper.ExecuteForegroundCallbacksAndWaitForThreadsToComplete();

Assert.IsTrue(completed);
Expand Down
Loading
Loading