feat: Added new camera parameters: 'zoominspeed', 'zoomoutspeed' and 'yscrollspeed'.

[Kasasagi77]

Adds the zoominspeed, zoomoutspeed and the yscrollspeed camera parameters - relative values in the [0, 1] range. See: discussion.

[Eiton]

I'd suggest you change newLeft and newRight instead of changing newScale directly.
With autocenter = 0, if you change zoominspeed, test with one character standing still and another character moving back and forth, camera pos X will change too.

[Kasasagi77]

I'd suggest you change newLeft and newRight instead of changing newScale directly. With autocenter = 0, if you change zoominspeed, test with one character standing still and another character moving back and forth, camera pos X will change too.

I have rewritten it in terms of newLeft and newRight and fixed the characters being pushed/dragged by the screen boundary. I think that this zoom speed limiting actually works as it should now.

However the ytension mode (as opposed to verticalfollow mode) would still benefit from a yscrollingspeed parameter, because right now the vertical scrolling in this mode is always at full speed, equivalent to verticalfollow = 1. I am currently thinking about adding this too.

[Eiton]

With autocenter = 0, if you change zoominspeed, test with one character standing still and another character moving back and forth, camera pos X will change too.

This problem persists. I think the function should take into the account the difference between oldLeft, newLeft and oldRight, newRight. e.g. If newLeft is not changed, the function should not change it.

[Kasasagi77]

With autocenter = 0, if you change zoominspeed, test with one character standing still and another character moving back and forth, camera pos X will change too.

This problem persists. I think the function should take into the account the difference between oldLeft, newLeft and oldRight, newRight. e.g. If newLeft is not changed, the function should not change it.

Hmm... let me clarify what the problem is, because I may not understand correctly.

Is the problem that the camera X changes after slowing down zoom? The camera X not changing is the correct behaviour?

If I understand this correctly, then:

1. If both characters are a distance away from the screen border, then the camera X does not change - i.e. the x changes are usually 0 and at most in the range of float32 calculations accuracy, most decidedly sub-pixel; I can try to reduce this, but I do not think this is a visible problem,

2. If a character is standing near the screen edge or the stage edge or both, then the camera X indeed changes due to enforced bounding; without this bounding either the out-of-bound/missing parts of backgrounds will be revealed or the character will be dragged/pushed by the camera edge,

   • an alternative to this behaviour is to just let the camera zoom out at full speed in this edge case; if I understand it correctly, this is how tensionvel behaves, but this is the difference between tensionvel and zoomoutspeed - zoomoutspeed does not allow to zoom out faster, it forcefully restricts the zoom speed.

To my eye, the observable behaviour of the camera bounding described in point 2 looks good? I do not observe any visible artifacts, except for the characters being blocked from escaping, by the screen edge, when both characters are on the outer edges, but it is an expected side-effect of forcibly slowing down zoom?

[Eiton]

zoominspeed = 1

Ikemen.GO.2024-05-30.08-51-30.mp4

zoominspeed = 0.1

Ikemen.GO.2024-05-30.08-53-17.mp4

The camera is moved towards right that rightest is far beyond the tension.

[Kasasagi77]

zoominspeed = 0.1

The camera is moved towards right that rightest is far beyond the tension.

This would also be true for tensionvel = 0.1?

The camera keeps moving to the right, because it is still zooming in. The zoom is trying to reach the same target, but it was slowed down, so it is "catching up". Is this in some way a problem?

[Kasasagi77]

In fact - during the "catching up" phase, as far as I can tell, my zoom slowdown function does not change the camera X given in the input at all - the same camera X goes out.

[Eiton]

tensionvel = 0.1

Ikemen.GO.2024-05-30.18-54-56.mp4

You can see camera pos X is not moving beyond -9.41178

[Kasasagi77]

tensionvel = 0.1

You can see camera pos X is not moving beyond -9.41178

OK - I think I can see what you mean now - thanks! 🙏

I pushed a commit fixing this.

[potsmugen]

I haven't had the chance to test it yet, but some opinions:

zoomoutspeed you normally want to be as fast as possible anyway. zoominspeed is already controlled by tensionvel as Eiton pointed out before. So these parameters aren't as groundbreaking as I first thought. Having more options is nice though, I guess.

In previous Ikemen builds, verticalfollow worked even if tensionhigh and tensionlow were specified. Perhaps bringing back that behavior if the stage has ikemenversion would be more streamlined than adding more parameters (yscrollspeed)?

[Kasasagi77]

In previous Ikemen builds, verticalfollow worked even if tensionhigh and tensionlow were specified. Perhaps bringing back that behavior if the stage has ikemenversion would be more streamlined than adding more parameters (yscrollspeed)?

I do not know how it worked in previous Ikemen versions - but looking at the current code I have some doubts it such a thing would be easily portable from some previous version.

I tried to add something like verticalfollow directly in the Y Tension algorithm code, but I did not achieve results that were satisfactory to me (without aproaching complex and arbitrary solutions) so I just implemented it as yscrollspeed (applied in the post-processing in an algorith-independent way) because it was simpler and achived the desired effect of allowing to slow down y scrolling in the Y Tension mode.

[Kasasagi77]

zoomoutspeed you normally want to be as fast as possible anyway. zoominspeed is already controlled by tensionvel as Eiton pointed out before. So these parameters aren't as groundbreaking as I first thought. Having more options is nice though, I guess.

It sounds to me like you are not convinced that these parameters are needed or useful - especially if you think that slowing down zoom out is of no value. I would appreciate a straight feedback, if you think that this is ultimately not worth merging and cluttering the stage parameters.

[potsmugen]

No, they are good. We've just been getting a lot of camera parameters lately, and most of them aren't even documented.

If verticalfollow can't be used to kill two birds with one stone anymore, maybe rename yscrollspeed to ytensionvel to fit with tensionvel?

You could also squash some of the commits. For the purpose of the main repo, some of them are the same commit.

[Kasasagi77]

If verticalfollow can't be used to kill two birds with one stone anymore, maybe rename yscrollspeed to ytensionvel to fit with tensionvel?

If someone else can do this the verticalfollow way then I have no problem with not merging yscrollspeed at this time, it is just that I personally tried implementing something similar and have given up on it, but I am not saying that it is impossible - it was just not obvious to me how to best do it simply and effectively, so I did it in another way altogether.

Regarding the rename yscrollspeed -> ytensionvel - the thing is - this parameter has nothing to do with tensionvel or any sort of tension currenty, it is abstracted away from the Y Tension algorithm and just literally scales the vertical scrolling speed (as a post-processing step to any camera algorithm) and can just be used instead of an alternative verticalfollow/ytensionvel implementation inside the Y Tension algorithm. That is why I think that the ytensionvel name is not appropriate.

Tell me if you think that it would be preferable to exclude the yscrollspeed parameter from this PR for the time being.

[Kasasagi77]

You could also squash some of the commits. For the purpose of the main repo, some of them are the same commit.

Sure. I will do it.

[Kasasagi77]

No, they are good. We've just been getting a lot of camera parameters lately, and most of them aren't even documented.

I can document my merged parameters or the parameters that I was substantially involved in.

Do you mean to document them here on the Github's wiki? Or in the source code?

If it concerns the wiki documentation - I assume the wiki can reflect the develop branch, and not the official release? If we are to start documenting these parameters now?

[potsmugen]

Regarding the rename yscrollspeed -> ytensionvel - the thing is - this parameter has nothing to do with tensionvel or any sort of tension currenty

I had a feeling that might be the case. Nevermind then. Another nitpick though (I nitpick names a lot, don't mind me) is they could be "mul" or something instead of "speed" since they're a multiplier rather than absolute velocity (tensionvel arguably has the same issue but is not worth changing). Then again I can't think of anything actually better than "speed" right now. Just a random thought.

Do You mean to document them here on the Github's wiki? Or in the source code?

In the wiki. Though source code notes are also helpful. We've been documenting stuff as "nightly build only" because that's a lot less work than documenting them all at once when the stable version is out.

Tell me if you think that it would be preferable to exclude the yscrollspeed parameter from this PR for the time being.

That could be the case if someone was working on implementing that other feature, but I don't think anyone's doing that right now so I don't see why not. We can always refactor some things and deprecate others down the line since the engine is constantly in flux.

One other thing is (ideally) new stage parameters would also be added to the StageVar trigger and ModifyStageVar state controller, but there's no rush.

[Kasasagi77]

Another nitpick though (I nitpick names a lot, don't mind me) is they could be "mul" or something instead of "speed" since they're a multiplier rather than absolute velocity (tensionvel arguably has the same issue but is not worth changing). Then again I can't think of anything actually better than "speed" right now. Just a random thought.

The verbose version describing what these parameters really do would be zoominspeedfactor, zoomoutspeedfactor and yscrollspeedfactor, since they literally multiply the speed (or in other words - the rate of change) of zooming and moving the camera. I would personally just prefer the factor suffix here instead of mul.

Should I change the names to be more verbose?

One other thing is (ideally) new stage parameters would also be added to the StageVar trigger and ModifyStageVar state controller, but there's no rush.

Should this be done as part of this PR, or can we merge this PR first?

[potsmugen]

Either just speed or just factor would be better I think. Or just rate as you also mentioned. Whichever you like best since they're all good.

The char stuff can be added later, yeah.

[Kasasagi77]

OK then - if we do not want to go verbose with the parameter names, then I will just leave the current ones.

Thank you very much for the feedback/review!

I assume there are no more changes required for this PR then.