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.

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

Make script UIDs optional #87706

Closed
wants to merge 1 commit into from
Closed

Make script UIDs optional #87706

wants to merge 1 commit into from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jan 29, 2024

This PR removes the UID code from GDScript's save() and instead the ScriptCreateDialog is now responsible for adding UIDs, which means that script no longer force adding UIDs.
image
The new checkbox is enabled by default and it's state is remembered per-project. The script is created as before and the UID is added afterwards by ResourceSaver.set_uid().

The script editor got a new File menu option to add UID to existing files:
image
Deleting a UID will now unlink it from the script, so you'll need to re-add it manually or generate a new one. I also added a method to ScriptLanguage to check whether the language supports UIDs.

There are still some bugs, so putting as draft.

@arkology
Copy link
Contributor

The script editor got a new File menu option to add UID to existing files

Also it would be nice to have ability generating uids for all scripts in project. Last time I tested, uid was generated only when saving with existing changes (saving file without making any changes did not generate uid), so algorithm was "open every script, make changes, save script"

@KoBeWi
Copy link
Member Author

KoBeWi commented Jan 29, 2024

Also it would be nice to have ability generating uids for all scripts in project.

I'm not sure about adding this option. It's only useful for old projects and you need to use it once. It can be solved with a simple editor script:

@tool
extends EditorScript

func _run() -> void:
	scan("res://")

func scan(dir: String):
	for file in DirAccess.get_files_at(dir):
		if file.get_extension() != "gd":
			continue
		
		var path := dir.path_join(file)
		if ResourceLoader.get_resource_uid(path) != ResourceUID.INVALID_ID:
			continue
		
		ResourceSaver.set_uid(path, ResourceUID.create_id())
	
	for dir2 in DirAccess.get_directories_at(dir):
		if dir2.begins_with("."):
			continue
		
		scan(dir.path_join(dir2))

...except set_uid() is not exposed. I'll fix that.

@KoBeWi KoBeWi mentioned this pull request Jan 29, 2024
@KoBeWi KoBeWi marked this pull request as ready for review January 29, 2024 23:45
@KoBeWi KoBeWi requested review from a team as code owners January 29, 2024 23:45
@KoBeWi
Copy link
Member Author

KoBeWi commented Jan 29, 2024

Updated. I fixed some weird error with modified times and added proper warnings when inserting UID. Also inserting UID if a script already has one is not allowed.

There is a weird bug where inserting UID twice will result in 2 UIDs generated; seems like set_uid() does not properly register it? Not sure how to do that.

Also UIDs were reverted, so this branch can't be rebased until they are re-added .-.

@arkology
Copy link
Contributor

It can be solved with a simple editor script
except set_uid() is not exposed. I'll fix that.

I think its enough, thanks

@dalexeev dalexeev modified the milestones: 4.3, 4.x May 31, 2024
@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 16, 2024

#97352 adds alternate script UID implementation that makes this unnecessary.

@KoBeWi KoBeWi closed this Oct 16, 2024
@KoBeWi KoBeWi deleted the newID branch October 16, 2024 12:09
@KoBeWi KoBeWi removed this from the 4.x milestone Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants