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

feat: Third Person Character Controller integration with custom IP UI #66

Conversation

fernando-cortez
Copy link
Collaborator

@fernando-cortez fernando-cortez commented Nov 9, 2022

This PR adds a new Player Prefab for NetworkManager, based off of the Third Person Controller Starter Assets. Previous functionality of movement, collisions, and picking up is intact. The new Input System has been integration for input polling. ClientNetworkAnimator enables the animations of the player to be client-driven.

Included as well is an overhaul on the UI to now include custom IPs, using UI Toolkit.

MTT-5013

Package Manager manifest will target the new Boss Room release containing the new Utilities release, once that is out!

Testing instructions:

  • Create a project clone via ParrelSync, or create a build of the game. To test the game via the editor, open up the Bootstrap scene and hit Play.
  • Have one instance of the game host, and have other instances join the host.
  • Here, validate that all clients see animations being played out from every other client. Test picking up ingredients with E.

Copy link
Contributor

@LPLafontaineB LPLafontaineB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor nitpicks in code. Otherwise I see an issue where when we drop an object it disappears into the ground. Also another really nitpicky comment, but since we can now jump, maybe the "counter" should be a bit higher so that we don't notice the invisible wall too much? It might be something for a later PR though

Basic/ClientDriven/Assets/Scripts/ClientPlayerMove.cs Outdated Show resolved Hide resolved
Basic/ClientDriven/Assets/Scripts/ClientPlayerMove.cs Outdated Show resolved Hide resolved
Basic/ClientDriven/Assets/Scripts/HostJoinUI.cs Outdated Show resolved Hide resolved
@RikuTheFuffs
Copy link
Contributor

RikuTheFuffs commented Nov 14, 2022

@fernando-cortez would it be possible to have some testing instructions listed in the PR? I'd like to test it locally, but dont't really know if I should do something in particular to set up the environment or if there's a specific key combination to test

@RikuTheFuffs RikuTheFuffs self-requested a review November 14, 2022 14:53
@fernando-cortez
Copy link
Collaborator Author

@fernando-cortez would it be possible to have some testing instructions listed in the PR? I'd like to test it locally, but dont't really know if I should do something in particular to set up the environment or if there's a specific key combination to test

Absolutely, I'll include some test instructions in the summary!

Copy link
Contributor

@RikuTheFuffs RikuTheFuffs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. There seem to be some warnings related to the Chintzy font (no stack trace). Do you know what's causing them? Did you try addressing them already? You can let them pop up by dropping a colored object in one of the circles of the same color as the object

image

  1. There's something wrong with one of the network variables:

image

  1. Going out of play mode after playing for a while thows:
Type of instance in array does not match expected type
UnityEngine.Object:Destroy (UnityEngine.Object)
Unity.Netcode.NetworkSpawnManager:OnDespawnObject (Unity.Netcode.NetworkObject,bool) (at Library/PackageCache/[email protected]/Runtime/Spawning/NetworkSpawnManager.cs:873)
Unity.Netcode.NetworkSpawnManager:DespawnAndDestroyNetworkObjects () (at Library/PackageCache/[email protected]/Runtime/Spawning/NetworkSpawnManager.cs:699)
Unity.Netcode.NetworkManager:ShutdownInternal () (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:1439)
Unity.Netcode.NetworkManager:OnDestroy () (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:1331)
Unity.Netcode.NetworkManager:OnApplicationQuit () (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:1325)

  1. Some fields don't seem to respect coding conventions (I.E: the private word is used even when unneeded, and private fields sometiems don't start with m_). This is not a blocker for my approval, but it's something we should be aware of.

@RikuTheFuffs
Copy link
Contributor

Also, I'm not sure if this is on purpose, but there seem to be some interactivity problems:

  1. Dropping an object outside of a circle by pressingthe E button again makes the object fall under the ground
  2. Under certain circumstances, I was able to grab objects very far away and to make them follow my character's rotation. I didn't manage to repro this consistently, but spamming the E button probably helps
  3. Sometimes, I was able to pick up objects even when I had nothing around me. Pressing E would just make something pop up on me, as if I summoned it out of thin air

@jilfranco-unity
Copy link
Contributor

jilfranco-unity commented Nov 16, 2022

  1. There seem to be some warnings related to the Chintzy font (no stack trace). Do you know what's causing them? Did you try addressing them already? You can let them pop up by dropping a colored object in one of the circles of the same color as the object

image

This is something that I will address in an art PR! @RikuTheFuffs

@fernando-cortez fernando-cortez force-pushed the feat/third-person-character-controller-integration-IP-UI branch from 1d77288 to 62ddb88 Compare December 2, 2022 16:41
@fernando-cortez fernando-cortez merged commit b8a5775 into develop Dec 6, 2022
@fernando-cortez fernando-cortez deleted the feat/third-person-character-controller-integration-IP-UI branch December 6, 2022 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants