ramokz / phantom-camera

👻🎥 Control the movement and dynamically tween 2D & 3D cameras. Built for Godot 4. Inspired by Cinemachine.
https://phantom-camera.dev/
MIT License
1.93k stars 62 forks source link

Unannounced change: Removal of set_limit(side, value) #270

Closed CantyCanadian closed 2 months ago

CantyCanadian commented 2 months ago

Issue description

It seems that the new version of the plugin has the set_limit and get_limit functions removed. Was this an intended removal? If so, is there a reason why? I use those functions and I cannot seem to find an explanation. I re-added them to my code in the meantime.

Steps to reproduce

This is in main right now.

(Optional) Minimal reproduction project

No response

ZenithStar commented 2 months ago

The API has changed to pcam.limit_left = -1000.0, etc: https://github.com/ramokz/phantom-camera/blob/7c358dd93df5cde3a9916c14c88c897527c50ff6/addons/phantom_camera/scripts/phantom_camera/phantom_camera_2d.gd#L325 The docs are still out of date

ramokz commented 2 months ago

This was done a while back, but remember the intent was due to the simplified property system where, as @ZenithStar says, you can assign the value directly to a given side. Must have forgotten to add that to the release notes.

That said, don't see a reason why having a general setter and getter where you pass both a side and a value as a parameter can't be added in again. So look into adding that in soon in a hotfix.

I did also discover a bug with setting the Limit Value with a number, so will be doing a fix for that as well.

The docs being out-out-date is definitely an oversight, so thanks for raising that!

CantyCanadian commented 2 months ago

Is there a changelog of function name changes between versions? I made a C# wrapper for the gd PhantomCamera and thus name changes will break without warning.

ramokz commented 2 months ago

I was considering making a list before the release, but decided not to in the end, as it would take quite some time to do. Given, writing the release notes and documentation were quite a large task alone. So didn't think it would have been worth the effort, as I imagined most wouldn't use many of the functions, and so would make for a relatively painless transition.

What I didn't know was that creating C# wrappers was a thing that people did, let alone something you could do, until yesterday…

ramokz commented 2 months ago

set_limit and get_limit should now be in the latest hotfix

CantyCanadian commented 2 months ago

I was considering making a list before the release, but decided not to in the end, as it would take quite some time to do. Given, writing the release notes and documentation were quite a large task alone. So didn't think it would have been worth the effort, as I imagined most wouldn't use many of the functions, and so would make for a relatively painless transition.

What I didn't know was that creating C# wrappers was a thing that people did, let alone something you could do, until yesterday…

It was the safest way to interact with a PCam in C# since I gotta magic-string every function calls, so might as well have the magic strings in a single class rather than scattered around. If you want the file, as a way to either add very rough C# support or get inspired to make your own, I can give it to you.

set_limit and get_limit should now be in the latest hotfix

Thank you!

ramokz commented 2 months ago

It was the safest way to interact with a PCam in C# since I gotta magic-string every function calls, so might as well have the magic strings in a single class rather than scattered around.

Definitely, I never liked how that was how you had to reference GDscripts in C# and vice versa. Always felt weirdly unstructured.

If you want the file, as a way to either add very rough C# support or get inspired to make your own, I can give it to you.

I would be very keen to on adding better C# support if that's possible, as I always assumed it wasn't. So if you're willing to share it, then I can see if that's something the addon could include out-of-the-box.

ramokz commented 2 months ago

Closing this issue as the original report is now merged and released.

CantyCanadian commented 1 week ago

PhantomCamera2D.zip

Oops, I missed the last message. Here's the file, though it hasn't been updated in a bit so might not be up to date.

ramokz commented 1 week ago

This is great, thanks for sharing this! Will look into getting C# support out-of-the-box at some point.