This repository has been archived by the owner on Jan 18, 2022. It is now read-only.
Refactoring the mobile package #929
Merged
Conversation
This file contains 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
improbable-prow-robot
added
A: core
jamiebrynes7
reviewed
May 2, 2019
workers/unity/Packages/com.improbable.gdk.mobile/Utility/DeviceInfo.cs
Outdated
Show resolved
Hide resolved
workers/unity/Packages/com.improbable.gdk.mobile/Worker/DefaultMobileWorkerConnector.cs
Outdated
Show resolved
Hide resolved
workers/unity/Packages/com.improbable.gdk.mobile/Worker/DefaultMobileWorkerConnector.cs
Show resolved
Hide resolved
workers/unity/Packages/com.improbable.gdk.mobile/Worker/DefaultMobileWorkerConnector.cs
Show resolved
Hide resolved
workers/unity/Packages/com.improbable.gdk.mobile/Worker/DefaultMobileWorkerConnector.cs
Show resolved
Hide resolved
workers/unity/Packages/com.improbable.gdk.tools/GdkToolsConfigurationWindow.cs
Outdated
Show resolved
Hide resolved
improbable-prow-robot
added
A: build-system
jamiebrynes7
reviewed
May 2, 2019
| @@ -2,10 +2,14 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| ### Breaking Changes | |||
| - Removed the `Improbable.Gdk.Mobile.Android` and `Improbable.Gdk.Mobile.iOS` packages. All functionality is now available inside the `Improbable.Gdk.Mobile` package. | |||
There was a problem hiding this comment.
Were the OS specific worker connectors public or no?
There was a problem hiding this comment.
they were in the playground. So kinda?
jessicafalk
force-pushed
the
feature/mobile-rework
branch
from
a214427 to
6f6330f
Compare
jamiebrynes7
approved these changes
May 3, 2019
| @@ -52,7 +52,7 @@ steps: | |||
| - logs/**/* | |||
| - workers/unity/build/worker/**/* | |||
| env: | |||
| WORKER_TYPE: "AndroidClient" | |||
| WORKER_TYPE: "MobileClient" | |||
There was a problem hiding this comment.
Can you change the label as well? 🙏
| @@ -38,7 +38,7 @@ steps: | |||
| - logs/**/* | |||
| - build/assembly/**/* | |||
| env: | |||
| WORKER_TYPE: "AndroidClient" | |||
| WORKER_TYPE: "MobileClient" | |||
There was a problem hiding this comment.
Change label here as well please 🙏
zeroZshadow
approved these changes
May 3, 2019
| { | ||
| public static class DeviceInfo | ||
| { | ||
| public const string AndroidEmulatorDefaultCallbackIp = "10.0.2.2"; |
There was a problem hiding this comment.
the ip necessary to connect your emulator to your localhost
| try | ||
| { | ||
| using (var unityPlayer = new UnityEngine.AndroidJavaClass("com.unity3d.player.UnityPlayer")) | ||
| using (var unityPlayer = new AndroidJavaClass("com.unity3d.player.UnityPlayer")) |
There was a problem hiding this comment.
If this is changed, should the 2 below also be cleaned up?
| { | ||
| Heartbeat = new HeartbeatParameters() | ||
| { | ||
| IntervalMillis = 5000, |
There was a problem hiding this comment.
These 2 values might need a comment as to why they are those values
There was a problem hiding this comment.
these values might change. Need to double-check with people, so would prefer not to do this now. The values have never changed since we introduced them btw
jessicafalk
pushed a commit
that referenced
this pull request
Nov 15, 2019
* Added generation of entities to ensure offloaded workers receive updates for the GSM (#929) * Added generation of entities to ensure offloaded workers receive updates for the GSM * Rename * More formatting * Update SpatialGDK/Source/SpatialGDKEditor/Private/SnapshotGenerator/SpatialGDKEditorSnapshotGenerator.cpp Co-Authored-By: Ally <[email protected]> * Update SpatialGDK/Source/SpatialGDKEditor/Private/SnapshotGenerator/SpatialGDKEditorSnapshotGenerator.cpp Co-Authored-By: Ally <[email protected]> * Update SpatialGDK/Source/SpatialGDKEditor/Private/SnapshotGenerator/SpatialGDKEditorSnapshotGenerator.cpp Co-Authored-By: Ally <[email protected]> * Updated Based on feedback * Feedback * Updated feedback * Created an entity representing a worker connection * Removed whitespace * Added release notes * Removed unused line * Removed another unused line * Updated line order * Remove usages of improbable namespace after rebase * Worker entity has interest query for the GSM entity This is a way to ensure the GSM is available on all server workers * Refactor snapshot generator to use worker types from new settings * Create worker entity in helper function and move call * Remove placeholder entities * Set worker entity metadata to WorkerEntity:<id> * Update changelog * Update CHANGELOG.md Co-Authored-By: Matt Young <[email protected]> * Retry failed worker entity creation * Update comment as suggested * Update CHANGELOG.md Co-Authored-By: Sahil Dhanju <[email protected]> * Fix and refactor retry logic Fixed off-by-one error in retry counter. Only retry if the status is timeout. * Move attempt counter to be a function argument * Remove one level of indentation from conditional As suggested by @alastairdglennie. * Capitalize names of local variables * Move CreateEntityDelegate from Dispatcher to Receiver * Move CreateServerWorkerEntity to Sender * Pass delegate by const reference as suggested by @danielimprobable * Use TWeakObjectPtr and do checks in timer lambdas * Update CHANGELOG.md Co-Authored-By: Ally <[email protected]> * Sort includes * Add comments to USpatialSender::CreateServerWorkerEntity
jessicafalk
pushed a commit
that referenced
this pull request
Nov 15, 2019
…g. (#1059) * Add ActorGroupManager and new runtime settings * Working. * Refactor Validation logic into ActorGroupManager * Prevent user from adding 'UnrealClient' as a WorkerType * Cleanup comments / blueprint tests. Wire up GetWorkerTypeForActorClass * Ensure that AActor is referenced in at least 1 ActorGroup * Tidy up * Moved ActorGroupManager to Utils. Add some error messages in ClassInfoManager if ActorGroups / WorkerType mappings not found * ActorGroupManager.h -> Utils/ActorGroupManager.h * Remove old function signature * Add switch to enable offloading (Default off) which makes ActorGroupManager use the defaults. Make some magic strings constants * Remove Debugging Log * Use DetailFont in WorkerAssociationCustomization * Make WorkerTypes FStrings in Settings * Copy WorkerTypeNames from SpatialGDKEditorSettings->LaunchConfigDesc.Workers to RuntimeSettings & Validate * Make WorkerTypes Invisible * Add Number of Editor Instances to Worker Launch Config * Refactored Worker Association to inside ActorGroups. * Change WorkerTypeName into an FName * Update usage of FName WorkerType * Remove PostEdit hooks for validation that has been removed * Remove now unused FWorkerAssociation * Move ActorGroupManager init to NetDriver and pass down to ClassInfoManager * Only add mapping from Class->ActorGroup if it doesn't exist * Remove Debug Log * WorkerAssociationCustomization -> WorkerTypeCustomization * Syntax fixes from review comments * Add VeryVerbose Log to ClassInfoManager about ActorGroups * Remove PostEditChange for bUsingQBI since it's not exposed in the editor anymore * Add comments to public api of ActorGroupManager. Revert plugin back to Default loading phase * Fixed references to NumberOfServers slider as it was removed * The settings in the GDK for what workers to launch and whether offloading is enabled or not is now propagated to the LevelEditorPlaySettings * Updated changelog * Review feedback * More feedback related to FNames * Fixed compilation error due to change from fstring to fname * Removed unused header include * Introduced the spatialengineconstants for server and client worker names * Review feedback * Removed redundant include * Updated FString to FName references * Updated renaming * More Review comments * Fixed bad rebase * Another review comment * Review Comments * Added comment explaining usage of TSoftClassPtr here * Access each element in the container as reference to avoid local copying * Review comments * Removed unused include * Added generation of entities to ensure offloaded workers receive updates for the GSM (#929) * Added generation of entities to ensure offloaded workers receive updates for the GSM * Rename * More formatting * Update SpatialGDK/Source/SpatialGDKEditor/Private/SnapshotGenerator/SpatialGDKEditorSnapshotGenerator.cpp Co-Authored-By: Ally <[email protected]> * Update SpatialGDK/Source/SpatialGDKEditor/Private/SnapshotGenerator/SpatialGDKEditorSnapshotGenerator.cpp Co-Authored-By: Ally <[email protected]> * Update SpatialGDK/Source/SpatialGDKEditor/Private/SnapshotGenerator/SpatialGDKEditorSnapshotGenerator.cpp Co-Authored-By: Ally <[email protected]> * Updated Based on feedback * Feedback * Updated feedback * Created an entity representing a worker connection * Removed whitespace * Added release notes * Removed unused line * Removed another unused line * Updated line order * Remove usages of improbable namespace after rebase * Worker entity has interest query for the GSM entity This is a way to ensure the GSM is available on all server workers * Refactor snapshot generator to use worker types from new settings * Create worker entity in helper function and move call * Remove placeholder entities * Set worker entity metadata to WorkerEntity:<id> * Update changelog * Update CHANGELOG.md Co-Authored-By: Matt Young <[email protected]> * Retry failed worker entity creation * Update comment as suggested * Update CHANGELOG.md Co-Authored-By: Sahil Dhanju <[email protected]> * Fix and refactor retry logic Fixed off-by-one error in retry counter. Only retry if the status is timeout. * Move attempt counter to be a function argument * Remove one level of indentation from conditional As suggested by @alastairdglennie. * Capitalize names of local variables * Move CreateEntityDelegate from Dispatcher to Receiver * Move CreateServerWorkerEntity to Sender * Pass delegate by const reference as suggested by @danielimprobable * Use TWeakObjectPtr and do checks in timer lambdas * Update CHANGELOG.md Co-Authored-By: Ally <[email protected]> * Fixed references to NumberOfServers slider as it was removed * The settings in the GDK for what workers to launch and whether offloading is enabled or not is now propagated to the LevelEditorPlaySettings * Updated changelog * Review feedback * More feedback related to FNames * Fixed compilation error due to change from fstring to fname * Removed unused header include * Introduced the spatialengineconstants for server and client worker names * Update changelog * Review feedback * Removed redundant include * Updated FString to FName references * Updated renaming * Fixed bad rebase * Update CHANGELOG.md Co-Authored-By: Matt Young <[email protected]> * Remove hardcoding of AIWorker (#924) * Remove hardcoding of AIWorker This adds the auto-populated list property to SpatialGDKSettings The list property gets reconstructed from the list of server worker names from the Launch Config Description property * Removed unused comments * Removed the commented code * Removed unused include * Update CHANGELOG.md Co-Authored-By: Sahil Dhanju <[email protected]> * Fixed bug where two worker types were assigned to the write ACL * Updated changelog * Updated the Snapshot generator to configure the GSM to for read access on all server workers * Readded header include that accidentally shouldn't have been removed * worker association can't be empty, removing check * worker type is fname now * server worker types * don't force push with merge conflicts * I wonder if one day I'll stop pushing merge conflicts * fix old reference name, remove rogue space * alphabetize imports * derp * level editor play settings workers to launch cleared before updates are applied * delete unnecessary import, update engine version * bump engine version check counter * runtime settings updates level editor play settings with default worker type and offloading toggle * formatting * move changelog entry to correct section, remove accidental spaces * remove duplicate includes and method definitions * remove newline, remove unnecessary namespace * remove unnecessary header * bad merge, ally dumb
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description
AndroidClient and iOSClient don't exist anymore. There is now one MobileClient that can be used for either Android or iOS.
Also added new menu items to launch the apk either for a local or a cloud deployment. This allows us to not have to rebuild the worker everytime you want to switch between connecting to a cloud or a local deployment.
Got rid of the mobile screen we had. Now you just connect as soon as the game starts.
Tests
Ran it locally and in cloud for Android & iOS.
Documentation
coming soon
Primary reviewers
If your change will take a long time to review, you can name at most two primary reviewers who are ultimately responsible for reviewing this request. @ mention them.