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

GD-421: Add uid path support to Scene Runner #421

Closed
mpewsey opened this issue Apr 29, 2024 · 1 comment · Fixed by #422
Closed

GD-421: Add uid path support to Scene Runner #421

mpewsey opened this issue Apr 29, 2024 · 1 comment · Fixed by #422
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@mpewsey
Copy link
Contributor

mpewsey commented Apr 29, 2024

Is your feature request related to a problem? Please describe.
Currently the Scene Runner does not support loading scenes by uid:// paths.

If a uid:// path is used, the GdUnitSceneRunner: Can't load scene by given resource path... branch from the following snippet of the Scene Runner initializer is executed:

# handle scene loading by resource path
	if typeof(p_scene) == TYPE_STRING:
		if !FileAccess.file_exists(p_scene):
			if not p_hide_push_errors:
                                # *** uid:// paths fall into this error ***
				push_error("GdUnitSceneRunner: Can't load scene by given resource path: '%s'. The resource not exists." % p_scene)
			return
		if !str(p_scene).ends_with("tscn"):
			if not p_hide_push_errors:
				push_error("GdUnitSceneRunner: The given resource: '%s'. is not a scene." % p_scene)
			return
		_current_scene = load(p_scene).instantiate()
		_scene_auto_free = true

Describe the solution you'd like
I would like to add support for uid:// paths to the Scene Runner initializer. These paths can be preferable over res:// paths since it is possible to rename project files without invalidating hard coded path references.

Describe alternatives you've considered
In GDScript, to get around this limitation, you can load the scene resource via the uid:// path, then pass that to the initializer instead. However, in C#, the ISceneRunner currently only accepts a string path as an argument. Therefore, adding uid:// path support would be especially useful for C#.

In C#, you can get around this issue by doing the following, but this requires some extra steps:

var path = "uid://cn8ucy2rheu0f"
var scene = ResourceLoader.Load<PackedScene>(path);
var runner = ISceneRunner.Load(scene.ResourcePath);

Additional context

I am willing to implement this feature and will submit a follow up pull request.

@mpewsey mpewsey added the enhancement New feature or request label Apr 29, 2024
@mpewsey mpewsey changed the title Add Uid path support to Scene Runner GD-421 Add Uid path support to Scene Runner Apr 29, 2024
@MikeSchulze
Copy link
Owner

Thank you for submitting this feature request.
I was not aware that there was this feature to load scenes by UID ;)

@MikeSchulze MikeSchulze added this to the v4.2.6 milestone Apr 29, 2024
@MikeSchulze MikeSchulze moved this to In Progress in GdUnit4 Apr 29, 2024
MikeSchulze pushed a commit that referenced this issue Apr 29, 2024
# Why
This PR adds `uid://` path support to the Scene Runner per #421.


# What
* Changed the file path exists check from using `FileAccess.file_exists`
to `ResourceLoader.exists` since the Resource Loader method works for
`uid://` paths.
* Added a check for the binary scene extension `.scn`.
* Added a new test for the `uid://` path. The test was copied from
`test_runner_by_resource_path()` but uses the `uid://` path for the test
scene instead.

Closes #421
@github-project-automation github-project-automation bot moved this from In Progress to Done in GdUnit4 Apr 29, 2024
@MikeSchulze MikeSchulze changed the title GD-421 Add Uid path support to Scene Runner GD-421: Add uid path support to Scene Runner Apr 29, 2024
@MikeSchulze MikeSchulze modified the milestones: v4.2.6, v4.3.0 May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants