Skip to content

Commit e6ea52b

Browse files
timcasselltimcassell
authored
Merge pull request #510 from timcassell/deprecate-isvalid
Deprecate `{IsValid*, Try*}` APIs.
2 parents 8ba0c19 + 5930503 commit e6ea52b

25 files changed

+275
-275
lines changed

‎Package/Core/InternalShared/DebugInternal.cs

+10
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ internal static void ValidateArgument<TArg>(TArg arg, string argName, int skipFr
162162
}
163163
internal static void ValidateArgument(Promise arg, string argName, int skipFrames)
164164
{
165+
#pragma warning disable CS0618 // Type or member is obsolete
165166
if (!arg.IsValid)
167+
#pragma warning restore CS0618 // Type or member is obsolete
166168
{
167169
throw new InvalidArgumentException(argName,
168170
"Promise is invalid. Call `GetRetainer()` if you intend to await multiple times.",
@@ -252,7 +254,9 @@ private IEnumerable<StackTrace> GetStackTraces()
252254

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

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

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

523531
static partial void ValidateElement(Promise<T> promise, string argName, int skipFrames)
524532
{
533+
#pragma warning disable CS0618 // Type or member is obsolete
525534
if (!promise.IsValid)
535+
#pragma warning restore CS0618 // Type or member is obsolete
526536
{
527537
throw new InvalidElementException(argName,
528538
$"A promise is invalid in {argName}." +

‎Package/Core/Promises/Deferred.cs

+72-32
Large diffs are not rendered by default.

‎Package/Core/Promises/Internal/DeferredInternal.cs

-22
Original file line numberDiff line numberDiff line change
@@ -100,28 +100,6 @@ internal static DeferredPromise<TResult> GetOrCreate()
100100
return promise;
101101
}
102102

103-
[MethodImpl(InlineOption)]
104-
internal static bool TryResolve(DeferredPromise<TResult> _this, int deferredId, in TResult value)
105-
{
106-
if (_this?.TryIncrementDeferredId(deferredId) == true)
107-
{
108-
_this.ResolveDirect(value);
109-
return true;
110-
}
111-
return false;
112-
}
113-
114-
[MethodImpl(InlineOption)]
115-
internal static bool TryResolveVoid(DeferredPromise<TResult> _this, int deferredId)
116-
{
117-
if (_this?.TryIncrementDeferredId(deferredId) == true)
118-
{
119-
_this.ResolveDirectVoid();
120-
return true;
121-
}
122-
return false;
123-
}
124-
125103
[MethodImpl(InlineOption)]
126104
internal void ResolveDirect(in TResult value)
127105
{

‎Package/Core/Promises/Promise.cs

-7
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,6 @@ public static implicit operator System.Threading.Tasks.ValueTask(in Promise rhs)
4343
=> rhs.AsValueTask();
4444
#endif // UNITY_2021_2_OR_NEWER || !UNITY_2018_3_OR_NEWER
4545

46-
/// <summary>
47-
/// Gets whether this instance is valid to be awaited.
48-
/// </summary>
49-
public bool IsValid
50-
// 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.
51-
=> _ref?.GetIsValid(_id) != false;
52-
5346
/// <summary>
5447
/// Mark this as awaited and prevent any further awaits or callbacks on this.
5548
/// <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).

‎Package/Core/Promises/PromiseT.cs

-7
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,6 @@ public static implicit operator System.Threading.Tasks.ValueTask(in Promise<T> r
5858
=> rhs.AsValueTaskVoid();
5959
#endif // UNITY_2021_2_OR_NEWER || !UNITY_2018_3_OR_NEWER
6060

61-
/// <summary>
62-
/// Gets whether this instance is valid to be awaited.
63-
/// </summary>
64-
public bool IsValid
65-
// 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.
66-
=> _ref?.GetIsValid(_id) != false;
67-
6861
/// <summary>
6962
/// Cast to <see cref="Promise"/>.
7063
/// </summary>

‎Package/Core/Promises/PromiseT_Deprecated.cs

+8
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@ namespace Proto.Promises
2121
/// </summary>
2222
public readonly partial struct Promise<T> : IEquatable<Promise<T>>
2323
{
24+
/// <summary>
25+
/// Gets whether this instance is valid to be awaited.
26+
/// </summary>
27+
[Obsolete("Due to object pooling, this property is inherently unsafe. This will be removed in a future version.", false), EditorBrowsable(EditorBrowsableState.Never)]
28+
public bool IsValid
29+
// 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.
30+
=> _ref?.GetIsValid(_id) != false;
31+
2432
/// <summary>
2533
/// 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.
2634
/// <para/><see cref="Forget"/> must be called when you are finished with it.

‎Package/Core/Promises/Promise_Deprecated.cs

+8
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ namespace Proto.Promises
1313
{
1414
partial struct Promise
1515
{
16+
/// <summary>
17+
/// Gets whether this instance is valid to be awaited.
18+
/// </summary>
19+
[Obsolete("Due to object pooling, this property is inherently unsafe. This will be removed in a future version.", false), EditorBrowsable(EditorBrowsableState.Never)]
20+
public bool IsValid
21+
// 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.
22+
=> _ref?.GetIsValid(_id) != false;
23+
1624
/// <summary>
1725
/// 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.
1826
/// <para/><see cref="Forget"/> must be called when you are finished with it.

‎Package/Tests/CoreTests/APIs/APlus_2_1_PromiseStates.cs

+11-34
Original file line numberDiff line numberDiff line change
@@ -28,30 +28,24 @@ public void _2_1_1_1_MayTransitionToEitherTheFulfilledOrRejectedState_void()
2828
string state = null;
2929

3030
var deferred = Promise.NewDeferred();
31-
Assert.IsTrue(deferred.IsValidAndPending);
3231

3332
deferred.Promise
3433
.Then(() => state = Resolved, () => state = Rejected)
3534
.Forget();
3635
Assert.IsNull(state);
3736

3837
deferred.Resolve();
39-
Assert.IsFalse(deferred.IsValidAndPending);
40-
4138
Assert.AreEqual(Resolved, state);
4239

4340
state = null;
4441
deferred = Promise.NewDeferred();
45-
Assert.IsTrue(deferred.IsValidAndPending);
4642

4743
deferred.Promise
4844
.Then(() => state = Resolved, () => state = Rejected)
4945
.Forget();
5046
Assert.IsNull(state);
5147

5248
deferred.Reject("Fail Value");
53-
Assert.IsFalse(deferred.IsValidAndPending);
54-
5549
Assert.AreEqual(Rejected, state);
5650
}
5751

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

6458
var deferred = Promise.NewDeferred<int>();
65-
Assert.IsTrue(deferred.IsValidAndPending);
6659

6760
deferred.Promise
6861
.Then(v => state = Resolved, () => state = Rejected)
6962
.Forget();
7063
Assert.IsNull(state);
7164

7265
deferred.Resolve(1);
73-
Assert.IsFalse(deferred.IsValidAndPending);
74-
7566
Assert.AreEqual(Resolved, state);
7667

7768
state = null;
7869
deferred = Promise.NewDeferred<int>();
79-
Assert.IsTrue(deferred.IsValidAndPending);
8070

8171
deferred.Promise
8272
.Then(v => state = Resolved, () => state = Rejected)
8373
.Forget();
8474
Assert.IsNull(state);
8575

8676
deferred.Reject("Fail Value");
87-
Assert.IsFalse(deferred.IsValidAndPending);
88-
8977
Assert.AreEqual(Rejected, state);
9078
}
9179
}
@@ -121,10 +109,8 @@ public void _2_1_2_1_MustNotTransitionToAnyOtherState_void()
121109
Assert.IsTrue(voidResolved);
122110
Assert.IsFalse(voidRejected);
123111

124-
Assert.IsFalse(deferred.TryResolve());
125-
Assert.Throws<InvalidOperationException>(() => deferred.Resolve());
126-
Assert.IsFalse(deferred.TryReject(RejectValue));
127-
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
112+
Assert.Catch<InvalidOperationException>(() => deferred.Resolve());
113+
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));
128114

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

154-
Assert.IsFalse(deferred.TryResolve(1));
155-
Assert.Throws<InvalidOperationException>(() => deferred.Resolve(1));
156-
Assert.IsFalse(deferred.TryReject(RejectValue));
157-
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
140+
Assert.Catch<InvalidOperationException>(() => deferred.Resolve(1));
141+
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));
158142

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

193176
Assert.AreEqual(expected, result);
194177
}
@@ -226,10 +209,8 @@ public void _2_1_3_1_MustNotTransitionToAnyOtherState_void()
226209
Assert.IsFalse(voidResolved);
227210
Assert.IsTrue(voidRejected);
228211

229-
Assert.IsFalse(deferred.TryResolve());
230-
Assert.Throws<InvalidOperationException>(() => deferred.Resolve());
231-
Assert.IsFalse(deferred.TryReject(RejectValue));
232-
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
212+
Assert.Catch<InvalidOperationException>(() => deferred.Resolve());
213+
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));
233214

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

259-
Assert.IsFalse(deferred.TryResolve(1));
260-
Assert.Throws<InvalidOperationException>(() => deferred.Resolve(1));
261-
Assert.IsFalse(deferred.TryReject(RejectValue));
262-
Assert.Throws<InvalidOperationException>(() => deferred.Reject(RejectValue));
240+
Assert.Catch<InvalidOperationException>(() => deferred.Resolve(1));
241+
Assert.Catch<InvalidOperationException>(() => deferred.Reject(RejectValue));
263242

264243
promiseRetainer.WaitAsync()
265244
.Then(_ => intResolved = true, () => intRejected = true)
@@ -288,8 +267,7 @@ public void _2_1_3_2_MustHaveAReasonWhichMustNotChange_void()
288267

289268
Assert.AreEqual(expected, rejection);
290269

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

322300
Assert.AreEqual(expected, rejection);
323301

324-
Assert.IsFalse(deferred.TryReject("Different Fail Value"));
325-
Assert.Throws<InvalidOperationException>(() => deferred.Reject("Different Fail Value"));
302+
Assert.Catch<InvalidOperationException>(() => deferred.Reject("Different Fail Value"));
326303
TestHelper.AddCallbacks<int, bool, string, string>(promiseRetainer.WaitAsync(),
327304
onResolve: v => Assert.Fail("Promise was resolved when it should have been rejected."),
328305
onReject: failValue =>

‎Package/Tests/CoreTests/APIs/APlus_2_2_TheThenMethod.cs

+4-8
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,7 @@ public void _2_2_2_3_ItMustNotBeCalledMoreThanOnce_void()
309309
);
310310
deferred.Resolve();
311311

312-
Assert.IsFalse(deferred.TryResolve());
313-
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Resolve());
312+
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Resolve());
314313

315314
Assert.AreEqual(
316315
(TestHelper.resolveVoidVoidCallbacks + TestHelper.resolveVoidConvertCallbacks +
@@ -337,8 +336,7 @@ public void _2_2_2_3_ItMustNotBeCalledMoreThanOnce_T()
337336
);
338337
deferred.Resolve(1);
339338

340-
Assert.IsFalse(deferred.TryResolve(1));
341-
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Resolve(100));
339+
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Resolve(100));
342340

343341
Assert.AreEqual(
344342
(TestHelper.resolveTVoidCallbacks + TestHelper.resolveTConvertCallbacks +
@@ -452,8 +450,7 @@ public void _2_2_3_3_ItMustNotBeCalledMoreThanOnce_void()
452450
);
453451
deferred.Reject("Fail value");
454452

455-
Assert.IsFalse(deferred.TryReject("Fail value"));
456-
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));
453+
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));
457454

458455
Assert.AreEqual(
459456
(TestHelper.rejectVoidVoidCallbacks + TestHelper.rejectVoidConvertCallbacks +
@@ -475,8 +472,7 @@ public void _2_2_3_3_ItMustNotBeCalledMoreThanOnce_T()
475472
);
476473
deferred.Reject("Fail value");
477474

478-
Assert.IsFalse(deferred.TryReject("Fail value"));
479-
Assert.Throws<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));
475+
Assert.Catch<Proto.Promises.InvalidOperationException>(() => deferred.Reject("Fail value"));
480476

481477
Assert.AreEqual(
482478
(TestHelper.rejectTVoidCallbacks + TestHelper.rejectTConvertCallbacks + TestHelper.rejectTTCallbacks +

‎Package/Tests/CoreTests/APIs/AsyncFunctionTests.cs

+8-2
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,10 @@ public void AsyncLocalWorks_void([Values] bool isPending)
907907
var deferred = isPending ? Promise.NewDeferred() : default(Promise.Deferred);
908908
_promise = isPending ? deferred.Promise : Promise.Resolved();
909909
FuncVoid().Forget();
910-
deferred.TryResolve();
910+
if (isPending)
911+
{
912+
deferred.Resolve();
913+
}
911914
}
912915

913916
private async Promise FuncVoid()
@@ -936,7 +939,10 @@ public void AsyncLocalWorks_T([Values] bool isPending)
936939
var deferred = isPending ? Promise.NewDeferred() : default(Promise.Deferred);
937940
_promise = isPending ? deferred.Promise : Promise.Resolved();
938941
FuncT().Forget();
939-
deferred.TryResolve();
942+
if (isPending)
943+
{
944+
deferred.Resolve();
945+
}
940946
}
941947

942948
private async Promise<int> FuncT()

‎Package/Tests/CoreTests/APIs/AwaitTests.cs

+16-4
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,10 @@ public void PromiseToTaskIsResolvedProperly_void([Values] bool isPending)
903903
bool completed = false;
904904

905905
Func();
906-
deferred.TryResolve();
906+
if (isPending)
907+
{
908+
deferred.Resolve();
909+
}
907910

908911
Assert.IsTrue(completed);
909912

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

925928
Func();
926-
deferred.TryResolve(1);
929+
if (isPending)
930+
{
931+
deferred.Resolve(1);
932+
}
927933

928934
Assert.IsTrue(completed);
929935

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

9941000
Func();
995-
deferred.TryResolve();
1001+
if (isPending)
1002+
{
1003+
deferred.Resolve();
1004+
}
9961005
TestHelper.ExecuteForegroundCallbacksAndWaitForThreadsToComplete();
9971006

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

10151024
Func();
1016-
deferred.TryResolve(1);
1025+
if (isPending)
1026+
{
1027+
deferred.Resolve(1);
1028+
}
10171029
TestHelper.ExecuteForegroundCallbacksAndWaitForThreadsToComplete();
10181030

10191031
Assert.IsTrue(completed);

0 commit comments

Comments
 (0)