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

instance mode improvements #8

Open
4 tasks
pietroppeter opened this issue Nov 28, 2022 · 3 comments
Open
4 tasks

instance mode improvements #8

pietroppeter opened this issue Nov 28, 2022 · 3 comments

Comments

@pietroppeter
Copy link
Owner

pietroppeter commented Nov 28, 2022

following up from #4 where (experimental) instance mode support was added, there remain some stuff to do:

  • extend the P5Instance object to really contain all fields the real instance allows for (they should be more or less all vars in p5js.nim except the vars that represent constants like TWO_PI)
  • make sure all procedures that require it are under globalAndLocal
  • extend the instance template to not only include setup and draw. This also requires an addition to the P5Instance class for each addition, as each is stored as a closure in the object. All that is in p5sugar.nim should be added. This could be done with a macro but it is simpler via copy & paste.
  • allow varargs untyped to p5Instance to pass computed values from c backend to js backend, see example here https://github.com/HugoGranstrom/PointCloudReconstruction/blob/80d6541a10f8c05289b50f9b142386a62bc970c2/presentation/slides.nim#L131
@pietroppeter
Copy link
Owner Author

I have started playing with instance mode and already found a new issue, symbols from p5 can be used in procs and funcs which are not inside the setup and draw templates (but I think not in "global" scope, see https://github.com/processing/p5.js/wiki/p5.js-overview#why-cant-i-assign-variables-using-p5-functions-and-variables-before-setup). Luckily for these cases there is a workaround (add p5inst. manually!). This could work for other procs we have not wrapped but it will not work for variables we have not wrapped in instance mode, e.g. frameCount.

My current working document for playing with instance mode (that shows the two examples mentioned above) is: https://github.com/pietroppeter/adventofnim/blob/day6-2015-with-p5/2015/day06.nim

@Vindaar
Copy link
Contributor

Vindaar commented Nov 29, 2022

I have started playing with instance mode and already found a new issue, symbols from p5 can be used in procs and funcs which are not inside the setup and draw templates (but I think not in "global" scope, see https://github.com/processing/p5.js/wiki/p5.js-overview#why-cant-i-assign-variables-using-p5-functions-and-variables-before-setup).

Ahh, sorry! This is something I wanted to ask you about and forgot. I did that consciously as I wasn't sure if we should just replace symbols in the complete instance template or per template. It's a trivial change though. Just replace this line:

https://github.com/pietroppeter/p5nim/blob/master/src/p5/p5instance_logic.nim#L175

by

  let s = proc(p5Inst {.inject.}: P5Instance) {.closure.} = replaceCalls(body)

at which point the existing replaceCalls in the templates within instanceImpl are not necessary anymore. I didn't do it initially to avoid having to think about how to handle the identifiers of the templates themselves, as they are fields of the P5Instance itself. However, it should be correct to just not perform replacement of AST that:

  • has a symbol found in the P5Instance type with type Closure
  • is of kind nnkCall

That should be correct I believe unless there is some weird edge case where one might sometimes want to access something of the same name as a template in local scope.

And you're probably aware that the name of the used symbol p5Inst in the template is defined here: https://github.com/pietroppeter/p5nim/blob/master/src/p5/p5instance_logic.nim#L6 and could be changed to something "nicer" if one wants to improve making manual calls. In that case it would need to be changed here https://github.com/pietroppeter/p5nim/blob/master/src/p5/p5instance_logic.nim#L167 and https://github.com/pietroppeter/p5nim/blob/master/src/p5/p5instance_logic.nim#L175 as well.

@pietroppeter
Copy link
Owner Author

Perfect thanks, I can take it from here!

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

No branches or pull requests

2 participants