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

set_spring_arm_spring_length does not change the spring length #172

Closed ZenithStar closed 9 months ago

ZenithStar commented 9 months ago

Issue description

This might be a problem with the Godot docs, but https://github.com/ramokz/phantom-camera/blob/07e0b9893385fea75bae7b5bf0b7cd014319356e/addons/phantom_camera/scripts/phantom_camera/phantom_camera_3D.gd#L977 does not work. It needs to be

_follow_spring_arm_node.spring_length = value 

Steps to reproduce

Here's a simple mouse wheel zoom to add to any test project

@export_group("Zoom", "zoom_")
@export_subgroup("Speed", "zoom_speed_")
@export var zoom_speed_mouse_wheel: float = 2.0 ** (1.0 / 10.0)
@export var zoom_speed_joy: float = 8.0
@export_subgroup("Clamp", "zoom_clamp_")
@export var zoom_clamp_min: float = 0.1
@export var zoom_clamp_max: float = 10.0
func _unhandled_input(event: InputEvent):
    if event is InputEventMouseButton:
        match event.button_index:
            MOUSE_BUTTON_WHEEL_UP:
                var new_zoom = clamp( controller.follow_distance / zoom_speed_mouse_wheel, zoom_clamp_min, zoom_clamp_max)
                controller.follow_distance = new_zoom  # does nothing
                controller.set_spring_arm_spring_length( new_zoom ) # does nothing
                controller._follow_spring_arm_node.spring_length = new_zoom # actually works
            MOUSE_BUTTON_WHEEL_DOWN:
                var new_zoom = clamp( controller.follow_distance * zoom_speed_mouse_wheel, zoom_clamp_min, zoom_clamp_max)
                controller.follow_distance = new_zoom  # does nothing
                controller.set_spring_arm_spring_length( new_zoom )  # does nothing
                controller._follow_spring_arm_node.spring_length = new_zoom # actually works

(Optional) Minimal reproduction project

No response

ramokz commented 9 months ago

Tired it out using the example code below in the _unhandled_input() function of player_controller_third_person.gd in the 3DFollowThirdPersonExampleScene scene and _follow_spring_arm_node.set_length(value) works fine for me.

if event is InputEventMouseButton:
  match event.button_index:
    MOUSE_BUTTON_WHEEL_UP:
       _player_pcam.set_spring_arm_spring_length(
           _player_pcam.get_spring_arm_spring_length() + 0.1
       )
    MOUSE_BUTTON_WHEEL_DOWN:
      _player_pcam.set_spring_arm_spring_length(
        _player_pcam.get_spring_arm_spring_length() - 0.1
      )

The syntax is a bit inconsistent in Godot's docs, but the setter method for SpringArm3D is correct.

Video example

https://github.com/ramokz/phantom-camera/assets/5159399/7c19db6b-15ab-4705-b92e-9d15976e1d03

ZenithStar commented 9 months ago

The following code fails to zoom the camera:

func _unhandled_input(event: InputEvent):
    if event is InputEventMouseButton:
        match event.button_index:
            MOUSE_BUTTON_WHEEL_UP:
                var follow_before = pcam.follow_distance
                var spring_before = pcam._follow_spring_arm_node.spring_length
                pcam.set_spring_arm_spring_length( pcam.get_spring_arm_spring_length() - 0.1 )
                var follow_after = pcam.follow_distance
                var spring_after = pcam._follow_spring_arm_node.spring_length
                print ( "MOUSE_BUTTON_WHEEL_UP | follow_distance %s -> %s | spring_length %s -> %s"%[follow_before,follow_after,spring_before,spring_after] )
            MOUSE_BUTTON_WHEEL_DOWN:
                var follow_before = pcam.follow_distance
                var spring_before = pcam._follow_spring_arm_node.spring_length
                pcam.set_spring_arm_spring_length( pcam.get_spring_arm_spring_length() + 0.1  )
                var follow_after = pcam.follow_distance
                var spring_after = pcam._follow_spring_arm_node.spring_length
                print ( "MOUSE_BUTTON_WHEEL_DOWN | follow_distance %s -> %s | spring_length %s -> %s"%[follow_before,follow_after,spring_before,spring_after] )
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5 -> 5.1 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.1 -> 5.2 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.2 -> 5.3 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.3 -> 5.4 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.4 -> 5.5 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.5 -> 5.6 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.6 -> 5.7 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.7 -> 5.8 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.8 -> 5.9 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_DOWN | follow_distance 5.9 -> 6 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 6 -> 5.9 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 5.9 -> 5.8 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 5.8 -> 5.7 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 5.7 -> 5.6 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 5.6 -> 5.5 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 5.5 -> 5.4 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 5.4 -> 5.3 | spring_length 5 -> 5
MOUSE_BUTTON_WHEEL_UP | follow_distance 5.3 -> 5.2 | spring_length 5 -> 5
ZenithStar commented 9 months ago

Okay. I updated PhantomCamera to the latest and it fixed the problem. Looks like my test project was running a version before https://github.com/ramokz/phantom-camera/commit/140b4a74ab64447dcc1d2144a833ed73e69dea6f

ZenithStar commented 9 months ago

By the way @ramokz, although this "works", is there a reason why this is its own separate function and not a part of the follow_distance property?

ramokz commented 8 months ago

It is due to some properties are declared in a PhantomCameraProperties.gd, where they will require a Properties. in front of the property name to be assignable directly. Which means it became a bit of a guesswork for a user as to whether if individual properties needed that or not. In some cases, they didn't even always get assigned in non-addon scripts, while working fine in addon scripts, for no discernible reason. So for the sake of consistency and to reduce the amount of bugs/questions, it has been a setter/getter only approach.

Properties will, however, be assignable directly in 0.7 due to the refactor for how they're set up.