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.

Sign up for GitHub

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

Jump to bottom

Docs: Fix the example to actually render a cube #24120

Merged
merged 2 commits into from
May 25, 2022
Merged

Docs: Fix the example to actually render a cube #24120

merged 2 commits into from
May 25, 2022

Conversation

oskar1233
Copy link
Contributor

Description

The example wasn't rendering the promised cube, this PR fixes it.

Live example in JSFiddle works fine so I took the BoxGeometry arguments from there.

@oskar1233
Fix the example to actually render a cube
5e674f3 
Live example in JSFiddle works fine so I took the BoxGeometry arguments from there.
@mrdoob mrdoob changed the title Fix the example to actually render a cube Docs: Fix the example to actually render a cube May 25, 2022
@mrdoob mrdoob added this to the r141 milestone May 25, 2022
@mrdoob mrdoob merged commit c03a6e8 into mrdoob:dev May 25, 2022
@mrdoob
Copy link
Owner

mrdoob commented May 25, 2022

Thanks!

@oskar1233 oskar1233 deleted the patch-1 branch May 25, 2022 06:56
@Mugen87
Copy link
Collaborator

Mugen87 commented May 25, 2022 •
edited
Loading

@oskar1233 In latest three.js versions, BoxGeometry's width, height and depth are already 1. Applying these values explicitly is not something we do usually in order to keep the code compact. And I don't understand how this change fixed your scene? Are you using an old version of three.js?

BTW: The translated versions of Creating-a-scene.html should also be updated.

@oskar1233
Copy link
Contributor Author

@Mugen87 I installed three from npm so I'm probably not on dev branch - should that then be a hotfix and merged to master?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 25, 2022

TBH, I think this PR should be reverted.

@mrdoob
Copy link
Owner

mrdoob commented May 25, 2022

Hmm, I think specifying the size of the box makes the code more understandable.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 25, 2022

Okay, but then I'll update the localized pages, too.

@Mugen87 Mugen87 mentioned this pull request May 25, 2022
Merged
abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
@mrdoob @abernier
Docs: Fix the example to actually render a cube (mrdoob#24120)
4004f18 
* Fix the example to actually render a cube

Live example in JSFiddle works fine so I took the BoxGeometry arguments from there.

* Update Creating-a-scene.html

Co-authored-by: mrdoob <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeviPesin LeviPesin LeviPesin left review comments

@mrdoob mrdoob mrdoob left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r141
Development

Successfully merging this pull request may close these issues.
4 participants
@oskar1233 @mrdoob @Mugen87 @LeviPesin