godot
godot copied to clipboard
godotengine
Added get_open_scenes_roots API to Editor Interface to retrieve all opened scenes root nodes
Added a single API get_open_scenes_roots to EditorInterface to retrieve all opened scenes root nodes instead of scene file paths.
- Changes applied to editor_interface.h and editor_interface.cpp
- Documentation added to EditorInterface.xml
Thanks for contribution. Please squash the commits as it's required by our pipeline (see https://docs.godotengine.org/en/latest/contributing/workflow/pr_workflow.html).
@Chaosus Done, I'm not sure if I did it correctly, This was my first time :D Let me know if ther's still a problem.
@Calinou Sorry to ping, Can we get this PR merged? It's already merged to other forks of Godot, I want to use jenova framework on official beta builds of 4.4
PR has no conflict and it's very easy to verify as it is a clone of same function just with different return type.
Thanks š§”
Sorry to ping, Can we get this PR merged?
We're currently in feature freeze for Godot 4.4, so we're not merging any new feature for the time being, even minor ones like this.
This still needs to be approved by contributors familiar with the EditorInterface API (cc @KoBeWi @passivestar ) and then it should be mergeable for 4.5.
What's the use-case for this method? The implementation is fine (though the method should return TypedArray<Node>), but there is no proposal.
What's the use-case for this method? The implementation is fine (though the method should return
TypedArray<Node>), but there is no proposal.
@KoBeWi It's required for Jenova Fremework hot-reload feature, when we want to get opened scenes as actual instances instead of their name to iterate over nodes in open scenes, collect, restore etc.
I added this PR on Oct 5, 2024 it went to 4.4 milestone and now it's on 4.5! Blazium and Redot forks both merged while it's dusting in PR and this is unacceptable. Jenova is one of the biggest things happened to godot and people who want to use it are forced to use an alternative distro.
There is proposal : https://github.com/godotengine/godot-proposals/issues/10895 It's just sad this came from 2020 to now and still not merged :)
Seems fine as far as API goes to have this in addition to the already existing get_edited_scene_root() method.
It's hard for me to understand the practical use-case though because I'm not familiar with the framework that you mentioned. Why does it need to hot-reload scenes in inactive tabs? Wouldn't it be better to do it lazily when a scene becomes active (i.e there's a EditorPlugin.scene_changed signal)?
Seems fine as far as API goes to have this in addition to the already existing
get_edited_scene_root()method.It's hard for me to understand the practical use-case though because I'm not familiar with the framework that you mentioned. Why does it need to hot-reload scenes in inactive tabs? Wouldn't it be better to do it lazily when a scene becomes active (i.e there's a
EditorPlugin.scene_changedsignal)?
@passivestar I'm not the developer so i can't explain in detail but since the project brings incredible c++ scripting and hot-reloading, it require to update C++ nested nodes in opened scenes if it doesn't it lead to a crash as those pointers are gone after hot reload.
This is video from their channel showing hot-reloading: https://github.com/user-attachments/assets/5522afe3-309e-47cf-9800-1f566bf2db35
Well, currently there is no way to get a scene root of another opened scene; you can only get the current scene. Adding a method that allows fetching other scenes probably makes sense š¤
@KoBeWi @passivestar Thanks guys, I know you're in feature freeze, but Iād really appreciate it if you could make this happen in 4.4
Congrats devs! jenova framework dropped official support of godot and moved to blazium... you guys are truly game changer :)
@KoBeWi @akien-mga Can we get this PR merged now? Many people are waiting for this to switch to new version and it's been 5 months... :)
It needs a rebase to pass CI, the macOS builds failed (the logs aren't available anymore so I assume this was a temporary CI fluke and not a bug in this PR).
@KoBeWi's suggestion to use TypedArray<Node> as return value is also good and should be included.
It needs a rebase to pass CI, the macOS builds failed (the logs aren't available anymore so I assume this was a temporary CI fluke and not a bug in this PR).
@KoBeWi's suggestion to use
TypedArray<Node>as return value is also good and should be included.
Yes, It first passed all checks and bugged out on second commit I did. Other forks that merged this change have successful build on all platforms. I agree with TypedArray<Node> as well, Btw, Should I do anything from my side?
Well as I mentioned:
- You should rebase the PR on current
master - You should amend the commit to use
TypedArray<Node>
But I guess I'll do it myself and tweak the commit message for clarity along the way.
Well as I mentioned:
* You should rebase the PR on current `master` * You should amend the commit to use `TypedArray<Node>`But I guess I'll do it myself and tweak the commit message for clarity along the way.
Awesome, looks like everything works as expected. I tested the binaries from action and they all working.
@akien-mga @AThousandShips Can this PR be merged? Everything looks good and they just added compatibility for new Godot release and I want to migrate from 4.3 soon as possible...
Please have some patience, we just released 4.4-stable and we're focusing on merging bug fixes right now and preparing a 4.4.1 bugfix release. This will be merged soon, it's slated for 4.5-dev1. If you want to use it, you can merge it in your own local branch, you'd need to do compile from source anyway once it's merged.
