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

AnimationClip: default constructor parameters #27956

Merged
merged 1 commit into from
Mar 21, 2024
Merged

Conversation

trusktr
Copy link
Contributor

@trusktr trusktr commented Mar 21, 2024

Related issue: N/A

Description

This makes AnimationClip easier to use without providing initial arguments, f.e.

const clip = new AnimationClip();

// ...any time later...
clip.tracks.push(...keyframeTracks)
clip.resetDuration()

The above code will currently throw because it tries to calculate duration based on nothing (tracks array is undefined).

Currently we have to do this,

const clip = new AnimationClip('', -1, []);

or this,

const clip = new AnimationClip('', 0);
clip.tracks = []

to successfully be able to do this:

// ...any time later...
clip.tracks.push(...keyframeTracks)
clip.resetDuration()

Sidenote, this new signature happens to match with the type definition in @types/three.

This contribution is funded by Lume (3D HTML)

This makes AnimationClip easier to use without providing initial tracks, f.e.

```js
const clip = new AnimationClip();

// ...any time later...
clip.tracks.push(...keyframeTracks)
clip.resetDuration()
```
Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
670.8 kB (166.3 kB) 678.5 kB (168.6 kB) +7.68 kB

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
450.8 kB (108.9 kB) 458.8 kB (111.4 kB) +8 kB

@trusktr trusktr marked this pull request as ready for review March 21, 2024 02:22
@trusktr
Copy link
Contributor Author

trusktr commented Mar 21, 2024

Filesize dev Filesize PR Diff
450.8 kB (108.9 kB) 458.8 kB (111.4 kB) +8 kB

What's the +8kb?

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 21, 2024

You are targeting the master branch, not dev. Let me fix this.

@Mugen87 Mugen87 changed the base branch from master to dev March 21, 2024 08:58
@Mugen87 Mugen87 added this to the r163 milestone Mar 21, 2024
@Mugen87 Mugen87 merged commit d769526 into mrdoob:dev Mar 21, 2024
10 checks passed
@trusktr
Copy link
Contributor Author

trusktr commented Mar 21, 2024

You are targeting the master branch, not dev. Let me fix this.

Ah thanks! I see what happened:

  • the "Source" links on threejs.org point to files on master,
  • I clicked the edit Screenshot 2024-03-21 at 11 02 32 AM button and edited in the GitHub UI
  • this resulted in the subsequent pull request base being master by default.

I'll keep this in mind. Should the Source links be set to point to dev?

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 21, 2024

The source links should should point to pages which reflect the latest version of the release so pointing to dev could be confusing, imo.

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