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

push() and pop() #252

Open
trych opened this issue Mar 7, 2018 · 13 comments
Open

push() and pop() #252

trych opened this issue Mar 7, 2018 · 13 comments

Comments

@trych
Copy link
Contributor

trych commented Mar 7, 2018

As @ffd8 suggested here:

should we consider adding a push() + pop() that simply mirrors our existing pushMatrix() and popMatrix()? P5js, switched to that style (…) would make the new ability of easily porting code from P5js thanks to the b-less upgrade that much smoother.

I think it's a good idea, but at the same time, I would not want to clutter our reference.

So here is a suggestion: How about we just implement push() and pop() into the code as simple wrappers, but just have pushMatrix() and popMatrix() in the docs and tutorials.

That way we have the both of best worlds: an official "suggested way", uncluttered docs, but code pasted over from p5.js still works.

@b-g
Copy link
Member

b-g commented Mar 7, 2018

+1. Yes to: push() and pop() aliasing to pushMatrix() and popMatrix()

@trych
Copy link
Contributor Author

trych commented Mar 7, 2018

@b-g And then not mentioning it in the docs, just allowing it to work in the background?

@ffd8
Copy link
Member

ffd8 commented Mar 7, 2018

I can imagine either. Hiding and just mirror/aliasing helps compatibility but could lead to confusion if it showed up in any example code. So probably best if listed and simply mentions it's a P5js compatibility for pushMatrix() <- linking if possible to the main documentation reference for it. They will anyways appear alphabetically next to one another so there shouldn't be much confusion.

@b-g
Copy link
Member

b-g commented Mar 7, 2018

Yes ... I would document both and make clear that it is identical.

Speaking of those hacks I would make b.print() an alias to b.println() ... as p5.js dropped println

@trych
Copy link
Contributor Author

trych commented Mar 7, 2018

Speaking of those hacks I would make b.print() an alias to b.println() ... as p5.js dropped println

Here you, @fabianmoronzirfas and @ffd8 voted to keep both as they are. I know the print statement in p5.js is somewhat annoying, but to stay compatible to Processing, we would need both.
Or we can assume that the print statement is used way more often in p5.js than in Processing (I have never used print()) and go ahead and make it an alias.

Opinions?

About the push() / pushMatrix(). I just learned that there is a difference between push and pushMatrix, see point 6 in this list. push() basically calls both pushMatrix() and pushStyle(), pop() calls both popMatrix() and popStyle().

Suggestion: We implement a pushStyle() and a popStyle() function (should be easy to do) and then add a push() function to call both pushMatrix() and pushStyle(). That way we have maximum compatibilty with all packages.

Opinions?

@ffd8
Copy link
Member

ffd8 commented Mar 7, 2018

Uh oh.. haunted by the past!? I still vote for both. Before I wasn't as keen on adopting the P5js capability, but since b-less, my eyes have been opened. Alias or adopting the difference in function are fine by me. I have also never used just print() in Processing.

Re: pop()/push() also adopting the popStyle() etc., – was also going to mention this, but realized we don't have those functions, so ignored it. If it was easy to implement such a function, fantastic.

@b-g
Copy link
Member

b-g commented Mar 7, 2018

+1! (wasn't aware of the small difference of push() and pushMatrix())

Same feeling here ... with b-less I see things a bit different now. Sorry for the back and forth.

@ff6347
Copy link
Member

ff6347 commented Mar 10, 2018

Do we have to align with P5.js?

@trych
Copy link
Contributor Author

trych commented Mar 10, 2018

We decided back at our Skype call in 2016 that it would be good to be align with both Processing and p5.js as much as possible (minus the 3D stuff and other things that don't translate well to InDesign).

And I think since then p5.js has gained even more momentum, so I don't see, why we should ignore it now.

@ffd8
Copy link
Member

ffd8 commented Mar 10, 2018

Back then it was more about style/conventions – but since b-less, it opens up a quicker port of code from P5js than Processing – both really useful, but the var aspect makes a difference and avoiding a common error like 'push() does not exist' is worth having the alias.

@ff6347
Copy link
Member

ff6347 commented Mar 10, 2018

Okay. I get it. I'm not sure if we should try to add something like the style push/pop. Would you guys find this helpful? I always thought of push and pop only in terms of coordinate space.

@trych
Copy link
Contributor Author

trych commented Mar 10, 2018

Why? What's the issue? pushMatrix() and popMatrix() will still exist and continue to work. I could imagine the style commands to be useful at times, just like the matrix commands.

@ff6347
Copy link
Member

ff6347 commented Mar 12, 2018

No issue. In my mind push and pop is only for the coordinate space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants