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
2.18k stars 72 forks source link

Follow Distance on ThirdPerson Follow Mode does not update Spring Arm spring_length property #135

Closed niceandgoodonline closed 10 months ago

niceandgoodonline commented 11 months ago

Issue description

Hey! First off, this library is so sick. Thank you so much for releasing this!

Motivation that lead to discovery (User Story): As a player I want to zoom the Third Person camera close and far away, depending on the situation.

Issue in more detail: So the title is pretty comprehensive, but I'll also go into how I am hacking around it and why I am framing the bug as I am. It COULD 100% be that I just do not understand how properly implement this feature with Phantom Camera, and I welcome an alternative solution!

If you script or manually adjust the active PhantomCamera3D's follow_distance(or follow parameters spring_length) parameter when it's set to ThirdPerson follow mode, mimic look at mode, and no look at target it does not adjust it's parent spring arm's length as expected.

Steps to reproduce

If you reach into the PhantomCamera3D during runtime and grab a reference to it's parent SpringArm3D node you can manipulate the spring length as expected.

heres some code snips you can drop into the provided character controller for the thirdperson demo to see this behaviour (or you could just open the example scene, and try to fiddle with the values in runtime Remote):

var thirdPersonSpringArm: SpringArm3D
var active_cam : PhantomCamera3D

@export var zoomSpeed: float = 5.0
@export_range(2.0, 20.0, 0.01) var zoomDistanceMax: float = 10.0
@export var zoomOverTheShoulderAmount: float = 2.0
@export var zoomStaticHeightOffset: float = 1.0
@export var zoomDynamicHeightOffset: float = 1.5

var zoomOverTheShoulderOffset: float = zoomOverTheShoulderAmount / 2
var zoom: bool  = false
var zoomIdentity: int   = 1
var zoomCurrent: float = 0
var zoomDistanceMin: float = 2.0

func _ready() -> void:
    if _player_pcam.get_follow_mode() == _player_pcam.Constants.FollowMode.THIRD_PERSON:
        Input.set_mouse_mode(Input.MOUSE_MODE_CAPTURED)

        active_cam = _player_pcam
        thirdPersonSpringArm = _player_pcam._follow_spring_arm_node
        zoom = true

func _unhandled_input(event: InputEvent) -> void:
    if event.is_pressed():
    if (event.is_action_pressed("zoom-in") or event.is_action_pressed("zoom-out")) and active_cam == _player_pcam:
        zoom         = true
        zoomIdentity = 1
        if event.is_action_pressed("zoom-out"):
            zoomIdentity = -1
        return

func _process(delta):
    if zoom:
        zoomCurrent = clamp(zoomCurrent + zoomIdentity * zoomSpeed * delta, zoomDistanceMin, zoomDistanceMax)
        thirdPersonSpringArm.set_length(zoomCurrent)
        _player_pcam.follow_distance = zoomCurrent
        var zoomCurrentNormalized = zoomCurrent / zoomDistanceMax
        var _xVal = zoomOverTheShoulderAmount * abs(zoomCurrentNormalized - 0.3) - zoomOverTheShoulderOffset
        _player_pcam.set_follow_target_offset(Vector3(_xVal, zoomDynamicHeightOffset * zoomCurrentNormalized + zoomStaticHeightOffset, 0.0))

(Optional) Minimal reproduction project

No response

ramokz commented 11 months ago

It looks like you're trying to set the follow_distance property via code, however, ideally it should be done with the setter method. Mainly because modifying the properties directly is often very inconsistent whether if they work or not (no idea why).

Does using set_spring_arm_spring_length(zoomCurrent) on the PCam work?

niceandgoodonline commented 11 months ago

Unfortunately, using the setter does not update the spring arm length with this setup. In my provided code block I am syncing the property on the PhantomCamera with the state of the SpringArm3D to avoid unknown side effects/bugs.

Other information I recalled while testing the setter just now:

  1. the PhantomCamera3D spring arm setting DOES matter for a very brief time during the initialization period (e.g. whatever you set the spring arm to be in the editor is respected during runtime).

  2. I made a "debug sphere" the child of the PhantomCamera3D to test what(if anything) was happening when the setter was used. the debug sphere keeps track of it's lastPosition, and checks if it's last position and global_position are not equal in _process. If true (e.g. its moved) then it prints the lastPosition.distance_to(global_position) and updates it's lastPosition.

Information I discovered from this:

A. since the sphere's position is visualized, the setter definitely does... something. that's why I setup the lastPosition tracking.

B. The sphere jumps to a static position for as long as I am "setting" a new value (my input logic is continuous while held). The debug sphere (which is a visualization of the the PhantomCamera3D) returns to it's "expected" position close to the Main Camera when the setter is not being updated.

C. No matter what value is provided to the setter it also goes to the same position, which seems to be it's local Vector3.ZERO + the follow offset. I tested this by lowering the dampening, and moving the follow target around. the distance displayed always followed a logic pattern of the camera resolving it's initial arm length + follow offset.

I wish I could just do a PR and fix this, but I am not to familiar with the way you've structured PhantomCamera3D with all the dynamic properties and such -- it's too complex for me to feel confident that I could make direct changes to a small portion without causing side effects elsewhere.

Aside -- what is the pattern/strategy you're using to architect PhantomCamera? It's probably something common, but in a mature state so getting a handle on it can't be done in 5 minutes.

EDIT: I can open a feature request (or maybe implement it once this issue is resolved) but the other zoom code here is creating an "over the shoulder" offset relative to the zoom level. the zoomOverTheShoulderAmount controls the magnitude of the offset, positive values create an over the right shoulder offset, and negative values create a over the left shoulder offset. the magic number 0.3 is actually the zoomOverTheShoulderApex where it's "fully offset", and the mixing of a zoomStaticHeightOffset + a dynamic one creates lets the designer/player fine tune the vertical offset depending on distance. this allows a smooth zoom transition between first person, over the shoulder, and player-centred-third-person.

ramokz commented 11 months ago

Actually, I think I know what the issue is.

Does replacing the PhantomCamera3D.gd inside addons/phantom_camera/scripts with the attached version below work? phantom_camera_3D.gd.zip

ramokz commented 11 months ago

EDIT: I can open a feature request (or maybe implement it once this issue is resolved) but the other zoom code here is creating an "over the shoulder" offset relative to the zoom level. the zoomOverTheShoulderAmount controls the magnitude of the offset, positive values create an over the right shoulder offset, and negative values create a over the left shoulder offset. the magic number 0.3 is actually the zoomOverTheShoulderApex where it's "fully offset", and the mixing of a zoomStaticHeightOffset + a dynamic one creates lets the designer/player fine tune the vertical offset depending on distance. this allows a smooth zoom transition between first person, over the shoulder, and player-centred-third-person.

Is the reason you're using one PhantomCamera with the zoom behaviour to enable the player to effectively “scroll” the camera's distance and offset it to be over-the-shoulder as they do that? As opposed to a toggle command that switches between third person and over-the-shoulder view, like in the third-person demo scene? Am mainly curious about the intended behaviour to see how that could potentially be supported.

I wish I could just do a PR and fix this, but I am not to familiar with the way you've structured PhantomCamera3D with all the dynamic properties and such -- it's too complex for me to feel confident that I could make direct changes to a small portion without causing side effects elsewhere.

Aside -- what is the pattern/strategy you're using to architect PhantomCamera? It's probably something common, but in a mature state so getting a handle on it can't be done in 5 minutes.

In short, nothing formal. It's mainly based on my own experiences and observations of what others have done. Most of it has been a case of studying other open source projects to see how they had set it up, and then try to replicate the best patterns I could identify with what fitted this project's goals.

One thing I aimed for around the code setup was to keep it as compartmentalized as possible, so if anything in particular is not working correctly it would typically be solved in one place rather than multiple. E.g. if you want to change the logic of the Third Person or Framed follow mode, then that's effectively all done inside their individual Match statement in PhantomCamera2D and PhantomCamera3D (line 527 here for Third Person) respectively.

The main density, and where I encounter the most slipups, are with the dynamic properties, which you also mention, as the way Godot does it have very little documentation and is very verbose — at least pre Godot 4.2. If you ignore the setter and getter functions, the dynamic property logic, i.e. _get_property_list(), _set(), _get(), are half of the code in phantom_camera_3D.gd, phantom_camera_2D.gd and phantom_camera_properties.gd alone. But if you're not looking to add, remove, or fix a property, you can effectively ignore all of that.

The main complication in the project, I think, is the interplay between PhantomCamera and PhantomCameraHost nodes, but I rarely need to tweak anything there. The only exception is when adding specific logic to how the Camera2D/Camera3D should operate, which is a fairly rare occurrence at this point.

In the case of the attached phantom_camera_3D.gd above, the only change was a missing a variable setter inside the set_spring_arm_spring_length(). Realized that there is nothing in the code to apply changes to the spring_arm3D node's spring_length after the _ready() function, which is why it worked when running the scene, but didn't do anything after that.

niceandgoodonline commented 11 months ago

Does replacing the PhantomCamera3D.gd inside addons/phantom_camera/scripts with the attached version below work? phantom_camera_3D.gd.zip

Using this phantom_camera_3d.gd script does update the spring arm when using the setter now! Thanks!

Is the reason you're using one PhantomCamera with the zoom behaviour to enable the player to effectively “scroll” the camera's distance and offset it to be over-the-shoulder as they do that?

The primary behaviour I wanted was third person with 2 or more offsets that the player/dev could have control over. The main offset I was looking for was along the Y axis -- so as they zoom in focus point of the camera frames things at the model "eye level", and then at max zoom out it places the character somewhere in the lower 3rd. The "over the shoulder" idea came later and it's not very well implemented/fleshed out (as it just offsets the x axis and not the z axis).

The most accessible way to expose a feature like this would probably be with a Path3D? I tried using the Path3D node since it would help me handle multiple offsets and visualize the curve, and I still might a Path3D in my next iteration.

But if you're not looking to add, remove, or fix a property, you can effectively ignore all of that.

It maybe useful to produce a tiny bit of documentation on how to add/remove/fix/extend things (or if it exists, promote that documentation's location). And even some recommendations on style for contributors? I might have been PRing this fix instead of posting about it if I had a bit of guidance 🐸

ramokz commented 10 months ago

The primary behaviour I wanted was third person with 2 or more offsets that the player/dev could have control over. The main offset I was looking for was along the Y axis -- so as they zoom in focus point of the camera frames things at the model "eye level", and then at max zoom out it places the character somewhere in the lower 3rd. The "over the shoulder" idea came later and it's not very well implemented/fleshed out (as it just offsets the x axis and not the z axis).

The most accessible way to expose a feature like this would probably be with a Path3D? I tried using the Path3D node since it would help me handle multiple offsets and visualize the curve, and I still might a Path3D in my next iteration.

To me, it sounds like what you're trying to achieve with the PCam moving along a linear path whilst being set to be in Third Person follow mode is encroaching on being a somewhat bespoke solution.

Using a Path3D node with a PCam in Path Follow mode seems like a sensible approach for part of this, but the Third Person mode in this setup complicates it quite a bit. Since the Third Person implementation is meant to be fairly “static” in terms of not doing anything clever beyond its intended purpose.

It absolutely does not need to be the case, and would love to increase its potential, but think it does become a question of how to make it more flexible without compromising its usability while avoiding a too narrow scope. Am not sure what that would look like, but confident that it's possible to achieve it. Personally, I would prefer not applying anything overly complex or specific to one particular PCam mode, unless it can be applied systemically and benefit other behaviours as well.

All that said, I'm sure there is a solve for what you want to do, though I'm frankly having a bit of trouble envisioning the end result fully, so can't suggest an approach over what you've got currently, unfortunately.

It maybe useful to produce a tiny bit of documentation on how to add/remove/fix/extend things (or if it exists, promote that documentation's location). And even some recommendations on style for contributors? I might have been PRing this fix instead of posting about it if I had a bit of guidance 🐸

In the case here, the solve for the original post ended up being fairly trivial, so personally don't mind receiving a bug report to solve it myself ;) Though I get the frustration with not being able to quickly solve a bug, submit a PR and already have the fix in your code.

The documentation has purposely not included any specific contribute guidelines nor code walkthrough / structure, as every release often moves a lot of it around. I want to minimize the amount of housekeeping with documentation, which is already a big pain point, until the addon gets into a more firm state — likely not until v1.0.

Am currently making a new documentation system meant to replace the GitHub wiki — in large part to reduce the time spent managing the documentation. So it's something that can be applied to that once the site is finished and, again, the addon gets into a firm state.

On that note, do you have any references to good documentation of codebases where you found it easy to grasp how it was structured? Mainly so there's a style I could potentially leverage for the documentation here.