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

Implementing a camera confinement option for the PhantomCamera2D #430

Open
Xinart opened this issue Dec 10, 2024 · 6 comments
Open

Implementing a camera confinement option for the PhantomCamera2D #430

Xinart opened this issue Dec 10, 2024 · 6 comments

Comments

@Xinart
Copy link

Xinart commented Dec 10, 2024

Project Type

2D

Feature Description

Limit the camera’s position to a volume or area defined by a CollisionPolygon2D
Should work like the Camera Confiner of Cinemachine in Unity : https://www.youtube.com/watch?v=CvJaHAvp6dA

Use Cases

As part of the design of a level, I'd like to confine the camera to several complex polygonal zones to easily limit its movement in various level design situations.

(Optional) Proposed Solution

I would like to add a field bellow the Limit target property of the Limit seciont of the PhantomCamera2D called "Limit confiner area" Which accept only a CollisionPolygon2D that will define that the camera have to stay in this area
image

As the plugin is written in GDSCRIPT i am abble to implement this myself into the code

@upbeatDiamond
Copy link

How would this be different from Limit Target, which can be pointed to a collision shape or tilemap and stay bounded to the object according to the Phantom Camera website ?

@Xinart
Copy link
Author

Xinart commented Dec 10, 2024

Actually the limit target is only limiting on a rectangle area

For more advanced level design i would need that the camera saty confined inside a polygonal Area like this one :
image

@ramokz
Copy link
Owner

ramokz commented Dec 10, 2024

This has been brought up before, and the advice I generally give at the minute is to have multiple CollisionShape2Ds within different Area2D nodes, and then switch the Limit Target between them during runtime.

Adding a free form collider limit like this is exponentially more complicated than the current system, both in terms of implementation, but also getting the expected behavior right.

The addon's current Limit system leverages Godot's built-in one, so adding this would be its own thing with its own logic and behavior.

Not against adding support for this but, again, it's a fair amount of work.

If you would like to take a stab at it, feel free!

I would like to add a field bellow the Limit target property of the Limit seciont of the PhantomCamera2D called "Limit confiner area"

For this, I would rather have it be part of the existing Limit Target property. It just needs to identify what type of shape it is. Adding a second option will start getting confusing, as the user now has to know which one to use, which is a question the user shouldn't have to answer.

@Xinart
Copy link
Author

Xinart commented Dec 12, 2024

Not against adding support for this but, again, it's a fair amount of work.

If you would like to take a stab at it, feel free!

Currently i've worked on my side to implement this system and it's not finished yet
So my will with this issue is to implement myself this to your PhantoMCamera project beceaus i think it is the best place to put in and make everyone benifit it ! (And also i really appreciated your work on this really thank you for that <3)

For this, I would rather have it be part of the existing Limit Target property. It just needs to identify what type of shape it is. Adding a second option will start getting confusing, as the user now has to know which one to use, which is a question the user shouldn't have to answer.

Ok so i will consider using this field instead of what i thought i agree with you

Can you give me your green light to start working on that ?

@ramokz
Copy link
Owner

ramokz commented Dec 13, 2024

You're welcome to give it a go!

However, I think it's important to mention a couple of things when it comes to working on PRs, big or small.

There's never a guarantee that it will be merged into the repository. Ultimately, it comes down to the maintainer (me) to promote the feature, maintain the addition, fix any future bugs it may have etc. Once a contributor has made a PR and submitted it, there's no assurance that they will help resolve any future changes their work requires. So while the idea here is good, if it's implemented in a very complex, unmaintainable or non-flexible way, it comes down to me maintaining it, which can be no small job.

The reason I'm mentioning this is that I don't like rejecting other people's work. Yet, when the choice comes down to choosing between maintaining an addition to a project that I don't align with and in turn struggle with working on, or rejecting someone's hard work, it will always the latter. As much as I dislike throwing people's time and effort away. One or two of the closed PRs are examples of that, and it's one of the most common reasons why PRs don't get merged.

So if you want to give it a go still, I would highly recommend not working on it in secret for 2 months and then comes back with a finished solution. But instead make a Draft PR with a written down plan on how you're thinking of implementing it, potentially with a basic but functional version, before sharing anything.
This is just to make sure that there's no fundamental disagreements with the given solution from a design or tech perspective. The more communicative the work is early on, the more likely it is to succeed or at least not be a waste of time for the person making it.

@Xinart
Copy link
Author

Xinart commented Dec 14, 2024

Thank you for your time putting clarification here !

Ok so i'll proceed the way you recommended by starting with a draft PR to put documentation and plan on how i see implementing this, i'll see doing this in the next 2 weeks

Thank you for your answer !

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