Fix issue629 #630
Fix issue629 #630
Conversation
In order to add an animation, AnimationPlayer should get its AnimationLibrary with empty name "".
The user should add a child Node3D and set it as the property of XRToolsStaging in order to be used to contain the current scene. Fix issue GodotVR#628
|
The hand change looks good, but if you want to do any changes to staging; could you do that in a separate PR. Ah, i see - yes dependent changes. Never mind. |
|
|
||
| ## If true, the player is prompted to continue | ||
| @export var prompt_for_continue : bool = true | ||
|
|
||
| ## Scene container file as a Node3D used as | ||
| ## the parent of the current scene. | ||
| @export var scene_container : Node3D: |
There was a problem hiding this comment.
This will break existing projects that use the existing Scene node. I would add some code in _ready that if scene_container is not assigned, that it checks for the Scene node and assigns it. That ensures backwards compatibility.
| @@ -269,12 +284,12 @@ func load_scene(p_scene_path : String, user_data = null) -> void: | |||
| ## unless hidden. | |||
| func set_fade(p_value : float): | |||
| if p_value == 0.0: | |||
| $Fade.visible = false | |||
| fade.visible = false | |||
There was a problem hiding this comment.
I think you lost a change here. The variable fade is not defined anywhere or set anywhere.
|
It would be good to have more of an explanation in your description on why you are changing this. The staging system is meant to be used with the staging scene, which is a scene you can inherit if there is more that you want to add in. This just introduces more work for a user to set things up that should already be there. So without knowing the justification for this change, it breaks the "keep things simple" rule. |
The comunity provides lot of tutorials, most of them helps you to "walk" for each step, adding by hand each node. This is good to know better how to make your own stuff. After following that tutorials, make lot of mistakes and found this missing, underdocumented things like Fade. So if the user wants to do it by hand, this should be better to be added as a warning. Yes there are more steps here, but that is the price to do it by hand. May is better to document, that the user should instantiate the staging scene, instead to create all step by step. |
XRToolsStaging requires a MeshInstance3D to hold a transition fade. This adds a new property so the user is warned to add this node to staging. Fix issue GodotVR#629
This should be merged only if #627 is fixed first, as this depends on it