search

GDevelopApp / GDevelop-extensions

Repository of behaviors, actions, conditions and expressions to be used in GDevelop for creating games
https://gdevelop.io
MIT License
131 stars 52 forks source link

Change camera zoom with mouse wheel or keys #200

Closed Elairyx7301 closed 2 years ago

Elairyx7301 commented 3 years ago

 

This rest of the original description and the files are outdated.

The final and merged version is v3.0.6. See the files for it here.

Files v3.0.6

Project.zip CameraZoom.zip

     

     

Quick overview

Change the zoom scale of a camera with the mouse wheel / keyboard keys, contains camera zoom as a regular action and one for limiting camera zoom scale.

Describe the extension

With this extension, the user can change the zoom scale of any camera on any layer using the mouse wheel or custom keyboard keys and the step of zooming can also be changed. The extension does not feature expressions or conditions since it's only manipulating the regular camera zoom and there already is a list of expressions and conditions for the current camera state including its zoom scale.

In combination with the "Drag camera with pointer" extension, this can make for a sweet RPG map layout, a searching game of any kind, a platformer where you have to move the camera manually while being under pressure (that's a fun idea, by the way ;) and anything the like.

Important parameters like "zoom step", "camera" and "layer" are dynamic and hence can be changed with the expression builder as well.

Extension Actions (v1.0.6)

Checklist

List of all actions (v1.0.6)

actions

Image of events used in the game file (v1.0.6)

events

Example video (v1.0.5)

This is a video showing the extension. The video was made with the extension project example linked below.

https://user-images.githubusercontent.com/83909121/133308174-5251d61f-31b3-4fb1-a87a-7ea472502314.mp4

Extension file (v1.0.6)

outdated

Bouh commented 3 years ago

A perfect extension for RTS games!

tristanbob commented 3 years ago

I’ll review this when I get back from vacation in a week and see if my RTS-like Camera Controls can be combined and merged into single extension.

On Sat, Aug 7, 2021 at 4:16 PM Aurélien Vivet @.***> wrote:

A perfect extension for RTS games!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GDevelopApp/GDevelop-extensions/issues/200#issuecomment-894712541, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDX5Q6UPI7NPXUK72U2G6DT3WWCZANCNFSM5BXQL5BA .

-- Tristan Rhodes

tristanbob commented 3 years ago

Don't hold this up for me to review. I have way too many projects in progress for me to commit to a fast resolution on this. Having multiple camera-related extensions not a problem, especially once we get a marketplace that includes submitting reviews.

Bouh commented 3 years ago

Instead of once action for zoom in and zoom out with mouse wheel condition integrated I recommend splitting this action in two and take off the condition.

This way we have:

We should use like:

image

tristanbob commented 3 years ago

Funny that I got myself to review this after my comment. :)

Overall, this is great, and very much like what my extension does. I have simplified your logic (a little) by:

Screenshot 2021-09-11 204907 Screenshot 2021-09-11 204725

Extension file

CameraZoom_Extension.zip

tristanbob commented 3 years ago
  • The conditions are completely hidden inside the extension. If we want to use a variable value for checking if we should zoom or unzoom we can't.

What do you mean by this? Are you thinking about something that could be fixed with "MaxZoom" and "MinZoom" parameters?
(I recommend adding those parameters, and change the "max" function to a "clamp" function.)

You added two actions, one for handle the mouse wheel, second for handle key inputs. I strongly recommend taking off the condition from the action. This way the second action with key can be deleted. The first action can be splited in zoom in and zoom out.

I don't understand this either (sorry!)

Instead of once action for zoom in and zoom out with mouse wheel condition integrated I recommend splitting this action in two and take off the condition.

I actually like this as one function, but I don't feel strongly about it. My only change would be to make it do the repeat by default. You could do this by changing the "Repeat" parameter to "Disable long press" (or some better wording) to reverse the logic.

This way we have:

  • Zoom In, action, parameters: step, camera, layer
  • Zoom Out, action, parameters: step, camera, layer

We should use like:

image

That change takes away some of the "it just works" functionality of the extension (but it does add flexibility). Perhaps another idea is to create new functions that can enable this. There would be a total of 4 functions:

  • In function parameter you wrote __camZoom_mainZoomStep this is the format for variable, for parameter the practice guide recommend PascalCase, so in your case ZoomStep is enought for this parameter. __camZoom_mainCamID => CameraID __camZoom_mainLayerID => LayerName Follow the same model for other parameters.

Yes, and I prefer using __camZoom.mainZoomStep format to make the debugger easier to read (it groups the child variables)

  • (optionnal) We add a parameter for zoom on all layers in zoom in and zoom out actions.

Is there a way to iterate through all layers?

I have a helper function that copies all camera values (cameraX, cameraY, cameraZoom, cameraAngle) from one layer to another, but that probably doesn't make sense inside this extension.

  • (optionnal) A condition can check if we're currently in a zoom in or out. It require a internal variable so here use the previous format you wrongly used for parameters, your extension is CameraZoom so the variable must be __CameraZoom_.IsZoomIn, __CameraZoom_.IsZoomOut, __CameraZoom_.IsZooming

I'm not sure what this does? The zoom change is instant.

  • Details in extension description what content is added, which conditions, actions.

Yes, please!

  • Refine short description.

And spell check ("keybaord")

@ElairyxOfficial Did I say that this extension is great? Well, it is! I am looking forward to seeing it in the official list.

Bouh commented 3 years ago

Don't take into account my feedback above, I did it.

Example_CameraZoom_extension_1.0.2.zip

Version 1.0.2

Still todo, I let you add these:

What mean CST into ZoomCstKeyboard? A simpler name is possible?

Elairyx7301 commented 3 years ago

Thank you @Bouh for making changes yourself but for me it's twice as complicated to merge now two unfinished projects and having to compare what I've already changed and what is still to do. Furthermore, your link returns a "Not Found" for me so I unfortunately couldn't use it either way.

Regular zoom actions

I now added "Zoom in" and "Zoom out" actions (cam, layer, step) so it can be used more dynamically but left the "allow zooming with mouse wheel/keyboard key" in. As Tristan said, I like the idea of "setup once and done". Doing that manually would cost at least 2 events but I agree that those regular actions were missing.

Thank you @tristanbob for optimizing the events. The "Trigger Once" inside the logic structures looks rather strange to me but I'm impressed and happy that works. I've now implemented max(x,y) and min(x,y) functions I admittedly forgot about. I have not used the clamp(x,y,z) function because I don't quite know how it works. If it matters much I could change it again.

"Allow zoom" changes

I've added a "invert" option to the "allow zooming with mouse wheel" action and to be honest, I didn't think it would be that important - comparing the default with the inverted, I can't say which one I like better or feels more intuitive. The "allow zooming with keyboard keys" action now has a "trigger once" option. I thought of using "trigger once" because users are used to that term, "zoom once" is not clear enough and a longer description felt too long or unnecessarily long. Both options of those actions have a description to help pick the best option.

Separate "limit" action

Adding another two parameters for minimum and maximum for all four actions would make the description even less clear and the usability worse, in my opinion. Therefore, I added another action to limit the zoom between minimum and maximum. The one action can now be triggered whenever needed or changed independently from the other zooming actions. Furthermore, the "limit camera zoom" can be used without other actions. If other game mechanics etc. zoom the camera, all other actions would not be able to force the zoom into limits like this one. Also, a separate action makes the events and actions more clear.

"Currently zooming" and "All layers"

I see no reason to add a "Currently zooming" condition because the zoom changes are made instantly, hence there is not real time span where the "Currently zooming" condition would return true. Apart from that the extension is only manipulating existing camera zoom and the user can always use the big list of conditions, actions and expressions for camera and camera zoom that is already available, any event/action that requires the camera to just be zooming has the exact same condition as the action that makes the camera zoom in the first place so said action perfectly fits that event.

I have not seen a "iterate through all layers" or "for all layers" option, action or parameter anywhere in the engine yet. I know the "Layer" parameter gives the user a list of all existing layers but I have not found a way yet to access them and find a event type/action to apply zoom changes to all layers. (Also, I really don't think "all layers" can just be an option but has to be a separate action because it would make the layer parameter optional and optional parameter values are not accepted)

If you know a way to easily get all or specific layers then you are free to add it :)

Just a personal note: I don't think I always want to zoom all layers (excl.: UI, menu, etc.) so an "all layers" action almost wants to have a "some layers" actions as well to be complete - if you know what I mean. This, however, seems like a little too much and might be solved more efficiently by copy/pasting the zoom action and change the layers manually.

Version 1.0.2 changes by Bouh

I've once been told that updating the version number is only needed if the project is published (again) and since it's not published yet, I still am submitting version 1.0.0 but I don'T want to scream about it if you want me to change it. Also, what happened to v1.0.1?

I have unified the position of the "step" parameter but I did intentionally not put it as the first in the list because it has a "long description" that I believe is rather helpful for users and putting this parameter in first position will automatically set the field to active and an empty / not filled value (e.g. when creating action) will show a warning blocking the hint.

Other fixes and changes

Also, Tristan - I'm pretty sure you spell "keybaord" like "keyboard" but I checked all uses of "keyboard" I could find. Changed phrasing of the short description, added "extension content" etc. to regular description, optimized function and variable names, changed "layerID" to "layerName". The extension name between underscores is supposed to be a prefix for variable names. I have now changed camZoom_Xyz to the accidentally not taken actual extension name which is __CameraZoom_Xyz. Moreover, I've changed the variable names from CameraZoom_Xyz to _CameraZoom.Xyz so it can be viewed better in the debugger.

Post update

Original post has been updated with the new files.

Bouh commented 3 years ago

your link returns a "Not Found" for me so I unfortunately couldn't use it either way.

Sorry my previous link is now working, and I put it here again: Example_CameraZoom_extension_1.0.2.zip

I've once been told that updating the version number is only needed if the project is published (again) and since it's not published yet, I still am submitting version 1.0.0 but I don'T want to scream about it if you want me to change it. Also, what happened to v1.0.1?

1.0.1 is the version from @tristanbob, he make edit, you did seen what he did? I took the changes he made (1.0.1), and made my own afterwards, so my version is 1.0.2.

The extension name between underscores is supposed to be a prefix for variable names. I have now changed camZoom_Xyz to the accidentally not taken actual extension name which is __CameraZoom_Xyz. Moreover, I've changed the variable names from CameraZoom_Xyz to _CameraZoom.Xyz so it can be viewed better in the debugger.

You misunderstood parameters and variables are differents, check how i've done in my version.

I admit forget isZooming, it was a bad idea.

I have not seen a "iterate through all layers" or "for all layers" option, action or parameter anywhere in the engine yet. I know the "Layer" parameter gives the user a list of all existing layers but I have not found a way yet to access them and find a event type/action to apply zoom changes to all layers. (Also, I really don't think "all layers" can just be an option but has to be a separate action because it would make the layer parameter optional and optional parameter values are not accepted)

I've done this part, check my version 1.0.2.

Elairyx7301 commented 3 years ago

Merging those two projects is an act I won't be able to start today and school begins for me so I can't say when I will have the time to. I will get to it eventually when motivation is better, sorry. I'll let you know when it's done.

Bouh commented 3 years ago

No worries, take your time, prefer to prioritize the school is fine 👍

Elairyx7301 commented 3 years ago

Well, motivation is back and school was way calmer than I thought it'd be. The changes are done, the original post is updated. I managed to add a set max/min zoom for all layers action and tried to implement the allow zooming with keyboard/mouse wheel on all layers but I couldn't get it to work so I would be really happy if you would be able to take that, @Bouh because you're more experienced in JS and how to properly use it in GD5

- Changed descriptions, order of parameters to be as consistent as possible in all actions and parameter names to Xyz and scene variables to __CameraZooom_.Xyz, version is now v1.0.3

Bouh commented 3 years ago

Thank you for this fast merging!

We have a problem with "Limit camera zoom" action, in your example you added zoom in/out with mouse clicks. As you can see in your example the camera have a wierd behavior because the limit camera zoom is called before the zoom with click. This Limited zoom can be integrated to the zoom actions. For me "Limit camera zoom" is too much and forward an issue.

For fixing that I'll add two parameters min and max in all other actions. As the default value is 0 even if a value is missing, if we have 0 for min and max then we don't limit the zoom. I can do the changes, just need your approval for deleting the "Limit action", if you wanna keep it can you explain what is your mindset?

action and tried to implement the allow zooming with keyboard/mouse wheel on all layers but I couldn't get it to work so I would be really happy if you would be able to take that,

I'll do it no problem.

Elairyx7301 commented 3 years ago

Thank you for the fast reply. Sure, feel free delete the action. My only reason for making it in the first place was because I thought adding it would be a workaround for adding 2 additional parameters (over 12 in total) making the event sentence even less clear - but form follows function and that issue is a solid reason against it so I, of course, have no problem with it :) Thank you for your help ^^

tristanbob commented 3 years ago

Great job you two!

I agree, adding the max and min as parameters to each function is the way to go.

That said, my RTSCameraControls has an "Enforce camera boundaries" function that moves the X/Y of the camera and calculates the min zoom allowed to stay within the boundaries. That function must be called after all other camera actions are run, which sounds similar to what @ElairyxOfficial was doing with "Limit camera zoom".

BTW, after this is complete I will likely remove the zoom functionality from RTSCameraControls because this extension does it so nicely.

Elairyx7301 commented 3 years ago

Big thank you to both of you for your feedback, support and patience <3

Bouh commented 3 years ago

example_CameraZoom1.0.4.zip

1.0.4

Please test it, I'll be able to upload the example with this extension this week if we're all ok.

Elairyx7301 commented 3 years ago

1.0.5

Hey @Bouh, everything is tested, v1.0.5 is ready to go.

Game files: example_CameraZoom1.0.5.zip

Extension file: Extension.zip

tristanbob commented 3 years ago

Great updates! I have a few suggestions:

Elairyx7301 commented 3 years ago

Thank you @Bouh for updating the Best Practices wiki page. The scene variable names were always at end of sentences so it wasn't clear if the dot is in the prefix or not was the reason I didn't use it in the first place / the first time at all. I don't know if it's just personal preference to leave out the last underscore or if it should be added as a note to the page as well (__Extension_.Xxx or __Extension.Xxx - if you put value on it of course).

v1.0.6

@tristanbob I've made the changes you recommended but I don't know what you mean with this, I'm sorry. Could you explain a bit differently, please?

Currently, you perform your actions twice (once without min/max limits, and one with the limits) Why not try this: Change camera zoom (always). If min/max values are set, then enforce limits

One thing missing

I have no idea how to get the TimeDelta() in JS so I couldn't multiply the ZoomStep value/argument with it. CameraSetZoomAllLayers obviously does not need it so the only JS block that is missing a TimeDelta() is CameraZoomAllLayers.

Files

Extension.json file (v1.0.6) Extension.zip

Project example (v1.0.6) A full extension example project with sprites to see the zooming in action (.zip) example_CameraZoom1.0.6.zip

tristanbob commented 3 years ago

Nice work! I intend to look at this tonight. I also want to see if I can incorporate my lerp() logic which really makes the zoom smooth as butter.

arthuro555 commented 3 years ago

I don't know if it's just personal preference to leave out the last underscore or if it should be added as a note to the page as well (_Extension.Xxx or __Extension.Xxx - if you put value on it of course).

IIRC I was the one who started naming my variables like that first to avoid conflicting with variables of the user, and when I did it I was originally naming them like this: __ExtensionName_VariableName. The extension name and underscores were just prefixes to avoid conflicts. When others started adopting it though, they suggested using it as a structure instead to not bloat the debugger with millions of variable since a structure is collapsed by default. Since my naming method was already used as a best practices, we just added a dot in the middle.

That's the history of how it got there. Though, it is indeed not very effective/readable and feel clunky. Best practices are supposed to be the best, and if they are not optimal they should be changed.

Elairyx7301 commented 3 years ago

@tristanbob Sounds cool! I think it would require another boolean parameter because I can imagine some users prefer instant and "sudden" change or for some applications a rapid change is better than a smooth change. I'm thinking of a hard/sudden change for actions triggered by the actual user and a smooth change for actions/events triggered by the game/program like inactivity for 20 seconds -> smoothly zoom out to zoom scale 0.8, zoom in when in small tunnel or zoom out to see whole boss arena etc. but still have the rapid change as an option. I think it would need experimentation how it feels because I currently always worked with zoom step values between 0.01 and 0.1 and they look smooth but not "animated"/with easing of course.

Bouh commented 3 years ago

I reading what you said, I trust you guys, you do a great job! Let's try to finish the features we have for a public release this weekend. Tag me when needed!

tristanbob commented 3 years ago

I ran out of time and energy last night, but I made some good progress. I almost don't want to share this because I am not done and will finish it tonight, but at least you can see what I'm working on.

Done

To Do

Version 1.0.7

example_CameraZoom1.0.7.zip

tristanbob commented 3 years ago

I apologize for the delay. I planned on this only taking 1 hour, but it took over 10 hours because I was learning JavaScript as I went. I'm ready to let someone else finish this up. Thanks for the teamwork!

Version 1.0.8

I made some fairly big changes that you can choose to keep or rollback:

To do:

Project folder

example_CameraZoom1.0.8.zip

Game demo

https://games.gdevelop-app.com/game-8a43af31-39e2-4d7e-8bcd-2ed6d773164d/index.html

Video

https://user-images.githubusercontent.com/8879811/133915561-d7327289-488a-4fed-8bd3-137ac90dce2a.mp4

Elairyx7301 commented 3 years ago

1.0.9

It's very late, I've made some changes but I'm not done and I will change and update the files tomorrow. Note: Original issue is not updated yet.

Changes made in 1.0.9 (very short version)

Not talked about yet (will comment that soon)

Still to do

Files

Don't bother downloading these, most likely not stable, still work in progress, basically stopped in the middle of work, tired, can't guarantee anything :) alright, see you soon for the - maybe - last update.

example_CameraZoom1.0.9.zip Extension.zip

Elairyx7301 commented 3 years ago

1.0.10

I've changed my GitHub account name because I disliked my previous one. I'm only saying this so if anyone finds something from me that is supposed to be there but isn't available anymore or returns a 404, you know what the source for that is. Now back to the extension update.

Current action list

image

Changes made / Annotations

Merged versions

Thank you, Tristan for the smooth camera zooming actions you made for the extension! I have now merged our two extension version because I was missing the regular/rapid zooming actions. I've seen that you added a "set speed to 1 for instant change" and I can imagine that's very nice for changing the speed dynamically, even to 1 but having a separate action that changes it directly.

Changed phrasings

I tried to avoid long event sentences as much as possible and keep it as clear as possible. I see the event sentence as a kind of overview, the user can edit the action to see the full description and functionality of all parameters. Also, I changed words like "control" to "allow" to make it sound clearer. I optimized function names, added "Smoothly" in front of the smooth camera zoom actions in method names and event sentence for visual clarity, moved parameter label values to long descriptions.

Example of what and how I changed event sentences: Changed: Control camera zoom with keyboard. Layer: _PARAM2_, Zoom in key: _PARAM5_, Zoom out key: _PARAM6_, Zoom step: _PARAM4_, Zoom speed: _PARAM10_, Trigger once: _PARAM7_, Min zoom: _PARAM8_, Max zoom: _PARAM9_, Change zoom on all layers: _PARAM3_ To: Smoothly zoom camera on _PARAM2_ with _PARAM5_ (_PARAM6_, step _PARAM4_, max: _PARAM8_, min:_PARAM9_, all layers: _PARAM3_, trigger once: _PARAM7_) (might differ from final sentence)

Moreover, I've grouped all boolean parameters at the bottom of the parameter list for clarity and usability and removed redundant events, optimized events a bit and unified action description, event sentences etc. as much as possible. I've fixed inconsistencies made my me and occurred by merging the two projects and found and fixed a little issue in "Smoothly set camera zoom" (action wouldn't respond)

Action added, checked all actions

Added the action "Smoothly change camera zoom for a camera on all layers". The action zooms rel. fast, so I've added a note telling the user about it. I checked all actions and made sure everything works. I had to change the project events a little because having a full sheet of all zooming actions couldn't be handles easily anymore.

I've added all actions into (grouped by smooth/instant and one/all layers) events with a "Never" condition so they can be viewed but won't trigger and tested each action separately at the bottom of the event sheet.

There are actually still two events I forgot to delete. Bouh/Trsitan, you can delete them or change the event sheet as you wish.

Other changes, version, authors

I changed the version to v1.0.10 even though I feel like it should rather be something like v3.4.7 because of how many huge changes there have been :v

Personal annotation and ready to go

Thank you Bouh and Tristan for adding your touch to and helping optimize the extension! I never thought this would turn out to be such a monster of an extension - and I guess you did neither because the "final fixed" tag has been added even before v1.0.1 -

Even though I am proud of the result, I think it is still pretty difficult when 3 people try to optimize 1 code because everyone thinks their way is more optimal :)

I'm happy with what this extension has become and I'm ready for a release.

Tristan, only if you don't have anything to add or to improve, I would mention Bouh and get this thing done :-)

Files

example_CameraZoom1-0-10.zip Extension.zip

tristanbob commented 3 years ago

I'm worried that we now have too many functions and they will confuse users. Even I have a hard time determining what each function does.

Current function full names:

Do you think these can be consolidated into 3 user-facing functions? (Hint: We should be able to still use the existing functions on the backend, and those functions changed to Private to hide them)

Bouh commented 3 years ago

The private functions are indeed in this objective, it is an excellent idea, we would have in the end only three actions that are very customizable We will have a complete and robust extension!

Elairyx7301 commented 3 years ago

Great point, thank you Tristan. I think I got too caught in the extension that I forgot that other users (now including me because I didn't touch the extension for a few days) are easily confused by it. I will get to it - probably around ~Monday~ Sunday. I have worked at it already but then the editor crashed (4ian/GDevelop/issues/3093) so I will have to remake it - I didn't get too far anyway so that's no problem ;p I'll post another comment with the files, update the issue information and files and would mention Bouh for a release if that would be alright with everyone.

Bouh commented 3 years ago

Sound good to me :+1:

Elairyx7301 commented 3 years ago

I've encountered an error in one of the now four total actions (allow keyboard, allow mouse wheel, set zoom absolute, change zoom relative). The rest is working perfectly (surprisingly) but this last action doesn't want to cooperate. I will have to push the release even more but I am trying my best to get it done because this little issue really can't be that hard ...he said.

D8H commented 3 years ago

Constant zoom

A zoom step is usually calculated with a multiplication not an addition For instance, for a continuous zoom, it would be a formula like this:

CameraZoom(GetArgumentAsString("LayerName"),GetArgumentAsNumber("CameraID"))*pow(GetArgumentAsNumber("ZoomStep"),Variable(__CameraZoom.ZoomCustomKeyValue)*TimeDelta())

See it in action: https://user-images.githubusercontent.com/2611977/136658542-2c724ba5-0a07-4b80-b5db-3c6a3fd9b9be.mp4

Mouse wheel

While keys can be pressed over time, a mouse wheel can't. A mouse wheel condition is like a released key condition. It must not use TimeDelta(). Try to change the frame rate, it will change the zoom speed in the current version.

For a mouse wheel, it's really is a zoom step. For keys, it should be called a zoom speed in "zoom factor per second".

The mouse wheel could be used for a continuous zoom if a click make it zoom for a given amount of time. It will make a better effect, but it's more complicated to implement also.

Factorization

The zoom formula is copy-pasted 4 times in each function. It makes it very difficult to modify it. Unless there is a performance constraint that forbid it, I think it should be factorize.

Elairyx7301 commented 3 years ago

Update on the situation

I feel like this is getting out of hand a little bit for me. Don't get me wrong, I really love and highly appreciate all the support and advice from all of you for the extension but I'm slowly getting overwhelmed. Merging two projects from two different people with different ideals of how code should work and how to structure it is by itself no easy task but finding and solving problems and errors is rather complex and exhausting over a long period of time for me. I think I now have (finally) understood where you three want to go with the extension - I am thinking about remaking everything from ground up and rebuilding it step by step adding everything we've discussed and testing everything.

—— Now, I am not saying that I'm done with the extension or that I won't continue working on it eventually but I can't say anything about how long making changes or any other action will take me from now on. I am sorry about this but I don't think there is a way for me to change it at the moment.

v1.0.11

This is the current status. I have now merged the two projects and added four actions for the user with all options they need to have instead of the original ten rather confusing actions and names. Almost everything is working with two or three exceptions (made clear in the project file / regular event sheet with comments and description) so it's far from done, it seems.

example_CameraZoom1.0.11.zip Extension1011.zip

Annotation

Hey @D8H, Thank you for the reply! We already discussed adding TimeDelta() to the expressions to make it independent from the project FPS. The only issue with that is that I have no idea how to implement it in a JS block since I didn't seem to be able to get the FPS of the project. I assume you could build your own FPS counter inside JS but I am far from saddle-proof to get that to work properly.

I like the idea of factorizing it but I'm not good at imagining how to implement it. Would it be a separate / private method that calculates the current zoom value for the other methods? Furthermore, the expressions are pretty similar, there are differences between "set to (instant)", "change by (instant)", "set to (smooth)" and "change by (smooth)" - where, to be perfectly honest, I'm currently not seeing too many reasons to modify the expressions or logic behind it that often for it to be incredibly annoying.

I think I should mention you, @tristanbob, as well because you've actively been with this project since the beginning and so you are also up to date.

D8H commented 3 years ago

The time delta can be got like this:

const timeDelta = layer.getElapsedTime() / 1000;

About the factorization, I didn't understand that it was limited by the way all layers zoom are got. So, you are right that doesn't seem possible to factorize the formula easily.

ZoomMinValue and ZoomMaxValue are repeated almost everywhere. There could be a ClampZoom and ClampZoomAllLayer instead. It can be kept in "UserMethod" though and their events will be easier to read as the clamp can be done once at the end instead of passing it for each mode possible.

The zoom formula is not correct and must be changed before a release in my opinion (see my previous comment).

Bouh commented 2 years ago

I'm confused, by openning the example there are more functions, the list is so long, the idea wasn't to kept 4 actions? There are descriptions that contain [URX], what does that mean? image Also in functions list some of them start with UserMethod_ image

I deduce guess you're still working on it, I'll wait before taking a new look.

Lerp is in the engine, if you prefer using it instead of create a function for it. gdjs.evtTools.common.lerp();

Elairyx7301 commented 2 years ago

@Bouh As I have mentioned in my comment at least once, it's still in the progress and the current version is far from stable. [URX] is my cat running over my keyboard is solely for me so I can clearly tell apart the final methods (prefixed with UserMethod_ because this method is supposed to be for the user) from the other ones. As you can see I already have partially done is to set actions to private so the user can't see them anymore. I could move all required events into those four methods but I see that as one of the last steps and shouldn't be considered in a state where not even the basic works / not even all functionalities are implemented properly.

D8H commented 2 years ago

What do you think about removing the actions to zoom every layer?

Elairyx7301 commented 2 years ago

Thanks @D8H (please let me know if you don't want to be mentioned here) I have also thought about this but just kept it in because it seemed like a nice addition and Bouh suggested and wrote the JS block for it but I definitely see your point, GUI might not be the most usual layer for zooming.

I will think about it again but it seems like we should leave out the methods for all layers.

Bouh commented 2 years ago

Has any progress been made recently? I don't see any activity in the last month here.

Elairyx7301 commented 2 years ago

As mentioned on the Discord, I have finished the general structure/events of version 2 and will hopefully be able to form those events into an extension in the near future.

ToDo List

Elairyx7301 commented 2 years ago

Version 2.0.0

I've drastically reduced the complexity of the project and extension to try to make it as clean as possible. Also, with those changes, the user has more control over when exactly the zooming stops, which I think is rather important, but has the ability to make it ease out if wanted/needed.

The only component that is missing is the implementation of TimeDelta(). I'm very unsure how I should go about it because multiplying a speed value the closest to an instant zoom (0.999) with the TimeDelta() will leave me with a max speed of 0,01665 (60 FPS) and I'm not sure where to include TimeDelta() as well, so I'd be happy about a hint here.

Extension content

Files

Extension.zip Project.zip

D8H commented 2 years ago

To help reviewers, this is the expressions from the v2.0.0. I find them a bit hard to understand. Can you explain your train of thought that results to these expressions?

if ZoomSpeed > 1

clamp
(
    GetArgumentAsNumber("ZoomTarget"),
    GetArgumentAsNumber("ZoomMin"),
    GetArgumentAsNumber("ZoomMax")
)
clamp
(
    lerp
    (
        CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera")),
        1 - GetArgumentAsNumber("ZoomTarget") * CameraZoom(GetArgumentAsString("Layer"),
        GetArgumentAsNumber("Camera")),
        clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)
    ),
    GetArgumentAsNumber("ZoomMin"),
    GetArgumentAsNumber("ZoomMax")
)

"(1 - a) b" in the following expression and "1 - a b" in the previous one. Is it intended? 

if ZoomSpeed > 1

clamp
(
    (1 - GetArgumentAsNumber("ZoomTarget")) * CameraZoom(GetArgumentAsString("Layer"),
    GetArgumentAsNumber("Camera")),
    GetArgumentAsNumber("ZoomMin"),
    GetArgumentAsNumber("ZoomMax")
)

Positive (0 to 1): Zoom in; Negative (-1 to 0): Zoom out

There is no such thing as a "negative zoom". It's either between 0 and 1 or between 1 and positive infinity.

Elairyx7301 commented 2 years ago

@D8H Awesome catch! That's not intended. It's supposed to be (1 - a) * b as well.

Absolute zoom (speed <= 1)

Lerp camera zoom to the target value.

Implementation concept:

lerp(currentZoom, target, speed)

Using

currentZoom = CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera"));
target = GetArgumentAsNumber("ZoomTarget");
speed = GetArgumentAsNumber("ZoomSpeed");

Additional

speed shouldn't be greater than 1 or smaller than 0, therefore:

clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)

Current status

lerp
( CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera")),
  GetArgumentAsNumber("ZoomTarget"),
  clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)
)

Final expression

The result of the lerp() method will return the zoom level the camera will be set to. This value should now be clamp()'d so the final zoom value is between the custom min/max zoom values.

clamp
(
    lerp
    (
    CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera")),
    GetArgumentAsNumber("ZoomTarget"),
    clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)
    ),
    GetArgumentAsNumber("ZoomMin"),
    GetArgumentAsNumber("ZoomMax")
)

     

Absolute zoom (speed > 1)

Instantly set camera zoom to the target value, since the description announces speed = 1 to be an instant change. This is ignoring the lerp() logic since easing is not needed here and x = lerp(x, y, 1) should be the same as x = y whereas I prefer the latter because it's significantly cleaner.

Current status

lerp
( CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera")),
  GetArgumentAsNumber("ZoomTarget"),
  clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)
)

Final expression

The target value should now be clamp()'d so the final zoom value is between the custom min/max zoom values.

clamp
(
    GetArgumentAsNumber("ZoomTarget"),
    GetArgumentAsNumber("ZoomMin"),
    GetArgumentAsNumber("ZoomMax")
)

     

Relative zoom (speed <= 1)

Lerp camera zoom to a zoom value relative to the current zoom value.

Implementation concept:

lerp(currentZoom, target, speed)

Using

currentZoom = CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera"));
speed = GetArgumentAsNumber("ZoomSpeed");

The target value here cannot just be the argument, since it should be relative to the current zoom. If I want a 10% change (target value might have unfortunate naming since it is rather the change than the actual target, I noticed) then I need to have the following calculation:

x = x - (x * change)
x = 100 - (100 * 0.1)
x = 100 - 10
x = 90

The same result can be achieved by subtracting change (or target) from 100% and multiplying the result with the initial value.

x = x * (1 - change)
x = 100 * (1 - 0.1)
x = 100 * (0.9)
x = 90

In this case is 90 the new target value. Therefore:

target = (1 - change) * currentZoom;

Using

currentZoom = CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera"));
change = GetArgumentAsNumber("ZoomTarget");

Additional

speed shouldn't be greater than 1 or smaller than 0, therefore:

clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)

Current status

lerp(currentZoom, target, speed)

Using the currentZoom value as usual, using the just described method for target and the clamp()'d ZoomSpeed argument as usual.

lerp
(
    CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera")),
    (1 - GetArgumentAsNumber("ZoomTarget")) * CameraZoom(GetArgumentAsString("Layer"),
    GetArgumentAsNumber("Camera")),
    clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)
)

Final expression

The result of the lerp() method will return the zoom level the camera will be set to. This value should now be clamp()'d so the final zoom value is between the custom min/max zoom values.

clamp
(
    lerp
    (
    CameraZoom(GetArgumentAsString("Layer"), GetArgumentAsNumber("Camera")),
    (1 - GetArgumentAsNumber("ZoomTarget")) * CameraZoom(GetArgumentAsString("Layer"),
    GetArgumentAsNumber("Camera")),
    clamp(GetArgumentAsNumber("ZoomSpeed"),0,1)
    ),
    GetArgumentAsNumber("ZoomMin"),
    GetArgumentAsNumber("ZoomMax")
)

     

Relative zoom (speed > 1)

Uses the exact same concept as the absolute zoom (speed > 1). Lerp() method is ignored, final value is wrapped inside a clamp() method to limit the resulting zoom value for the camera.

Final expression

clamp
(
   (1 - GetArgumentAsNumber("ZoomTarget")) * CameraZoom(GetArgumentAsString("Layer"),
   GetArgumentAsNumber("Camera")),
   GetArgumentAsNumber("ZoomMin"),
   GetArgumentAsNumber("ZoomMax")
)

     

Negative zoom

I have thought about this for a long time and the current implementation seems the most logical and intuitive to me. Zooming a camera to 1.5 (absolute) is what you are describing and what I definitely agreeing on. However, when I want to change the camera zoom by 10%, I would have to think thrice before entering 1.1 into the argument field. I tried to follow the x = x + y system, where you can decide the y value. If you want a smaller x result, you need a negative y value but adding or subtracting a certain value is the the same when you look at the absolute value (like 0.2 and -0.2). This is not the case for when the change is relative to another value (like 0.8 and 1.2)

I wanted to have the user enter e.g.: 0.156 if they want to zoom in by 15.6% of the current zoom and enter e.g.: -0.756 if they want to zoom out by 75.6%, also because I can't really see how many users - when wanting to zoom out by 17.6% - would enter 0.824 but, again, -0.176 seems more intuitive to me.

D8H commented 2 years ago

However, when I want to change the camera zoom by 10%, I would have to think thrice before entering 1.1 into the argument field.

This is how the scale action from sprites works. This is the most intuitive way of doing it. The scale action adds a long description to explain it to people without the math knowlegde. But, decimal multiplication and percentage are seen in middle school.

Trying to make something easier when it's already simple will likely just make it more opaque and hard to understand.

Elairyx7301 commented 2 years ago

Again, I'm totally agreeing to that. As already mentioned, the issue is that sprite scale uses an absolute value for the scale. Here I need a relative change and using 1.1 for a 10% change seems highly counter intuitive to me. This is the basic concept of how lerp() works in the first place. If I want a sprite to be at 1.5 scale, that value is absolute and independent from the current zoom so only 1.5 makes sense here.

D8H commented 2 years ago

Again, I'm totally agreeing to that. As already mentioned, the issue is that sprite scale uses an absolute value for the scale.

For the sprite, it's a scale relative to the original dimensions. In this case, It's relative to the current zoom.

Here I need a relative change and using 1.1 for a 10% change seems highly counter intuitive to me.

Well, "a 10% change" means 110% of the original value.

This is the basic concept of how lerp() works in the first place.

I don't understand why you're bringing this. You want: a = 0.1, -0.1, 0.5... I suggest: b = 1.1, 0.9, 1.5... b = 1 + a

If I want a sprite to be at 1.5 scale, that value is absolute and independent from the current zoom so only 1.5 makes sense here.

No, the issue is the parameters of the zoom action. There should be only a speed parameter. There is no need for this parameter:

Zoom change (usually in combination with "Trigger once" condition)

And, the speed parameter should be something understandable: a zoom factor per second. If it's set to 2, the zoom will go from 4.3 to 8.6 in a second, 17.2 the next second....

Elairyx7301 commented 2 years ago

Alright, I see. I will implement that soon.

Just in case, here are the old but fixed files for v2.0.0 with the correct expression and other minor changes Extension.zip, Project.zip

D8H commented 2 years ago

You will probably need an exponential. That might not be very fast but it should work and it's simple. We just have to hope the browser has some good optimizations for it.

zoom(t) = zoom(t - TimeDelta) * pow(speed, TimeDelta)