Skip to content

Tilde grab bugfixes#3712

Closed
patriboz wants to merge 3 commits intomasterfrom
tilde-grab-bugfixes
Closed

Tilde grab bugfixes#3712
patriboz wants to merge 3 commits intomasterfrom
tilde-grab-bugfixes

Conversation

@patriboz
Copy link
Contributor

This PR fixes the following issues when editing scenes:

  • grabbed object transform out of sync
  • collisions
  • pivot point (apparent when entering 3rd person mode)

Copy link
Contributor

@jakezira jakezira left a comment

Choose a reason for hiding this comment

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

Need to optimize more.

let position = null,
quaternion = null;

if(_getSession()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Need proper spacing.

const localVector4 = new THREE.Vector3();
const localVector5 = new THREE.Vector3();
const localVector6 = new THREE.Vector3();
const localVector7 = new THREE.Vector3();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use existing variables if possible.

quaternion = null;

if(_getSession()) {
const h = this[hand === 'left' ? 'leftHand' : 'rightHand'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Any difference between localPlayer and this?
I think only local player can grab inventories.

game.js Outdated

if (o.position.distanceTo(localVector) > offset) {
collision = null;
if(downCollision) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is downCollision boolean or map?

} else {
if(localVector.distanceTo(localVector9) < offset && localVector7.y < localVector9.y) localVector5.copy(localVector9);
if(localVector5.y < localVector7.y) localVector5.setY(localVector7.y);
o.position.copy(localVector5);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this formula for? Better to have a comment with explaination.

grab(app, hand = 'left') {
let position = null,
quaternion = null;
if(this instanceof LocalPlayer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Can player call grab when it's not local player?
  • Even though if it's the case, you can check this with different code. Please reference how it did on existing code.

@patriboz patriboz closed this Sep 16, 2022
@patriboz
Copy link
Contributor Author

patriboz commented Sep 16, 2022

Closed this PR and made a new branch & PR: #3714

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