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

Nodes: Rename uniforms() to uniformArray(). #28910

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

cmhhelgeson
Copy link
Contributor

@cmhhelgeson cmhhelgeson commented Jul 18, 2024

Related Issue: #28878
Related PR: #28897

Description

Rename the UniformsNode's uniforms() function to uniformArray(). This better communicates the difference in functionality and expected argument types between the uniform() and uniforms() functions.

Copy link

github-actions bot commented Jul 18, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
684 kB (169.4 kB) 684 kB (169.4 kB) +0 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
460.9 kB (111.2 kB) 460.9 kB (111.2 kB) +0 B

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 18, 2024

Wouldn't be uniformArray() the more correct name? This method is about defining a single array-typed uniform, right?

@sunag
Copy link
Collaborator

sunag commented Jul 18, 2024

Class name should follow the TSL method too like UniformArrayNode

@sunag sunag changed the title Nodes: Rename uniforms() to uniformsArray(). Nodes: Rename uniforms() to uniformArray(). Jul 19, 2024
@cmhhelgeson cmhhelgeson force-pushed the webgpu_uniforms_name_change branch from 0d8d454 to 71f183a Compare July 25, 2024 21:37
@cmhhelgeson
Copy link
Contributor Author

None of the r168 additions after rebase seem to necessitate additional changes to this branch.

@Mugen87 Mugen87 added this to the r168 milestone Jul 26, 2024
@Mugen87 Mugen87 merged commit bfe65cd into mrdoob:dev Jul 26, 2024
12 checks passed
@Mugen87 Mugen87 changed the title Nodes: Rename uniforms() to uniformArray(). Nodes: Rename uniforms() to uniformArray(). Jul 26, 2024
brunosimon pushed a commit to brunosimon/three.js that referenced this pull request Jul 28, 2024
* init branch

* Change class naming to match function
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.

3 participants