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

Adding Example of Wave Function Collapse with 3D Tiles #19395

Closed
wants to merge 1 commit into from
Closed

Adding Example of Wave Function Collapse with 3D Tiles #19395

wants to merge 1 commit into from

Conversation

sketchpunk
Copy link

No description provided.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 19, 2020

Can you please explain in more detail what features of the library this example demonstrates?

@sketchpunk
Copy link
Author

Just an example of Procedural Generation using an algorithm called "Wave Function Collapse" but instead of 2d pixel tiles, it shows how to handle 3D tiles, all in three.js. I asked on twitter @threejs_org about if they would like something like this in the examples and was told yes.

@sketchpunk
Copy link
Author

So I need to generate a screenshot and modify file.js to pass the one failed test?

@WestLangley
Copy link
Collaborator

Can you please explain in more detail what features of the library this example demonstrates?

Just an example of Procedural Generation using an algorithm called "Wave Function Collapse"

@sketchpunk The three.js examples are used to demonstrate specific three.js features. I think your example is best-suited as a "showcase", which you can share on the three.js forum.

@sketchpunk
Copy link
Author

https://twitter.com/threejs_org/status/1261321582678388738?s=20
I dont know who runs threejs_org twitter account. They said they it would be great to have an example and told me to do a pull request.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 19, 2020

If this code is really wanted in the repo, I suggest you refactor the code a bit so it uses a similar code style like the other examples (meaning e.g. no class syntax or no usage of # or $ in variable names). It would be also great if you can remove the 6 lgtm alerts this PR would going to introduce.

@donmccurdy
Copy link
Collaborator

I'd be glad to see an example of this type in the three.js repository, and would be willing to help with simplifying glTF loading to use THREE.GLTFLoader, if @sketchpunk is OK with that. This would streamline the example by -300 LOC or so.

@sketchpunk
Copy link
Author

@donmccurdy Sure

@Mugen87 So no ES6 to ES2020 features, but the Threejs i'm using is the ES Module that uses import and script module block. That stuff is newer then classes. So I dont get why I can't have things written as classes over using prototypes or other ways. Plus, what are lgtm alerts?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 19, 2020

These alerts are listed in the "checks" section:

image

So I dont get why I can't have things written as classes over using prototypes or other ways.

Sorry, but I don't think this is the right time to questioning the established code style. Please use the existing examples as a code template.

@DefinitelyMaybe
Copy link
Contributor

@Mugen87 no, a new example isn't necessarily needed to be confined to the established style. the reason for that is that folks with links to old examples might expect those assumptions but this is not the case for new examples. it's also the case that our intention is to move to the new style.

@donmccurdy
Copy link
Collaborator

donmccurdy commented May 19, 2020

We do expect all code in the three.js repository to follow the three.js styleguide, with a maybe a few exceptions. The styleguide may evolve over time, as we allow specific language features, but the styleguide still applies.

That said, I think ES Classes would be fine, those are already used in our source code?

@DefinitelyMaybe
Copy link
Contributor

Ah, thank you @donmccurdy. I should have said something like please use the linter within the three.js package (it will let you use classes).

Small bump @looeee regarding #19112 as we could've just pointed @sketchpunk in that direction.

appropriate comment for @sketchpunk

@looeee
Copy link
Collaborator

looeee commented May 21, 2020

Small bump @looeee regarding #19112

Yeah, sorry its been a busy few weeks. @mrdoob maybe you could give @DefinitelyMaybe access to edit the wiki?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 21, 2020

Yeah, sorry its been a busy few weeks. @mrdoob maybe you could give @DefinitelyMaybe access to edit the wiki?

There are only two ways of adding permission for this. You can either allow editing the wiki for everybody (see this guide) or by making the user to a collaborator. TBH, I do not vote of doing both in this case. One the one hand, the wiki might be an easier target for Spam, on the other hand users should only "become" collaborators if they have done serious contributions for a project over a certain amount of time.

@DefinitelyMaybe
Copy link
Contributor

hahaha I agree.

@munrocket are you free? I recall talk of DX improvements

@munrocket
Copy link
Contributor

@sketchpunk there was a breaking change in CI and we need .jpg files for new site #19375

If this PR will be merged I can add screenshot by yourself just ping me pls.

@sketchpunk
Copy link
Author

@munrocket So if the PR gets merged, to ping you about generating a screenshot?

Everyone else, So, do I still need to remake this without ES Classes or can I keep the classes but just remove some ES2020 related things like private fields? Plus the LGTM alerts.

@DefinitelyMaybe
Copy link
Contributor

honestly just pass all the tests via npm and LGTM on PR. that's 90% right there. Feedback from review is the last part.

@mrdoob
Copy link
Owner

mrdoob commented May 25, 2020

@sketchpunk I think the main thing to do is to modularize the code in the example a bit. Moving the relevant code to examples/jsm/Wfc.js would clean up things a lot.

@mrdoob mrdoob added this to the r118 milestone May 25, 2020
@mrdoob mrdoob modified the milestones: r118, r119 Jun 24, 2020
@mrdoob mrdoob modified the milestones: r119, r120 Jul 29, 2020
@mrdoob mrdoob modified the milestones: r120, r121 Aug 25, 2020
@mrdoob mrdoob modified the milestones: r121, r122 Sep 29, 2020
@mrdoob mrdoob modified the milestones: r122, r123 Oct 28, 2020
@mrdoob mrdoob modified the milestones: r123, r124 Nov 25, 2020
@mrdoob mrdoob modified the milestones: r124, r125 Dec 24, 2020
@mrdoob mrdoob modified the milestones: r125, r126 Jan 27, 2021
@mrdoob mrdoob modified the milestones: r126, r127 Feb 23, 2021
@mrdoob mrdoob modified the milestones: r127, r128 Mar 30, 2021
@mrdoob mrdoob modified the milestones: r128, r129 Apr 23, 2021
@mrdoob mrdoob modified the milestones: r129, r130 May 27, 2021
@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@mrdoob mrdoob modified the milestones: r131, r132 Jul 28, 2021
@mrdoob mrdoob modified the milestones: r132, r133 Aug 26, 2021
@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 30, 2021

Moving the relevant code to examples/jsm/Wfc.js would clean up things a lot.

Let's reopen the PR when this is actually done.

@Mugen87 Mugen87 closed this Aug 30, 2021
@Mugen87 Mugen87 removed this from the r133 milestone Aug 30, 2021
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.

8 participants