Skip to main content
added 376 characters in body
Source Link
Theraot
  • 27.5k
  • 4
  • 52
  • 79

Thus, first we need to somehow sort out what properties do an Spatial have.

Addendum: Using ClassDB.class_get_property_list is a better way to do this (so we don't have to create a node to ask it about its properties and we don't need to memoize the results). However, I'm keeping my old solution for reference.

This is the solution I came up with:

 
 

Thus, first we need to somehow sort out what properties do an Spatial have. This is the solution I came up with:

Thus, first we need to somehow sort out what properties do an Spatial have.

Addendum: Using ClassDB.class_get_property_list is a better way to do this (so we don't have to create a node to ask it about its properties and we don't need to memoize the results). However, I'm keeping my old solution for reference.

This is the solution I came up with:

 
 
added 5244 characters in body
Source Link
Theraot
  • 27.5k
  • 4
  • 52
  • 79

Addendum on subclassing Camera.

One of the problem I ran on while testing the above solution was that I was delegating some properties that I shouldn't. Which have led to the question: Can we intercept a property? And I don't mean to override it, much less remove it. The property must remain there, and still work, so it does not break any functionality.

As you know, if we do this:

extends Camera

var transform:Transform

We get an error. That does not work.

But… We can use _get and _set!

The first part to figure out was what to intercept. So I set a _get and _set that would print whatever property is being access. And I found a couple things:

  • Moving the node from the spatial gizmo avoid _get and _set entirely. Will deal with that later.
  • Moving the node from the inspector panel will interact with "scale", "translation" and "rotation_degrees"

So we will intercept those, and of course "transform" too. That is something like this:

tool extends Camera

var _base_transform:Transform

func _get(property: String):
    match property:
        "transform":
            return self._base_transform
        "scale":
            return self._base_transform.basis.get_scale()
        "translation":
            return self._base_transform.origin
        "rotation_degrees":
            return self._base_transform.basis.get_euler() * 180/PI
        _:
            return null

func _set(property: String, value) -> bool:
    match property:
        "transform":
            self._base_transform = value
            return true
        "scale":
            var quat = self._base_transform.basis.get_rotation_quat()
            self._base_transform.basis = Basis(quat).scaled(value)
            return true
        "translation":
            self._base_transform.origin = value
            return true
        "rotation_degrees":
            var scale = self._base_transform.basis.get_scale()
            self._base_transform.basis = Basis(Quat(value * PI/180)).scaled(scale)
            return true
        _:
            return false

Those self will become relevant soon.

With this code, when you modify the transform from the inspector panel, you are actually editing this new _base_transform variable.

Now, we want to reintroduce our orbit properties. These look the same:

export(float) var orbit_distance:float setget set_orbit_distance
export(float, -180, 180) var orbit_yaw:float setget set_orbit_yaw
export(float, -180, 180) var orbit_pitch:float setget set_orbit_pitch
export(float, -180, 180) var orbit_roll:float setget set_orbit_roll

func set_orbit_distance(new_value:float) -> void:
    orbit_distance = new_value
    update_transform()

func set_orbit_yaw(new_value:float) -> void:
    orbit_yaw = new_value
    update_transform()

func set_orbit_pitch(new_value:float) -> void:
    orbit_pitch = new_value
    update_transform()

func set_orbit_roll(new_value:float) -> void:
    orbit_roll = new_value
    update_transform()

But we will change update_transform to this:

func get_relative_transform() -> Transform:
    return Transform.IDENTITY.rotated(Vector3.UP, deg2rad(orbit_yaw)) \
    * Transform.IDENTITY.rotated(Vector3.RIGHT, deg2rad(orbit_pitch)) \
    * Transform.IDENTITY.rotated(Vector3.FORWARD, deg2rad(orbit_roll)) \
    * Transform.IDENTITY.translated(Vector3.BACK * orbit_distance)

func update_transform() -> void:
    transform = _base_transform * get_relative_transform()

That is, I extracted get_relative_transform from update_transform. And also, I'm not dealing with a child Camera, since the Camera is the Node we are working on.

Why?

Well, as I said, when we move the Node using the spatial gizmo, that avoids _get and _set. We need changes to _base_transform to become changes to transform and changes to transform to become changes to _base_transform. We accomplish this by giving a getter and a setter to _base_transform:

var _base_transform:Transform setget set_base_transform, get_base_transform

func set_base_transform(new_value:Transform) -> void:
    _base_transform = new_value
    update_transform()

func get_base_transform() -> Transform:
    _base_transform = transform * get_relative_transform().affine_inverse()
    return _base_transform

And there we are using get_relative_transform() which is why we extracted it from update_transform.

Also, the reason why I wrote access to _base_transform with self is to make sure it called the getter and setter.

There is just one more thing to do: make sure _base_transform gets persisted. We could do it by using export. However, to not have it appear in the inspector panel, we use _get_property_list, like this:

func _get_property_list() -> Array:
    var properties = []
    properties.append({
        name = "_base_transform",
        type = TYPE_TRANSFORM,
        usage = PROPERTY_USAGE_STORAGE
    })
    return properties

I believe this approach would benefit from yet another spatial gizmo, one that eases setting the point around the camera orbits. And again, I don't know enough about gizmos to about implementing that. The approach I would follow to use this node without such gizmo is to set the orbit properties at zero, position the camera at the point it will orbit around, and then edit the orbit properties to what you want.


Addendum on subclassing Camera.

One of the problem I ran on while testing the above solution was that I was delegating some properties that I shouldn't. Which have led to the question: Can we intercept a property? And I don't mean to override it, much less remove it. The property must remain there, and still work, so it does not break any functionality.

As you know, if we do this:

extends Camera

var transform:Transform

We get an error. That does not work.

But… We can use _get and _set!

The first part to figure out was what to intercept. So I set a _get and _set that would print whatever property is being access. And I found a couple things:

  • Moving the node from the spatial gizmo avoid _get and _set entirely. Will deal with that later.
  • Moving the node from the inspector panel will interact with "scale", "translation" and "rotation_degrees"

So we will intercept those, and of course "transform" too. That is something like this:

tool extends Camera

var _base_transform:Transform

func _get(property: String):
    match property:
        "transform":
            return self._base_transform
        "scale":
            return self._base_transform.basis.get_scale()
        "translation":
            return self._base_transform.origin
        "rotation_degrees":
            return self._base_transform.basis.get_euler() * 180/PI
        _:
            return null

func _set(property: String, value) -> bool:
    match property:
        "transform":
            self._base_transform = value
            return true
        "scale":
            var quat = self._base_transform.basis.get_rotation_quat()
            self._base_transform.basis = Basis(quat).scaled(value)
            return true
        "translation":
            self._base_transform.origin = value
            return true
        "rotation_degrees":
            var scale = self._base_transform.basis.get_scale()
            self._base_transform.basis = Basis(Quat(value * PI/180)).scaled(scale)
            return true
        _:
            return false

Those self will become relevant soon.

With this code, when you modify the transform from the inspector panel, you are actually editing this new _base_transform variable.

Now, we want to reintroduce our orbit properties. These look the same:

export(float) var orbit_distance:float setget set_orbit_distance
export(float, -180, 180) var orbit_yaw:float setget set_orbit_yaw
export(float, -180, 180) var orbit_pitch:float setget set_orbit_pitch
export(float, -180, 180) var orbit_roll:float setget set_orbit_roll

func set_orbit_distance(new_value:float) -> void:
    orbit_distance = new_value
    update_transform()

func set_orbit_yaw(new_value:float) -> void:
    orbit_yaw = new_value
    update_transform()

func set_orbit_pitch(new_value:float) -> void:
    orbit_pitch = new_value
    update_transform()

func set_orbit_roll(new_value:float) -> void:
    orbit_roll = new_value
    update_transform()

But we will change update_transform to this:

func get_relative_transform() -> Transform:
    return Transform.IDENTITY.rotated(Vector3.UP, deg2rad(orbit_yaw)) \
    * Transform.IDENTITY.rotated(Vector3.RIGHT, deg2rad(orbit_pitch)) \
    * Transform.IDENTITY.rotated(Vector3.FORWARD, deg2rad(orbit_roll)) \
    * Transform.IDENTITY.translated(Vector3.BACK * orbit_distance)

func update_transform() -> void:
    transform = _base_transform * get_relative_transform()

That is, I extracted get_relative_transform from update_transform. And also, I'm not dealing with a child Camera, since the Camera is the Node we are working on.

Why?

Well, as I said, when we move the Node using the spatial gizmo, that avoids _get and _set. We need changes to _base_transform to become changes to transform and changes to transform to become changes to _base_transform. We accomplish this by giving a getter and a setter to _base_transform:

var _base_transform:Transform setget set_base_transform, get_base_transform

func set_base_transform(new_value:Transform) -> void:
    _base_transform = new_value
    update_transform()

func get_base_transform() -> Transform:
    _base_transform = transform * get_relative_transform().affine_inverse()
    return _base_transform

And there we are using get_relative_transform() which is why we extracted it from update_transform.

Also, the reason why I wrote access to _base_transform with self is to make sure it called the getter and setter.

There is just one more thing to do: make sure _base_transform gets persisted. We could do it by using export. However, to not have it appear in the inspector panel, we use _get_property_list, like this:

func _get_property_list() -> Array:
    var properties = []
    properties.append({
        name = "_base_transform",
        type = TYPE_TRANSFORM,
        usage = PROPERTY_USAGE_STORAGE
    })
    return properties

I believe this approach would benefit from yet another spatial gizmo, one that eases setting the point around the camera orbits. And again, I don't know enough about gizmos to about implementing that. The approach I would follow to use this node without such gizmo is to set the orbit properties at zero, position the camera at the point it will orbit around, and then edit the orbit properties to what you want.

Source Link
Theraot
  • 27.5k
  • 4
  • 52
  • 79

First of all, I want to say, I sympathize. I have gone down the path of making a custom variation of a Node by composition and turning it into a scene and then having to rely on reaching deep into the structure to use it. And I have also implemented property delegates for ease of use.

The current Godot response is Editable Children. You can instance a scene with editable children, and this will allow you to overwrite the properties of the children on the editor.

There have also been some proposals. For example: Automate delegation of exported properties to internal nodes to improve GDScript abstractions.


but that feels clunky and prone to breaking changes when new methods are added to the Camera class.

I submit to you that the engine will not change behind your back.

If you are making a game, you decide if and when you upgrade to a new version of the engine, at which point you can update your class.

If you are making an addon or library, it is expected that you would release a new version if there are significant changes to the engine.

And you might even have a tool generate the code for your custom class.

However, I understand this is not ideal.


On the issue with what you want…

You could talk directly to VisualServer to create a low level camera (which is what the Camera node does behind the scenes). Or you could simply create the Camera node from code. But the node that does it would not be a Camera. So when you get it with get_node, what you get is not a Camera.

That leads you to either delegate the properties of a Camera… Or extend Camera except its Transform will not longer be the position of the camera, instead the position would be based on that Transform plus some other Transform that describe its motion around a point. That is, you want to break Liskov substitution principle.

The correct approach, is not to make a Camera, but a thing that has a Camera, and there is a perfectly reasonable to do that in engine: A Spatial with a Camera. And there is a perfectly reasonable way to reuse it: a scene. We are not going to change that.

So, we need a workaround get_node to get a Camera, and we need a solution to delegate the properties.


Before we go into that, I want to suggest an alternative approach. Consider that sometimes you might want to orbit something around something else… And have that first something not be a Camera. It is possible to create a custom Node that extends Spatial, that has the logic to orbit around something else (perhaps taking advantage of a NodePath). You can conceptualize it as a Joint, or you can conceptualize it a RemoteTransform.

In fact, it might be useful to have different controls for the Camera (e.g. first person, third person orbiting around the player, looking from a fixed position in a room, etc) and interpolate between them with an InterpolatedCamera. In fact, you don't necessarily need custom Cameras for those cases, you just need Spatials to serve as targets for the InterpolatedCamera.


Not convinced? Ok...

All problems in computer science can be solved by another level of indirection.

-- Attributed to Butler Lampson

So, we have Cameras, and we have Camera-like things. Let us call them CameraHolder. We are going to make a helper that gives you a Camera regardless if you give it a Camera or a CameraHolder. Easy:

class_name CameraGetter extends Object

static func from(node:Node) -> Camera:
    var camera_holder:CameraHolder = node as CameraHolder
    var camera:Camera

    if is_instance_valid(camera_holder)
        camera = camera_holder.get_camera()
    else:
        camera = node as Camera

    if is_instance_valid(camera)
        return camera

    return null

And now you can say CameraGetter.from(get_node("CameraPivot")) and it will give you a Camera.

We of course need to write the class CameraHolder. It must have a get_camera method. And we want to be able to use as if it were a Camera, in particular we want to appear to have all the properties of a Camera.

And that last part is tricky. This is the outline of what we will use:

tool class_name CameraHolder extends Spatial

func get_camera() -> Camera:
    return null

func _get_property_list() -> Array:
    return []

func _get(property: String):
    return null

func _set(property: String, value) -> bool:
    return false

First thing to notice is that there is a method get_camera, which in this case returns null. We will make everything else will refer to that get_camera method, so that a class that extends this one can simply override it.

Second, we are using _get_property_list. Godot will call get_property_list (which will call _get_property_list behind the scenes) to figure out what properties the Node exposes. In particular _get_property_list must return what additional properties the Node exposes. With _get_property_list you can do what you would do with export and much more.

The idea is to report all the properties of a Camera, and we can get one of those from get_camera, so we will use that to get its properties. We don't want to tell the editor to persist these properties, because we will delegate them to a Camera node that can be persisted instead. I will come back to that.

Then we have _get and _set, which will handle the case when we set properties that the Node does not have. However, you need to know that Godot will call them first, and only when they report that the property does not exist, then it will go to an actual property.

This is a problem, because we cannot rely on whether or not the Camera has a property, because our Node is also a Spatial and we don't want to delegate the its properties.

In fact, one of the properties is the script, and I spent a lot of time figuring out why the script was disappearing. It was because I was delegating the script property to a Camera that didn't have a script.

This problem is further complicated by the fact that calling get from _get will result in a stack overflow. Similarly calling get_property_list from _get_property_list is a stack overflow. And Godot will crash.

Thus, first we need to somehow sort out what properties do an Spatial have. This is the solution I came up with:

var _spatial_property_names:Array = []

func _get_spatial_property_names() -> Array:
    if _spatial_property_names.size() == 0:
        var spatial = Spatial.new()
        var spatial_properties = spatial.get_property_list()
        for property in spatial_properties:
            if property.type == TYPE_NIL:
                continue

            _spatial_property_names.append(property.name)

        spatial.free()
    
    return _spatial_property_names

That is, we are going to create a new Spatial, and ask it what properties does it have. We also skip TYPE_NIL, because these are used to communicate property categories to the editor. They are not real properties. And, of course, also memoize the result in an array for future reference. We don't want to make a node every time we need this.

Now, we can build our property list like this:

var _camera_properties:Array = []

func _get_property_list() -> Array:
    var spatial_property_names = _get_spatial_property_names()
    if _camera_properties.size() == 0:
        var camera:Camera = get_camera()
        if is_instance_valid(camera):
            var properties = camera.get_property_list()
            for property in properties:
                if !spatial_property_names.has(property.name):
                    property.usage = property.usage & ~PROPERTY_USAGE_STORAGE 
                    _camera_properties.append(property)

    return _camera_properties

As you can see, if a property is in the list of properties that Spatial has, it is not an additional property. It would be a duplicate. And we don't report it.

For those that we report, we make sure they don't have the flag PROPERTY_USAGE_STORAGE set, so the editor will not persist them. We, of course, want to let the editor persist the Spatial properties, which we skipped.

And we are also memoizing this result.

Finally, these are _get and _set:

func _get(property: String):
    var spatial_property_names = _get_spatial_property_names()
    if spatial_property_names.has(property):
        return null

    var camera:Camera = get_camera()
    if is_instance_valid(camera):
        return camera.get(property)

    return null

func _set(property: String, value) -> bool:
    var spatial_property_names = _get_spatial_property_names()
    if spatial_property_names.has(property):
        return false

    var camera:Camera = get_camera()
    if is_instance_valid(camera) and property in camera:
        get_camera().set(property, value)
        return true

    return false

As you would expect, we rely on _get_spatial_property_names to know what property to avoid delegating to the Camera.

Ah, and get_camera can keep returning null. It is ok. We are checking with is_instance_valid everywhere we use it.

Now, we can extend that class to implement CameraPivot:

tool class_name CameraPivot extends CameraHolder

func get_camera() -> Camera:
    if is_inside_tree():
        return get_node_or_null("Camera") as Camera

    return null

Here I'm being extra careful because I know Godot will call this in _get_property_list. So I do not to call get_node which can result in an error, I call get_node_or_null. And I do not call get_node_or_null if the Node is not in the scene tree.

And this is my implementation for the orbit logic:

export(float) var orbit_distance:float setget set_orbit_distance
export(float, -180, 180) var orbit_yaw:float setget set_orbit_yaw
export(float, -180, 180) var orbit_pitch:float setget set_orbit_pitch
export(float, -180, 180) var orbit_roll:float setget set_orbit_roll

func set_orbit_distance(new_value:float) -> void:
    orbit_distance = new_value
    update_transform()

func set_orbit_yaw(new_value:float) -> void:
    orbit_yaw = new_value
    update_transform()

func set_orbit_pitch(new_value:float) -> void:
    orbit_pitch = new_value
    update_transform()

func set_orbit_roll(new_value:float) -> void:
    orbit_roll = new_value
    update_transform()

func update_transform() -> void:
    var camera:Camera = get_camera()
    if !is_instance_valid(camera):
        return

    camera.transform = \
          Transform.IDENTITY.rotated(Vector3.UP, deg2rad(orbit_yaw)) \
        * Transform.IDENTITY.rotated(Vector3.RIGHT, deg2rad(orbit_pitch)) \
        * Transform.IDENTITY.rotated(Vector3.FORWARD, deg2rad(orbit_roll)) \
        * Transform.IDENTITY.translated(Vector3.BACK * orbit_distance)

You don't have to do it this way, of course. You can extend CameraHolder and implement get_camera to get you the Camera from wherever you have it, and add whatever properties and methods make sense to you.

The only thing missing is that the "Preview" toggle will not appear for out CameraHolder or our CameraPivot. I suppose this can addressed with a custom gizmo (which is a type of plugin). However, I haven't delved into that.