Bind Node methods for getting its SceneState

[mihe]

Closes godotengine/godot-proposals#12080.

This adds Node::get_scene_instance_state and Node::get_scene_inherited_state to the script bindings, for the reasons laid out in the proposal linked above.

Node::get_scene_inherited_state is of course somewhat redundant, given the newly exposed SceneState.get_base_scene_state, but the two seemed to go hand-in-hand, so I figured why not.

[KoBeWi]

Node::get_scene_inherited_state is of course somewhat redundant, given the newly exposed SceneState.get_base_scene_state, but the two seemed to go hand-in-hand, so I figured why not.

Because we shouldn't expose things on a whim if they are not really needed.
There is already some redundancy with get_scene_instance_state().get_path() vs scene_file_path.

[Mickeon]

I have already voiced concerns of exposing things willy-nilly in the past. For the purposes described in the proposal, I think these fit the bill.
"Exposed to the API" implies a degree of maintainability. Things have to be functional on a lot more cases, because you don't know what users are going to do with them. They also have to be properly documented, ensure users do not misuse them.
But, most important to me personally, they should not be utilities barely worth using. By bloating the class reference and code suggestions, these can be more confusing than one may think.
Furthermore, it makes their implementation stricter. To ensure compatibility, these methods kinda cannot functionally change anymore (whether it be for performance, or for any other reason).

Granted, it's not much here, but by accepting these PRs for the sake of it, that additional baggage grows. At least, that's how I've felt, looking at this happening for a while.

Regarding the PR itself, I can somewhat foresee a confusion between PackedScene and SceneState. For example, users wondering why they can't just my_node.get_scene_instance_state.instantiate(). But that is also due to the lacking documentation making the distinction between the two.

[AThousandShips]

I think the critical question here is:
Does this work for, for example, change_scene_to_packed with a non file based scene? Like one loaded via other means

I.e. does this make you able to fetch this data for cases when you can't do so via other means, like loading the scene file

If the answer is no I'd say this is redundant and shouldn't be exposed for the reasons given above

If the answer is yes that's a different story

[KoBeWi]

When an instance exists in a scene, the PackedScene is not loaded, but the SceneState exists. Currently you need PackedScene to obtain SceneState, which means you need to load the scene (which can potentially take a few seconds if it's big). get_scene_instance_state () allows to fetch SceneState directly from the Node, so at most it saves loading time. It doesn't do something that was completely impossible.

I think that's kind of fine, but get_scene_inherited_state() is redundant, as already noted.

[AThousandShips]

So to clarify, if I do for example construct a PackedScene from some plugin or other means, packing it from a tree for example, and then use that, and then drop that scene, will this give access to that state?

For example:

var my_tree := generate_some_node_tree() # This is some function that generates my tree for me, like a level.
var my_packed_scene := PackedScene.new()
my_packed_scene.pack(my_tree)

... # We do something with this, perhaps we pick one randomly or something.

var my_child = my_packed_scene.instantiate()
add_child(my_child)

my_packed_scene = null # For some reason we drop this.

...

var my_state := my_child.get_scene_instance_state() # For some reason we want this.

Because in this case we have no other way of getting this, if it even matters

What I'm getting at is that if this is a thing we can't do otherwise this is less of a workaround or convenience and more of a genuine necessary feature

[KoBeWi]

Right now you can do var my_state: SceneState = load(my_child.scene_file_path).get_state(). The drawback is that it (potentially) loads the PackedScene again and doesn't work with type inference.

[AThousandShips]

That doesn't work if the scene isn't from a file, does it?

Check my example again please, does this work for scenes without paths

[KoBeWi]

Ah yes, the scene needs to be a file.
Although that's a made up use-case, the proposal only mentions scenes that are files.

[AThousandShips]

It's a made up case in this case because I came up with it on the spot, but it's presented as a case where this method isn't redundant, assuming it does work for such a case

I'd say that if evaluated from strictly not wanting to have to fetch the state again this is more clutter than it's worth IMO, but if we consider wanting to fetch the state from non-file based sources is relevant it has a value