[OF-1736] feat: Expose AsyncCall to DuplicateSpace #844
+419
−243
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🚀 Pull Request Review GuidelinesThank you for taking the time to review this Pull Request. The following is a summary of our Pull Request guidelines. The full guidelines can be found here. 💬 How to Provide FeedbackWe use a comment ladder when leaving review comments to avoid any ambiguity.
All comments should be prefixed with one of the above tags, for example: 🎯 PR Author Focus Areas
Thanks again for taking the time to review this Pull Request. |
MAG-AdamThorn
commented
Oct 28, 2025
MAG-ElliotMorris
requested changes
Oct 29, 2025
- Update the error message logged when the 'AsyncCallCompletedCallback' is not set successfully. - Correct a typo comment in the 'AsyncCallCompletedEventData' class.
The SpaceSystem was being passed to the NetworkEventBus ctor by ptr and then we had a nullptr check to ensure it was valid before attempting to add an event listener. However, the NetworkEventBus cannot be null and should therefore be passed by ref. The MultiplayerConnection::GetEventBus() method has been updated to return a reference to the EventBus. This ref is then passed to the systems, which dereference it before passing it to the SystemBase class. This is similar to what is done with the LogSystem. I also took the opportunity to remove any 'In' prefixes from related method arguments. [OF-1736] fix: Update
- Added more detailed comments to the DuplicateSpaceAsync() method to enumerate the operation data that would be returned via the AsyncCallCompletedCallback. - Updated the comments for the OnAsyncCallCompletedEvent() method to match.
-Updated the DuplicateSpaceAsyncTest promise to capture the NetworkEventData rather than asserting equality within the lambda. - Added a 30 second timeout to the future wait to account for the async operation timing out due to backed service load. - Updated the EXPECT_* to be ASSERT_*.
MAG-AdamThorn
force-pushed
the
OF-1736_Expose_AsyncCall_field_to_DuplicateSpace_call
branch
from
10ba011 to
e6d8ab2
Compare
MAG-mv
requested changes
Oct 30, 2025
… dtor Previously individual systems were responsible for deregistering their system callbacks with the NetworkEventBus on destruction. However, the NetworkEventBus (which is owned by the MultiplayerConnection) is one of the last systems to be cleaned up by the SystemsManager, so it made sense to move this logic to the NetworkEventBus dtor.
Updated docs for the DuplicateSpace and DuplicateSpaceAsync methods to outline behaviour if the user disconnects while the operation is being carried out.
- Inject UserSystem into the SpaceSystem via it's ctor so that it can be used without having to rely on the singleton. - Adopt the Convert utility for creating a vector of MemberGroupIds from the provided Optional Array.
MAG-ElliotMorris
approved these changes
Oct 31, 2025
MAG-mv
approved these changes
Oct 31, 2025
Updated the SystemsManager::GetEventBus() method to return a ptr to the EventBus. I had updated it to return a ref forgetting that the wrapper generator does not support that.
MAG-AdamThorn
deleted the
OF-1736_Expose_AsyncCall_field_to_DuplicateSpace_call
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current DuplicateSpace method is a synchronous operation that can timeout and fail if the Space is overly complex or the backend services are under excessive load.
A new generic
AsyncCallCompletedevent has been added by the backend services to alert the user when a specific async operation has completed. Operations such as DuplicateSpace can take a AsyncCall bool argument, which determines whether the operation should be performed synchronously or asynchronously.If performed asynchronously the call will immediately return a result object, which in the case of Duplicate Space will contain a 204 - No content response on success. Then, once the actual duplication operation has been completed, the
AsyncCallCompletedevent will fire, passing aAsyncCallCompletedEventDataobject.As part of this work the current
DuplicateSpacemethod has been marked deprecated and a newDuplicateSpaceAsyncmethod has been introduced. This approach has been taken for two reasons:A new
AsyncCallCompletedCallbackHandlerhas been introduced along with a public setter to allow clients to register for theAsyncCallCompletedevent:SetAsyncCallCompletedCallback().As noted above, this event will return a generic 'AsyncCallCompletedEventData` object, which in the case of the duplicate Space operation, will contain the following data: