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

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

Closed
CantyCanadian opened this issue Apr 29, 2024 · 10 comments
Closed

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

CantyCanadian opened this issue Apr 29, 2024 · 10 comments

Comments

@CantyCanadian
Copy link

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
Copy link
Contributor

The API has changed to pcam.limit_left = -1000.0, etc:


The docs are still out of date

@ramokz
Copy link
Owner

ramokz commented Apr 29, 2024

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
Copy link
Author

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
Copy link
Owner

ramokz commented Apr 29, 2024

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
Copy link
Owner

ramokz commented Apr 29, 2024

set_limit and get_limit should now be in the latest hotfix

@CantyCanadian
Copy link
Author

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
Copy link
Owner

ramokz commented Apr 30, 2024

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 ramokz mentioned this issue May 2, 2024
@ramokz
Copy link
Owner

ramokz commented May 6, 2024

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

@ramokz ramokz closed this as completed May 6, 2024
@CantyCanadian
Copy link
Author

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
Copy link
Owner

ramokz commented Jul 2, 2024

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

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

No branches or pull requests

3 participants