Skip to content

[Character Controller Revamp] Initial Push#713

Open
SketchFoxsky wants to merge 7 commits intoBasisVR:developerfrom
SketchFoxsky:CharacterControllerRevamp
Open

[Character Controller Revamp] Initial Push#713
SketchFoxsky wants to merge 7 commits intoBasisVR:developerfrom
SketchFoxsky:CharacterControllerRevamp

Conversation

@SketchFoxsky
Copy link
Copy Markdown
Contributor

Replaced Unity Character Controller with a Kinematic Rigidbody Character Controller.

It's at a good point for others to start poking around it! I wanna add a few more things to it but it will require me to touch IK stuff and dooly is currently doing that so i'm gonna hold off on it till they're done!

Replaced Unity Character Controller with a Kinematic Rigidbody Character Controller
@SketchFoxsky
Copy link
Copy Markdown
Contributor Author

My IK updates have been merged! I will update this PR with some stuff later this weekend!

Copy link
Copy Markdown
Collaborator

@dooly123 dooly123 left a comment

Choose a reason for hiding this comment

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

needs a bit of cleanup

}

Vector3 up = UpDirection;
Vector3 pos = transform.position;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we can merge this with the below to getpositionandrotation

public class BasisKinematicCharacterController : MonoBehaviour
{
// Capsule Collider
[SerializeField] private float _height = 2f;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lets make all of this stuff public or => single source of truth is better then bits and bobs

/// MAYBE add native swimming!
///
/// </summary>
[RequireComponent(typeof(Rigidbody))]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I hate the required component system, ok to keep just hate it

}
}

public bool isGrounded => _isGrounded;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yer like at this point just expose the bool

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didnt initially expose it cause I didnt want to touch LocalPlayerDriver too much, since some of my other changes got merged im going through and properly integrating it with it!


#region UNITY LIFECYCLE

private void Awake()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dont do this initalize it from the local players bootup else you will create all kinds of race conditions


private void SyncCapsule()
{
if (_capsule == null) return;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i love assumption code if the capsule is going to be there 99% of the time just remove the null

}
}

Vector3 pos = transform.position;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should really check if we can grab the position from a per frame control or by just doing it once

// Phase 1: Cast UP to find ceiling clearance
float maxUpDist = _stepOffset;
GetCapsuleEnds(pos, out Vector3 up1, out Vector3 up2);
int hitCount = Physics.CapsuleCastNonAlloc(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

works great double check we have enough hit buffers

if (other == _capsule) continue;

if (Physics.ComputePenetration(
_capsule, position, transform.rotation,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we are costing so much performance from having to transform.position and transform.rotation all the time

point2 = worldCenter - capsuleUp * halfHeight;
}

private int GetCollisionMask()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why are we not just caching this once?

Removed Legacy Character Controller check as it is not needed.
Changes made previously were not pushed or git stroked out

IK changes from Developer branch broke need to rewire it!

- Exposed more fields
- Properly Cache Collision Masks
- Cache transform data so its not being called a shit ton
IK changes were regressed during a previous merge, These will also be cherry picked later for an update upstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants