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

Add missing functions and properties in several classes #466

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

repalash
Copy link

Changes in: OrbitControls, GLTFExporter, GLTFWriter, ObjectLoader, Material, Matrix3, Matrix4, WebGLMultipleRenderTargets

Why

Add missing properties and functions.

What

  • Extend OrbitControls from EventDispatcher like in three.js

  • Put GLTFWriter in GLTFExporter.Utils like in three.js

  • Add functions to GLTFWriter

  • Add name property in GLTFLoaderPlugin

  • Add missing parse functions to GLTFLoader and fix return types of others.

  • Add onBeforeRender in Material

  • Add readonly props isMatrix3 and isMatrix4

  • Add samples to MultisampleRenderTargets

  • Checked the target branch (current goes master, next goes dev)

  • Added myself to contributors table

  • Ready to be merged

@repalash repalash changed the title Add missing functions and properties in Add missing functions and properties in several classes May 25, 2023
Copy link
Contributor

@Methuselah96 Methuselah96 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I did not have time to review the whole thing, if there are parts you would like to get merged in sooner you could consider splitting this into smaller PRs since some of these changes are easier to verify than others.

Comment on lines +90 to +92
static Utils: {
GLTFWriter: typeof GLTFWriter;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I see Utils being set on GLTFExporter but I'm having trouble seeing where GLTFWriter is declared on it. Can you show where that happens?

Copy link
Author

Choose a reason for hiding this comment

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

Oh, my bad, this was a change I was making a PR for three.js, forgot that its not there.
I will remove this change.

@repalash
Copy link
Author

repalash commented Jun 6, 2023

Hi @Methuselah96, No hurry, please take your time.

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