Can't preload resources #360
Comments
How do you imagine this to work? The Godot engine is not available at gdext compile time (see also godot-rust/gdnative#400). At startup/registration time it's possible, but what would the difference between The syntax for easy loading has been suggested before (godot-rust/gdnative#980) and is probably a separate discussion from load/preload 🙂 |
Bromeon
added
feature
|
I think as @Bromeon said, we need a syntax for easy loading more than preloading. But with all the introduction, if someone really needs the functionality, a build script may be the answer. Disclaimer! I still think interactive loading may be a better approach in most of cases! Now considering the easy loading matter: |
|
Thanks for the input!
A big issue is that Godot is currently not required for compiling a library with gdext (this is to allow clean programming against a given Godot API, independent of installed/runtime versions). And we would need the engine to parse resource files. But as you say, I haven't heard a good argument for preloading in Rust that goes beyond "GDScript has it".
Background means separate thread. While nice from a UX perspective, it needs a bit of thinking because resources are not thread-safe during loading (see thread-safety guidelines). It needs even further thought regarding how to map this soundly to Rust's threading/safety model.
I think it's good here, as it's a general discussion about loading ergonomics. Feel free to post examples. I can adjust the issue title later if needed. Also, please read existing discussions I linked on the topic. We have a tendency to reinvent wheels in gdext 😉 |
I didn't look at it that fancy! The build script would only create one
Yes sure! I suggested it as a general design advice, not using Rust GDExt necessarily. |
|
Regarding the easy loading, I think a little syntax sugar on top of what already exists can help with ergonomics. #[derive(GodotClass)]
#[class(init, base=Node)]
pub struct Catcher {
#[base]
base: Base<Node>,
#[catch(res="res://resources/fish.svg", hint="ImageTexture", cache="CACHE_MODE_REUSE")]
fish: Gd<Texture2D>,
}Which expands as if we had implemented it ourselves: fn init(base: Base<Control>) -> Self {
let fish = {
godot::engine::ResourceLoader::singleton()
.load_ex("res://resources/fish.svg".into())
.type_hint("ImageTexture".into())
.cache_mode(godot::engine::resource_loader::CacheMode::CACHE_MODE_REUSE)
.done()
.expect("required `ImageTexture` resource from 'resources/fish.svg'")
.cast::<Texture2D>()
};
// ...
Self {
base,
fish
}
}And if it was In terms of getting reference to the other nodes, I guess, things get a little tricky, because I'm not sure if there is a safe way to access scene tree in that stage. I suggest either let #[derive(GodotClass)]
#[class(init, base=Node)]
pub struct Parent {
#[base]
base: Base<Node>,
#[fetch("./foo/bar")] // or even "%global_node"
child: Option<Gd<Control>>,
}It will expand as if we had implemented this ourselves: #[godot_api]
impl Parent {
#[func]
fn _fetch_on_ready(&mut self) {
self.child = Some( self.base.get_node_as::<Control>("./foo/bar") );
}
}
#[godot_api]
impl NodeVirtual for Parent {
fn init(mut base: Base<Node>) -> Self {
let base_ref = base.share();
base.connect(
"ready".into(),
Callable::from_object_method(base_ref, "_fetch_on_ready")
);
// ...
Self {
base,
child: None,
}
}
}These are for convenience of course. The #[derive(GodotClass)]
#[class(init, base=Node)]
pub struct Parent {
#[base]
base: Base<Node>,
#[add(use_bbcode=true, text="[color=yellow]crab![/color]")]
child: Gd<RichTextLabel>,
}Which expands to: fn init(mut base: Base<Node>) -> Self {
let mut child = RichTextLabel::new_alloc();
child.set("use_bbcode".into(), Variant::from(true));
child.set("text".into(), Variant::from("[color=yellow]crab![/color]"));
base.add_child(child);
// ...
Self {
base,
child
}
}Another wishful example of pushing #[derive(GodotClass)]
#[class(init, base=Node)]
pub struct Parent {
#[base]
base: Base<Node>,
#[add(internal="INTERNAL_MODE_FRONT")]
#[set(use_bbcode=true, text="[color=yellow]crab![/color]")] // (when `#[add]` exists)
#[connect(signal="gui_input", handler="_on_child_gui_input", flag="CONNECT_DEFERRED")] // (handler in parent)
child: Gd<RichTextLabel>,
}Sky is the limit, you know! |
|
We should not push too much into proc-macro APIs -- while convenient, they have several disadvantages:
Some of your proc-macro examples use the equivalent amount of code as if those fields were initialized in Also, anything late-initialized cannot have type |
|
So weighing disadvantages against conveniences how further we may expect I think it does what it's supposed to do already, so nothing further is necessary. Are there any more conveniences you are contemplating? I can imagine more possibilities off the top of my head, like automatically implementing initializer that accepts optional struct fields of the extension type and returns |
There's no formal set of rules, but I tend to prioritize roughly in this descending order:
Don't quote me on that though 😉 I don't follow such a list, it's just roughly how I see things and it's subject to change all the time. In general, the priority of an issue is mainly decided by two factors:
The 2nd point also depends on contributors, and if there is already a design that could work. I am rather conservative in accepting massive changes in a single PR, because the chances that things fit well into existing systems are lower. Also, people are often eager to contribute but forget that every line of code has to be maintained for an indefinite amount of time, so there's a cost to every addition as well. Sometimes, other core parts of the library need to be ironed out before a new feature is ready. I hope that clarifies things a bit 🙂 |
I have no concrete plans yet -- there is already Something like the Whether a feature is worth a dedicated proc-macro API depends on the individual use case and the motivation behind it. Over time, we should probably establish some clearer criteria w.r.t. what is in scope and what isn't. As you say, the sky is the limit, but ultimately gdext aims to be pragmatic for everyday gamedev usage, and APIs should be as simple as possible without restricting more advanced usages. Of course that's very general again 😀 |
|
It all sounds wise. I think platform support there tries to climb up in the list of priorities, specially regarding the ever growing mobile market. Current state of everything else may be considered ready enough for some projects, but when export time arrives, the platform support can scare some away! Anyway! You and GDExt contributors have done a great job. I really appreciate it and wish you all health and happiness. |
|
How to best approach the resource loading problem is also something I've been wondering about. I'm not sure if there are significant performance differences between "script-based preloading" or simply loading them at runtime in Rust, but I'd expect loading them in Rust is fine. My initial approach was to manually load a bunch of resources at once within Currently I'm experimenting with having one central use godot::engine::ShaderMaterial;
use godot::prelude::*;
use once_cell::unsync::Lazy;
use std::sync::Mutex;
// Since the Godot thread safety guidelines specify that loading resources isn't thread
// safe, I'm guarding the loading with a mutex.
static LOADING_MUTEX: once_cell::sync::Lazy<Mutex<()>> =
once_cell::sync::Lazy::new(|| Mutex::new(()));
pub fn shader_material_foo() -> Gd<ShaderMaterial> {
thread_local! {
static INSTANCE: Lazy<Gd<ShaderMaterial>> = Lazy::new(|| {
let _lock = LOADING_MUTEX.lock();
try_load::<ShaderMaterial>("shaders/foo.tres").unwrap()
});
}
INSTANCE.with(|instance| instance.share())
}
pub fn shader_material_bar() -> Gd<ShaderMaterial> {
thread_local! {
static INSTANCE: Lazy<Gd<ShaderMaterial>> = Lazy::new(|| {
let _lock = LOADING_MUTEX.lock();
try_load::<ShaderMaterial>("shaders/bar.tres").unwrap()
});
}
INSTANCE.with(|instance| instance.share())
}
// ... many more resources (could use a macro to reduce the boilerplate of the pattern above)
pub fn shader_material_baz() -> Gd<ShaderMaterial> {
// ...
}
// I call the `preload_resources` from a central place at start-up to preload a selected
// subset of resources. This allows to relatively easily switch from preloading to lazy
// loading a certain resource.
pub fn preload_resources() {
shader_material_foo();
shader_material_bar();
// shader_material_baz() // let's lazy load this one...
}Not quite sure if that's a good idea though 😉 EDIT: An obvious flaw of the current implementation is that it doesn't drop the resources at the process exit, leading to "RID allocations ... were leaked at exit" warnings. Perhaps with a bit more fancy wrapping it would be possible though to come up with a manual Regarding the side discussion on |
One thing I really don't like about Rust is how excessively boilerplaty the code becomes as soon as anything global is involved. I mean I understand why some of it is needed, and it's probably good to discourage reckless use of globals (especially with multi-threading), but... some valid use cases are still very hard to model safely. For example, if you have a "loading phase" where the globals are initialized, and a "using phase" where all the access is read-only. All your data types still need to account for late-initialization, locking, etc. The only way to avoid this is That was a bit off-topic... In terms of resource loading, there is maybe potential to find some patterns that can be abstracted away, so that gdext users don't need deeply nested synchronization types.
Yeah, probably makes sense to look at those as orthogonal issues. |
Absolutely, especially for occasional Rust users like me! This would be a revised version of the thread-local-lazy-static pattern that doesn't leak resources by offering an explicit cleanup. Just demonstrating it for a single resource here: thread_local! {
static INSTANCE_RESOURCE_FOO: Mutex<RefCell<Option<Gd<ShaderMaterial>>>> = Mutex::new(RefCell::new(None))
}
// The lazy resource getter:
pub fn get_resource_foo() -> Gd<ShaderMaterial> {
INSTANCE_RESOURCE_FOO.with(|mutex| {
let ref_cell = mutex.lock().unwrap();
let mut option = ref_cell.borrow_mut();
let instance = match option.as_ref() {
None => {
let _lock = LOADING_MUTEX.lock();
// Actual loading functionality goes here:
let instance = try_load::<ShaderMaterial>("shaders/line_basic.tres").unwrap();
*option = Some(instance.share());
instance
}
Some(instance) => instance.share(),
};
instance
})
}
// Corresponding cleanup function:
pub fn release_resource_foo() {
INSTANCE_RESOURCE_FOO.with(|mutex| {
let ref_cell = mutex.lock().unwrap();
let mut option = ref_cell.borrow_mut();
*option = None;
})
}And my scene tree root node takes care of calling a fn on_notification(&mut self, what: ControlNotification) {
if what == ControlNotification::Predelete {
release_resources();
}
}The challenge with static + thread_local seems to be that it is not possible to drop the underlying type explicitly. That's why I decided to switch from However, the fact that manual cleanup calls are necessary raises the question if |
|
Now that If yes, we should discuss what concretely is missing. |
Speaking for myself, I think your valuable time is better invested on other fronts. |
Thanks for the input! Indeed "external APIs" have been discussed for quite a while, most recently in #372 🙂 |
|
Well, yes, that... and... |
Yes, we're talking about that too in #372 🙂
You can have them through |
|
Oh! my bad! |
In GDscript preload() loads resource when script is parsed.
But in Godot Rust there's no way to preload a resource.
I suggest adding a preload attribute to GodotClass derive macro:
So during the class registration, the resource is loaded (maybe using Godot's ResourcePreloader class).
The text was updated successfully, but these errors were encountered: