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.

Sign up for GitHub

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

Expose scene unique id functionality in Resource #88111

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Expose scene unique id functionality in Resource #88111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions core/io/resource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,16 @@ String Resource::generate_scene_unique_id() {
}

void Resource::set_scene_unique_id(const String &p_id) {
bool is_valid = true;
for (int i = 0; i < p_id.length(); i++) {
if (!is_ascii_identifier_char(p_id[i])) {
is_valid = false;
scene_unique_id = Resource::generate_scene_unique_id();
break;
}
}

ERR_FAIL_COND_MSG(!is_valid, "The scene unique ID must contain only letters, numbers, and underscores.");
scene_unique_id = p_id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an escape we can do that will let us use arbitrary strings but avoid breaking the scene?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I tested.

@tool
extends Node

@onready var world_environment: WorldEnvironment = $WorldEnvironment

@export var run: bool:
	set(v):
		if !is_inside_tree(): return
		world_environment.environment.resource_scene_unique_id = "123\""

Which resulted in this, and broke the scene (notice the duplicate quotes).

[gd_scene load_steps=3 format=3 uid="uid://ifg8x7gf74rm"]

[ext_resource type="Script" path="res://mydscript2.gd" id="1_pk36j"]

[sub_resource type="Environment" id="123""]

[node name="Myscene" type="Node"]
script = ExtResource("1_pk36j")

[node name="WorldEnvironment" type="WorldEnvironment" parent="."]
environment = SubResource("123"")

If you make it use a \n then it will look like this (this one wont break it from loading, but it will still look weird)

[gd_scene load_steps=3 format=3 uid="uid://ifg8x7gf74rm"]

[ext_resource type="Script" path="res://mydscript2.gd" id="1_pk36j"]

[sub_resource type="Environment" id="123
"]

[node name="Myscene" type="Node"]
script = ExtResource("1_pk36j")

[node name="WorldEnvironment" type="WorldEnvironment" parent="."]
environment = SubResource("123
")

I can't really answer your original question though, because the way the scene is saved is out of my domain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do reiterate my original concern in #88111 (comment). That is, the method exposed to outside developers should not be able to brick scenes this way (see comment for more).

}

Expand Down Expand Up @@ -532,6 +542,10 @@ void Resource::_bind_methods() {
ClassDB::bind_method(D_METHOD("get_local_scene"), &Resource::get_local_scene);
ClassDB::bind_method(D_METHOD("setup_local_to_scene"), &Resource::setup_local_to_scene);

ClassDB::bind_static_method("Resource", D_METHOD("generate_scene_unique_id"), &Resource::generate_scene_unique_id);
ClassDB::bind_method(D_METHOD("set_scene_unique_id", "id"), &Resource::set_scene_unique_id);
ClassDB::bind_method(D_METHOD("get_scene_unique_id"), &Resource::get_scene_unique_id);

ClassDB::bind_method(D_METHOD("emit_changed"), &Resource::emit_changed);

ClassDB::bind_method(D_METHOD("duplicate", "subresources"), &Resource::duplicate, DEFVAL(false));
Expand All @@ -542,6 +556,7 @@ void Resource::_bind_methods() {
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "resource_local_to_scene"), "set_local_to_scene", "is_local_to_scene");
ADD_PROPERTY(PropertyInfo(Variant::STRING, "resource_path", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_EDITOR), "set_path", "get_path");
ADD_PROPERTY(PropertyInfo(Variant::STRING, "resource_name"), "set_name", "get_name");
ADD_PROPERTY(PropertyInfo(Variant::STRING, "resource_scene_unique_id", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NONE), "set_scene_unique_id", "get_scene_unique_id");

MethodInfo get_rid_bind("_get_rid");
get_rid_bind.return_val.type = Variant::RID;
Expand Down
12 changes: 12 additions & 0 deletions doc/classes/Resource.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@
[/codeblock]
</description>
</method>
<method name="generate_scene_unique_id" qualifiers="static">
<return type="String" />
<description>
Generates a unique identifier for a resource to be contained inside a [PackedScene], based on the current date, time, and a random value. The returned string is only composed of letters ([code]a[/code] to [code]y[/code]) and numbers ([code]0[/code] to [code]8[/code]). See also [member resource_scene_unique_id].
</description>
</method>
<method name="get_local_scene" qualifiers="const">
<return type="Node" />
<description>
Expand Down Expand Up @@ -98,6 +104,12 @@
The unique path to this resource. If it has been saved to disk, the value will be its filepath. If the resource is exclusively contained within a scene, the value will be the [PackedScene]'s filepath, followed by a unique identifier.
[b]Note:[/b] Setting this property manually may fail if a resource with the same path has already been previously loaded. If necessary, use [method take_over_path].
</member>
<member name="resource_scene_unique_id" type="String" setter="set_scene_unique_id" getter="get_scene_unique_id">
An unique identifier relative to the this resource's scene. If left empty, the ID is automatically generated when this resource is saved inside a [PackedScene]. If the resource is not inside a scene, this property is empty by default.
[b]Note:[/b] When the [PackedScene] is saved, if multiple resources in the same scene use the same ID, only the earliest resource in the scene hierarchy keeps the original ID. The other resources are assigned new IDs from [method generate_scene_unique_id].
[b]Note:[/b] Setting this property does not emit the [signal changed] signal.
[b]Warning:[/b] When setting, the ID must only consist of letters, numbers, and underscores. Otherwise, it will fail and default to a randomly generated ID.
</member>
</members>
<signals>
<signal name="changed">
Expand Down
2 changes: 1 addition & 1 deletion editor/doc_tools.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ void DocTools::generate(BitField<GenerateFlags> p_flags) {
}

if (properties_from_instance) {
if (E.name == "resource_local_to_scene" || E.name == "resource_name" || E.name == "resource_path" || E.name == "script") {
if (E.name == "resource_local_to_scene" || E.name == "resource_name" || E.name == "resource_path" || E.name == "script" || E.name == "resource_scene_unique_id") {
// Don't include spurious properties from Object property list.
continue;
}
Expand Down
Loading