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

remove unneeded localPlayer updateds #2646

Closed
wants to merge 4 commits into from

Conversation

0reo
Copy link
Contributor

@0reo 0reo commented Mar 22, 2022

closes #2621

this PR removes the update code which will not be needed once the vrm templates handle that via useFrame. Currently requires webaverse/totum#97 before it can be merged

@0reo 0reo linked an issue Mar 22, 2022 that may be closed by this pull request
@0reo 0reo marked this pull request as ready for review March 23, 2022 21:32
@0reo
Copy link
Contributor Author

0reo commented Mar 23, 2022

ready for review, webaverse/totum#86 which this depends on is also waiting for review

Copy link
Contributor

@avaer avaer left a comment

Choose a reason for hiding this comment

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

One question.


transformControls.update();
game.update(timestamp, timeDiffCapped);

localPlayer.updateAvatar(timestamp, timeDiffCapped);
playersManager.update(timestamp, timeDiffCapped);
Copy link
Contributor

@avaer avaer Mar 24, 2022

Choose a reason for hiding this comment

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

This line is removed and not added in the Totum PR. Where did this go?

@0reo 0reo requested a review from avaer March 24, 2022 21:04
@0reo 0reo force-pushed the 2621-performance-tracker-track-avatar-cpu branch from 874f523 to dbf1c8e Compare March 31, 2022 18:55
@0reo 0reo force-pushed the 2621-performance-tracker-track-avatar-cpu branch from dbf1c8e to 53d02e2 Compare March 31, 2022 19:08
@0reo 0reo requested a review from alisaad673 March 31, 2022 19:10
Copy link
Contributor

@avaer avaer left a comment

Choose a reason for hiding this comment

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

No comments on the previous review, and the code has not changed.

#2646 (comment)

@avaer
Copy link
Contributor

avaer commented Apr 1, 2022

The Totum PR has been merged but this one has not, so they are desynchronized and I suspect puling totum into master at this point would be broken.

@avaer
Copy link
Contributor

avaer commented Apr 1, 2022

Totum PR has been reverted: webaverse/totum#86

So it will need to be submitted and merged synchronously with this one next time.

@0reo 0reo requested a review from avaer April 5, 2022 16:55
@0reo
Copy link
Contributor Author

0reo commented Apr 5, 2022

new totum PR with useFrame was added to this. have not recieved any feedback regarding the review notes, but both this and totum should be ready to merge.

@0reo 0reo force-pushed the 2621-performance-tracker-track-avatar-cpu branch from 7c67767 to 1bac913 Compare April 5, 2022 17:40
@0reo 0reo mentioned this pull request Apr 5, 2022
@0reo
Copy link
Contributor Author

0reo commented Apr 8, 2022

closing this PR as there will be a general change in approach. will be addressed in a new PR

@0reo 0reo closed this Apr 8, 2022
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.

Performance tracker: track avatar CPU
2 participants