Zoom never sets the zoom, just adds to it?

[Folcon]

I was trying to use zoom! to change the zoom level to 1px = 1m away from the default of 100px = 1m.

However it was really unclear that zoom! is actually adding to the current zoom level instead of setting it.

In this case a doc to that effect or naming it zoom+! may be better?

So the correct operation is actually:

(b/zoom! -99)

not

(b/zoom! 1)

Otherwise this looks like quite a nice library, thanks for creating it!

[plexus]

Hi @Folcon , I'm happy to see you've found this library. You're the first person (besides me) that I know of that has tried running this, so you are likely to run into some rough edges, but we'll do our best to get them resolved.

I think you're right in this case, it would make more sense to have zoom! directly call through and set an absolute zoom level. Adding a helper function zoom-by! or zoom+! that calls zoom! would be useful too. I guess for what I was doing a sort of relative zoom made more sense, but I agree that it's not intuitive, and it's better to have the more primitive call. Do we have a call for getting the zoom level?

Let me know if you want to take a stab at making a PR.

[Folcon]

@plexus, just so I understand how your naming works, what is the * in zoom*! signifying? I thought it was scaling, ie multiplication, but it's addition?

So we're looking for:
zoom! -> sets absolute zoom level
zoom-by! or zoom+! -> Adds to zoom level

Anything else? Just trying to figure out what consistent names would be =)...

[plexus]

I don't remember exactly but I think in this case it might just mean it's a protocol function, which has a corresponding wrapper without the * which is a regular function, see also alter-user-data*!

I would start by adding a zoom method to IProperty, and implement that for camera. We don't have any way to read the zoom level right now I think. Then zoom! can become absolute zoom, and you can implement your own logic for setting a new zoom level based on the existing zoom level.

zoom+! or zoom-by! can then be added as a convenience, it would simply be

(defn zoom-by! [camera amount]
  (zoom! camera (+ (zoom camera) amount))

[Folcon]

Ok, thinking a little about implementation.

zoom is a scalar, but it's also a transform, (Mat22. (Vec2. zoom-level 0) (Vec2. 0 zoom-level)) by the looks of it, I'm thinking:

1. Have zoom return the Mat22 transform instead of the zoom-level scalar.
2. If you feed zoom! a transform or a scalar it correctly sets the absolute zoom value.
3. zoom-by! can work as you suggested.

I'd vaguely like the set and get to be symmetric with the set accepting both a scalar or a Mat22 as a convenience.

I've also noticed there is an existing set-scale! function, which seems to actually be a zoom!, but it's not super clear that it is the zoom function. From that perspective, is it worthwhile doing this or should we extend the set-scale! function to also handle Mat22's?

Assuming you agree it shouldn't take too long to turn around this =)...

[plexus]

That all sounds good. set-scale! is part of the camera namespace. I think it's ok to have that one there, and still have a zoom! in the top level. It's a little redundant but it'll help people find what they're looking for. In that case maybe make set-scale! polymorphic (can take scalar or Mat22), and then zoom! can just be an alias.

[Folcon]

Cool, let me know what you think. Happy to change stuff if that's not what you were expecting =)...

[Folcon]

@plexus Well this is embarrassing, sorry about this, but it looks like I forgot to delete this before merging.

[plexus]

It's not hurting anyone, but feel free to make a pr to take it out. I'll cut a new release today. Thank you for helping make cljboc2d better!

…

On Thu, Mar 16, 2023, 02:27 Folcon ***@***.***> wrote: @plexus <https://github.com/plexus> Well this is embarrassing, sorry about this, but it looks like I forgot to delete this <Folcon@eb8f34e> before merging. — Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAH3VDAM2236W6ZYEDN7O3W4JUABANCNFSM6AAAAAAVGIOERI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Folcon]

As you've not cut a release yet, I figured I could turn around a new PR =)...