ScenicFramework / scenic

Core Scenic library
Apache License 2.0
1.99k stars 137 forks source link

Dynamic viewport example #271

Closed axelson closed 2 years ago

axelson commented 2 years ago

Description

Adds a dynamic view port example and allows creating the view port dynamically with the least amount of code changes. A different API may be desired, I didn't think too hard about this particular implementation. I thought about adding the example to the guides but since the guides are currently empty, putting it directly in the view port docs seemed to make the most sense.

Previously the Scenic.Driver.validate/1 function was considered private (because it used @doc false)

Types of changes

Checklist

boydm commented 2 years ago

The :namefield in the Viewport options is causing people trouble as they don't understand that it is purely optional. In fact it is only there as a debugging hint when you look at processes in Observer.

So what everybody does is copy the example options from scenic_new (which gives a name), and then get confused when the dynamic Viewport fails - because they gave two Viewports the same name. This is effectively trying to give two GenServer processes the same name, which would also fail.

So: Please remove the :name option from the example.

The name option also needs to be removed from the scenic_new examples. @bwheatie was talking about putting a PR in for that, if he doesn't get to it, then I'll do it.

crertel commented 2 years ago

@boydm weird idea: could we change the param name to something like process_debug_name or whatever to make it painfully obvious it's just used in that capacity?

BWheatie commented 2 years ago

I like where you are thinking @crertel but I think if we remove it entirely and keep it as optional in the docs with some notes on gotchas when using that field would help.

axelson commented 2 years ago

I've removed the name from the newly created view port in 8c4997aef8f95d9cf92f917428bad665dd175dec

Although it still depends on finding the original view port via Scenic.ViewPort.info and passing in the view port name. I'm not sure how to do that in a more generic way without depending on the view port having a name.