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

Added JSON-based procedural code generator to the Playground #15243

Merged
merged 5 commits into from
Jul 9, 2024

Conversation

Tricotou
Copy link
Contributor

@Tricotou Tricotou commented Jul 1, 2024

This PR is following the same topic on the forum

  • source code is the same than CTRL+SPACE menu (tools/playground/public/templates.json)
  • any new code block from this code generator is now as well available from the CTRL+SPACE menu ✔️
  • added a structure json in tools/playground/public/procedural.json which is used to build the drop down menu ✔️
  • the fetch("templates.json").then() has been move to the monaco constuctor since now it's used from different places.
  • NB: the code generator is not "100%" based on the JSON (for now...?) since some conditions have been included outside, such as for example a shadowGenerator check (cannot be enabled on hemi light), etc.
  • NB: In the monaco editor I added a if (model.isDisposed()) check (see line 720 & 727), but I could not find exactly when the error Model is disposed! is triggered. Seems to be a bit random 🤔

Demo : https://github.com/BabylonJS/Babylon.js/assets/167624418/6b95747b-8f10-42ca-955a-46dc873e0f96

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 1, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 1, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 1, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 1, 2024

1 similar comment
@bjsplat
Copy link
Collaborator

bjsplat commented Jul 1, 2024

@sebavan sebavan requested a review from RaananW July 1, 2024 20:33
@sebavan
Copy link
Member

sebavan commented Jul 1, 2024

Lets see if @PatrickRyanMS can tests the code to validate the UI/UX? and if @RaananW can check the code :-)

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 2, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 2, 2024

@Tricotou
Copy link
Contributor Author

Tricotou commented Jul 2, 2024

Catching the JSON fetch + decode, in order to avoid braking the playground in the event of pushing a broken JSON (tested with broken JSON)

@RaananW
Copy link
Member

RaananW commented Jul 3, 2024

Sorry fro taking the time here, was preoccupied with other tasks. I will review this PR today

@RaananW
Copy link
Member

RaananW commented Jul 3, 2024

Code-wise I am perfectly fine with it. I'll wait for @PatrickRyanMS to try this out and give some feedback, when he has the time.
To test it you need to pull the branch and test it locally, as the playground and sandbox are the one thing that doesn't get pushed to the snapshot

Just an opinion - I think that as a feature it is a nice idea, but this needs to be very carefully implemented. Not sure i am 100% fine with the UX here.

@PatrickRyanMS
Copy link
Member

I am out of the office for the US holiday. I will be back on Monday and able to take a look.

Copy link
Contributor

@deltakosh deltakosh left a comment

Choose a reason for hiding this comment

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

Looks AWESOME!
Love it

@deltakosh deltakosh merged commit a00410a into BabylonJS:master Jul 9, 2024
12 checks passed
@deltakosh
Copy link
Contributor

I merged to have it used by people and we can do a new PR if we want improved UX

@Tricotou
Copy link
Contributor Author

we can do a new PR if we want improved UX
Absolutely ! @PatrickRyanMS don't hesitate to give a feedback about design :)

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.

None yet

6 participants