DONE! developer friendly syntax for getParameterDefinitions #875

hrgdavor · 2021-07-06T20:18:08Z

hrgdavor
Jul 6, 2021
Collaborator

The feature is now part of jscad. If you have more ideas or there is a bug please add a new issue.

Here is a presentation of a proposal for a new jscad feature: http://3d.hrg.hr/jscad/three/new_params_proposal/parameters.html#0

It is an additional syntax that can be used instead getParameterDefinitions to simplify writing scripts that use parameters.

The intention is to keep getParameterDefinitions and have also this alternative way of defining parameters

the idea is allow developer to write the source source file like this:

function main({//jscadparams
  // size
  radius = 22,// Radius {type:'slider', live:1}
  // location (initially closed) {initial: 'closed'}
  x=0, // {type:'slider', live:1}
  y=0, // {type:'slider', live:1}
  z=0, // {type:'slider', live:1}
}){
  return translate([x,y,z],sphere({radius, height:x}))
}

and the script does not have to provide getParameterDefinitions

The parser would internally generate this definition based on the source above

function getParameterDefinitions(){
  return [
    {"name":"_group_1","type":"group","caption":"size"},
    {"caption":"Radius","type":"slider","name":"radius","initial":22,"live":1,"min":0,"max":44},
    {"name":"_group_2","type":"group","caption":"location (initially closed)","initial":"closed"},
    {"caption":"x","type":"slider","name":"x","initial":0,"live":1,"min":0,"max":100},
    {"caption":"y","type":"slider","name":"y","initial":0,"live":1,"min":0,"max":100},
    {"caption":"z","type":"slider","name":"z","initial":0,"live":1,"min":0,"max":100},
  ]
}

and the form would be something like this:

@z3dev please check it out fi you wish to discuss or propose changes. I would like a green light that this is something that would be accepted into jscad so I can start coding the PR

also, this demo uses a new worker implementation (with regl-renderer) that renders to offscreen canvas transferControlToOffscreen so both renderer and script execution are in the worker.

Also, it is intention that this demo is somehow later integrated into openjscad ... and expanded as tutorial for scripts with parameters.

z3dev · 2021-07-08T18:15:10Z

z3dev
Jul 8, 2021
Maintainer

@hrddavor thanks for the new idea.

i can see advantage of having a simple way to describe the parameters, but parsing code is probably the wrong direction. Even simple parsing soon becomes complex. And there's all kinds of hooks that will have to be implemented as well.

Remember, JSCAD is about the 'core' pieces. We want to create the pieces for others to assemble into different applications, libraries, etc.

having said that, there may be other options.

defining a global array of "parameters'?
looking for a specific module by name?
etc

By the way, the OpenSCAD parser does something similar, parsing the code. And it was never completed, tested, etc. And unsupported at this time.

3 replies

hrgdavor Jul 8, 2021
Collaborator Author

Well, it is not just a new idea....

The code is alredy made, there is even an interactive demo.
Did you try the demo and try adding a param in a script in there ?
Actually I was waiting to propose this until I finish the interactive demo.
That interactive demo is intended to be improved to later become an interactive tutorial about scripting with parameters.

It is something that I am using for few months now and is much more fun than getParameterDefinitions.
But it does not replace getParameterDefinitions they can even work together.

It already has unit tests in a separate repo.

I am not seing what hooks you would have in mind.

Parser is pretty straight forward with rules.

first part of the comment is caption
rest is JS object (anything eval can chew)
It is nothing extra strange that is not already being done when executing the script in code-evaluation package.
the code-evaluation package has access to script source.

I have even created a viewer

that is the basis for the new jscad web I am working on
that is used in the demo
that is also very easy to use

const canvas = document.getElementById("canvas")
jscadRenderWorker.init({canvas})
jscadRenderWorker.runScript({script:`module.exports = {main:()=>[]}`, url:'clear_script', params:{}})

and that is it, you can run a jscad model on your website.
and as bonus:

I have implemented that worker so both render and code-evaluation run in the worker
worker is not destroyed when parameters change (many performance improvements for future now possible)
it uses regl-renderer

check page #7 of the demo and how it changes sphere size smoothly.

Yes jscad is about core pieces and this would be integrated into the code-evaluation package.

I switched from OpenSCAD to here because OpenSCAD was developing very slowly.

hrgdavor Jul 9, 2021
Collaborator Author

@z3dev sorry if I seem agressive, but I am really passionate about this feature, and I would much like to have it integrated in near future.
You have mentioned you are on sabatical until sept 1. so I understand if you will not have time to review in depth right away.

I have my own personal setup that I use instead of openjscad.xyz and, I could keep it for myself, but I am eager to integrate those things I deem as improvement to performance and user experience back to openjscad.

I am asking here just for green light, that you would be willing to accept a PR that implements this into openjscad later on.
but not that you would just accept anything. I expect that with your input I would integrate it properly with tests and improvements based on your suggestions.

I am pretty confident we I can make this much better than OpenSCAD folks did for their parameters.

Please don't feel pressured to respond quickly to my long comments, I am working on number of things jscad related, even learning webgl2, so I will continue playing with those other things in the meantime.

z3dev Jul 10, 2021
Maintainer

@hrgdavor please continue. Your enhancements have been excellent, so I’m not worried.

Like before, let’s continue to discuss and make incremental changes.

I’m trying to review some of the other pull requests now, but not so easy at my current location as the PC here is a little out of date.

SimonClark · 2021-07-10T11:25:26Z

SimonClark
Jul 10, 2021
Collaborator

Sorry to jump in when I've been so detached lately.

My initial reaction to this is to bristle at comments being parsed into code. In my head, code comments should be non-functional, non-impactful. That said, I'm trying to be open minded and hip :p

ok, so what I like:

It's optional, and backward compatible. No existing models will break (unless they accidentally implemented something that will get parsed by this.).
it puts the param names and default values front and center. You don't have to go hunting in a different method for that.

My concerns:

parsing of comments to code is "magical" - potentially harms debuggability. Something special happens somewhere, but how do I dive into the code to find it?
We lose the IDE hinting on proper structure of the params object. If I make a syntactical error in the comments, I get no advanced warning. Runtiime errors had better be more helpful to compensate. Or provide an IDE plugin to help author these.
The current reference implementation breaks with the jscad way of passing in a single params object. I understand it can be defined either way by any given author, though.

Possible enhancements:

ok, so I admit this is a weird mix of annotation type syntax and markup. Bear with me...

function main(params = {
  // @jscad-magic-params
  // # size
  radius = 22, // Radius {type:'slider', live:1}
  // # location (initially closed) {initial: 'closed'}
  x = 0, // {type:'slider', live:1}
  y = 0, // {type:'slider', live:1}
  z = 0, // {type:'slider', live:1}
}) {
  return translate([params.x, params.y, params.z], sphere({radius: params.radius, height: params.x}))
}

2 replies

hrgdavor Jul 10, 2021
Collaborator Author

Hi @SimonClark , no problem. Glad to have you in this discussion.

parsing of comments to code is "magical" - potentially harms debuggability. Something special happens somewhere, but how do I dive into the code to find it?

Yes this is a tradeof I would say. But code there should not be complex it is just defining an object with feww key:value.
I if I just report a nice error with correct line number and parameter name, it should not be hard to find the error.
I was even looking should it be strict JSON, but I decided to eval the expression as it is cleaner simpler JS syntax

We lose the IDE hinting on proper structure of the params object. If I make a syntactical error in the comments, I get no advanced warning. Runtiime errors had better be more helpful to compensate. Or provide an IDE plugin to help author these.

Yes another tradeof, but not for me as I use sublime text and livereload. So I can see the error pretty fast. And I am working on number of improvements to get many scripts near realtime execution.

The current reference implementation breaks with the jscad way of passing in a single params object. I understand it can be defined either way by any given author, though.

have you noticed select box that is explained on page#4 , for 3 different styles supported on and can be used in the demo

and

Also as a bonus I already have code implemented that can use getParameterDefinitions output and generate this new syntax

Since I like my new fancy syntax so much, I use it constantly and then use that same demo to produce script that is compatible with current openjscad.xyz 😄 when I want to share it.

hrgdavor Jul 10, 2021
Collaborator Author

@SimonClark also, try to use top editor of the demo to write a simple jscad script. Code from editor is evaluated as you type. It is not 100% ready for suc use but maybe will help yo get the feel how faster execution will be to use

hrgdavor · 2021-07-10T16:58:38Z

hrgdavor
Jul 10, 2021
Collaborator Author

@z3dev @SimonClark

I like the direction for the marker that @SimonClark mentioned // @jscad-magic-params

I like the use of @ as it signifies that there is something special going on in that part of the code. I am honoured that you would add word magic to it 😄 and would be fine with that if that is really the version you want.

There is plenty of time to decide for such detail but would like to start the brainstorm with proposing to go with one of these

// @jscad-params
// @parse-params
// @params

0 replies

hrgdavor · 2021-07-24T14:22:26Z

hrgdavor
Jul 24, 2021
Collaborator Author

@z3dev add a pull request for this #891 please review when u got time :)

0 replies

z3dev · 2021-09-16T06:27:30Z

z3dev
Sep 16, 2021
Maintainer

OK. I spent some time looking over the parameter definitions via comments. It's kind of cool. But let's get the syntax done before @hrgdavor does the final coding.

I took the All Parameter Types Demo as the starting place, and converted to what we are discussing.

const main = ({
  // @jscad-params
  // # Group 1: Text Entry
  text = '',               // Plain Text: { type: 'text', size: 20, maxLength: 20, placeholder: '20 characters' }
  integer = 20,            // Integer: { type: 'int', min: 1, max: 100, step: 1 }
  number = 2.0,            // Number: { type: 'number', min: 1.0, max: 10.0, step: 0.1 }
  date = '2020-01-01',     // Date: { type: 'date', min: '2020-01-01', max: '2030-12-31', placeholder: 'YYYY-MM-DD' }
  email = '',              // Email: { type: 'email' }
  url = 'www.example.com', // Url: { type: 'url', size: 40, maxLength: 40, placeholder: '40 characters' }
  password = '',           // Password: { type: 'password' }

  // # Group 2: Interactive Controls
  checkbox = 20, // Checkbox: { type: 'checkbox', checked: true }
  color,         // { type: 'color', initial: '#FFB431' }
  slider = 3,    // Slider: { type: 'slider', min: 1, max: 10, step: 1 }
  choice1 = 2,   // Dropdown Menu: { type: 'choice', values: [0, 1, 2, 3], captions: ['No', 'Yes', 'Maybe', 'So so'] }
  choice3,       // { type: 'choice', values: ['No', 'Yes', 'Maybe', 'So so'], initial: 'No' }
  choice2 = 5,   // Radio Buttons: { type: 'radio', values: [3, 4, 5, 6], captions: ['three','four','five','six'] }

  // # Group 3: Initially Closed Group { initial: closed }
  checkbox2 // Optional Checkbox { type: 'checkbox', checked: true, initial: '20' }
}) => {
  // add design here...
}

Notes on making definitions:

parameter definitions must be specified as a comment on the same line as the variable definition
group definitions must be specified on separate lines
names can be provided as part of the parameter definition, or taken from the variable name
initial values can be provided as part of the parameter definition, or taken from the variable value

General comments:

Initial values are required for HTML forms, so initial values must be provided by variable definition OR by parameter definition
cannot reuse definitions as these are comments. values, captions, etc can be reused in getParameterDefintion()

Some discussion points:

parsing each line of every file is very time consuming. adding @jscad-params at both start and end could be a solution
syntax of definitions may be confusing to novice users. but in general, not bad
why is syntax for group different? maybe better to add { type: group, initial: closed }
cannot intersperse general comments as those will be considered definitions. maybe adding '@' prefix for each definition

@hrgdavor @SimonClark All thoughts are welcome.

13 replies

z3dev Sep 16, 2021
Maintainer

@hrgdavor No problem. This looks like a nice feature.

would be good to get some more thoughts from @SimonClark and others

hrgdavor Sep 16, 2021
Collaborator Author

@z3dev here is updated parser at: https://3d.hrg.hr/jscad/three/new_params_proposal/parameters.html#9

it does more than just ignoring the extra comments, but adds them as hint to parameter definition

const jscad = require('@jscad/modeling')
const { sphere } = jscad.primitives
const { translate } = jscad.transforms

function main({//@jscad-params
  // size
  // extra comment lines count as hint for the group 
  // defined by the first line
  radius = 22,// Radius {type:'slider', live:1}

  // location (initially closed) {initial: 'closed'}
  x=0, // {type:'slider', live:1}
    // if comment indented is further than the end of the variable name
      // it counts hint for the parameter and not a new group

  y=0, // {type:'slider', live:1}
  z=0, // {type:'slider', live:1}
}){
  return translate([x,y,z],sphere({radius, height:x}))
}

module.exports = {main}

the generated parameter definitions can then be used by customizer in a nice way ... a quick demo is already integrated, but could be made nicer

check it out and give some feedback, but I am hoping to go forward soon with the PR: #891 the tests are added for this new functionality, but there is small bug that cuts group name if it has : in the title

z3dev Sep 17, 2021
Maintainer

@hrgdavor this looks pretty good now. I’m not crazy about indenting comments but alternatives are not great.

let me know when you squash that last bug, and I will do a review of the code. I want to review pretty code. ;)

hrgdavor Sep 17, 2021
Collaborator Author

@z3dev it is ready for review ...

the bug I mentioned was a feature, not a bug

      const idx = caption.lastIndexOf(':')
      if (idx !== -1) {
        name = caption.substring(idx + 1).trim()
        caption = caption.substring(0, idx).trim()
      }

it was meant to allow to set name for group

// groupName: Group caption

It confused me, and I wrote the feature and forgot about it, I concluded it is not worth having as it confused me, and would confuse users.

the name for the group can be set anyway like any other attribute for parameters, so this name:caption feature is not needed

//  Group caption {name:'groupName'}

hrgdavor Sep 17, 2021
Collaborator Author

Added a change to only parse these comments in top-level script

z3dev · 2021-10-03T12:11:50Z

z3dev
Oct 3, 2021
Maintainer

@hrgdavor shall we close this discussion? If there’s more to discuss then it will probably be some enhancements.

2 replies

hrgdavor Oct 3, 2021
Collaborator Author

Yes. We can close it 👍

z3dev Oct 3, 2021
Maintainer

Wow! We have neve closed a discussion before… what to do? How about locking this discussion after adding a comment?

hrgdavor · 2021-10-03T12:33:51Z

hrgdavor
Oct 3, 2021
Collaborator Author

My thought exactly, I see no other suitable. option.

0 replies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DONE! developer friendly syntax for getParameterDefinitions #875

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Replies: 7 comments 20 replies

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

DONE! developer friendly syntax for getParameterDefinitions #875

hrgdavor Jul 6, 2021 Collaborator

Replies: 7 comments · 20 replies

z3dev Jul 8, 2021 Maintainer

hrgdavor Jul 8, 2021 Collaborator Author

hrgdavor Jul 9, 2021 Collaborator Author

z3dev Jul 10, 2021 Maintainer

SimonClark Jul 10, 2021 Collaborator

ok, so what I like:

My concerns:

Possible enhancements:

hrgdavor Jul 10, 2021 Collaborator Author

hrgdavor Jul 10, 2021 Collaborator Author

hrgdavor Jul 10, 2021 Collaborator Author

hrgdavor Jul 24, 2021 Collaborator Author

z3dev Sep 16, 2021 Maintainer

z3dev Sep 16, 2021 Maintainer

hrgdavor Sep 16, 2021 Collaborator Author

z3dev Sep 17, 2021 Maintainer

hrgdavor Sep 17, 2021 Collaborator Author

hrgdavor Sep 17, 2021 Collaborator Author

z3dev Oct 3, 2021 Maintainer

hrgdavor Oct 3, 2021 Collaborator Author

z3dev Oct 3, 2021 Maintainer

hrgdavor Oct 3, 2021 Collaborator Author

hrgdavor
Jul 6, 2021
Collaborator

Replies: 7 comments 20 replies

z3dev
Jul 8, 2021
Maintainer

hrgdavor Jul 8, 2021
Collaborator Author

hrgdavor Jul 9, 2021
Collaborator Author

z3dev Jul 10, 2021
Maintainer

SimonClark
Jul 10, 2021
Collaborator

hrgdavor Jul 10, 2021
Collaborator Author

hrgdavor Jul 10, 2021
Collaborator Author

hrgdavor
Jul 10, 2021
Collaborator Author

hrgdavor
Jul 24, 2021
Collaborator Author

z3dev
Sep 16, 2021
Maintainer

z3dev Sep 16, 2021
Maintainer

hrgdavor Sep 16, 2021
Collaborator Author

z3dev Sep 17, 2021
Maintainer

hrgdavor Sep 17, 2021
Collaborator Author

hrgdavor Sep 17, 2021
Collaborator Author

z3dev
Oct 3, 2021
Maintainer

hrgdavor Oct 3, 2021
Collaborator Author

z3dev Oct 3, 2021
Maintainer

hrgdavor
Oct 3, 2021
Collaborator Author