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

Phantom camera's global_position getting incorrectly(?) floored when it's limit is set #268

Closed
audeck opened this issue Apr 28, 2024 · 2 comments

Comments

@audeck
Copy link
Contributor

audeck commented Apr 28, 2024

Issue description

I've been playing around with 2D phantom cameras recently (great initiative, btw!) and ran into weird behavior. Using lawnjelly/smoothing-addon, simple cameras with no limit work as expected.

However, once I set a limit target (and, quickly looking through the code, I imagine the same thing happens when limit sides are set), the camera's global_position kept getting rounded down to integer values, causing visual jitter while following a node with a position that wasn't rounded to integers.

After dissecting phantom_camera_2d.gd, it seems the _set_limit_clamp_position function, called from _process through _interpolate_position on every frame here returns a Vector2i, resulting in the aforementioned rounding down. After changing the return type to Vector2, everything works as expected.

As someone with little to no experience using this plugin, I don't whether:

  1. I'm doing something wrong and the _set_limit_clamp_function is getting called incorrectly every frame (and otherwise works as expected), or
  2. the return type is indeed wrong.

That is mainly why I'm creating an issue, rather than submitting a PR (the single-commit PR would also change literally one (1) character). Thank you.

Steps to reproduce

  1. Create a node that is able to move at non-integer increments (a player character, etc.)
  2. Setup a basic Camera2D (no smoothing, etc.) w/ a PhantomCameraHost
  3. Add a PhantomCamera2D following said node on simple mode
  4. Set any limits to said PhantomCamera2D

(Optional) Minimal reproduction project

No response

@ramokz
Copy link
Owner

ramokz commented Apr 28, 2024

Can't quite replicate it, however I think your rationale and finding makes sense.

While the Limit sides have to be integers, that doesn't mean the positioning of the target_position (on line 563) needs to be too, as it will still be clamped to the side definitions anyway.

In fact, the target position is currently always an integer when a limit target is applied, which is definitely not right. _set_limit_clamp_position() is also casting a regular Vector2 to a Vector2i, which is kinda silly too.

Some quick tests don't show any issues with what you suggest. If it solves your problem, then I am happy to get that change in.

Feel free to make the PR and get the contribution credit; otherwise, I'm happy to make that quick change and cite this issue.

@audeck
Copy link
Contributor Author

audeck commented Apr 28, 2024

Submitted #269. Thanks!

@audeck audeck closed this as completed Apr 28, 2024
sircodemane added a commit to sircodemane/phantom-camera that referenced this issue Oct 14, 2024
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

2 participants