Issues I noticed while implementing a scripting language for godot via godot-rust

[walksanatora]

Issues:

• make GDExtensionCallErrorType a actual enum instead of a type int (used in ScriptInstance::call's return value)
• make IScriptLanguageExtension::make_template infallable as returning None causes a engine crash due to null ref
• #[godot_api] causes IDE autocomplete to have a stroke while filling incomplete information. eg: if you start typing fn handles_t and try to get the autocomplete it wont work. suggestion: make the span only cover the attribute instead of the whole block mabey?
• object_get_script_instance from the gdextension_interface.h is not exposed in any safe way to rust. it should be decently easy though since it just needs two pointers and returns oneinterface
• while trying to work around the above issue I was annoyed at the lack of ability to cast Gd to raw pointers. probally would be a unsafe api for obvious reasons. but it would be usefull in cases like this where the function isn't avaliable or you have a external cpp section of your crate that needs those pointers (they are #[doc(hidden)] would be nice if it was visible with a giant warning across it

[walksanatora]

docs link: https://docs.rs/godot/latest/godot/classes/trait.IScriptLanguageExtension.html#tymethod.make_template

[walksanatora]

I also noticed alot while editing that the the #[godot_api] should NOT block autocomplete.
if I start typing fn handles_t it would autocomplete if I dont have the godot_api. I think it is because of how the span is placed.

[Bromeon]

IDE interaction with proc-macros is very brittle, and varies a lot between VS Code and Jetbrains. I don't know how we can act upon this in a general way. If you find concrete changes that improve the user experience, please let us know (or open a PR), that would be appreciated a lot!

[lilizoey]

IDE interaction with proc-macros is very brittle, and varies a lot between VS Code and Jetbrains. I don't know how we can act upon this in a general way. If you find concrete changes that improve the user experience, please let us know (or open a PR), that would be appreciated a lot!

I was actually able to get it working:

This particular solution is very hacky and definitely not PR-worthy, but basically by ensuring that the #[godot_api] macro never fails to return the original impl block (and updating rust-analyzer) i got autocompletions to work.

I also found that venial panics if you pass in an incomplete impl Foo block. Which is a problem, but a catch_unwind does catch the panic which lets me just turn #[godot_api] into a noop in that case.

[walksanatora]

also if we wanna guarentee that the rust type from object_get_script_instance is the same rust type I have some example code ideas.

use std::marker::PhantomData;

#[repr(C)]
struct InstancePtr<T> where T: Instance {
    ptr: Option<*mut std::ffi::c_void>,
    ghost: PhantomData<T>
}

trait ScriptInstance {}

trait IScriptLanguage {
    type Inst: ScriptInstance;
}

trait IScript {
    type Lang: IScriptLanguage;
    fn new_instance(&self) -> InstancePtr<<<Self as IScript>::Lang as IScriptLang>::Inst>;
}

fn create_instance<T>(rust: T) -> InstancePtr<T> where T: IScriptInstance{
    InstancePtr {
        ptr: None,
        ghost: PhantomData
    }
}

struct MyInstance {} impl IScriptInstance for MyInstance {}
struct MyLanguage {} impl IScriptLanguage for MyLanguage {type Inst = MyInstance;}
struct MyScript {}
impl IScript for MyScript {
    type Lang = MyLanguage;

    fn new_instance(&self) -> InstancePtr<<<Self as IScript>::Lang as IScriptLanguage>::Inst> {
        create_instance(MyInstance {})
    }
}

this would allow object_get_script_instance to be

fn obj_get_script_instance<T>(object: Gd<Object>,lang: Gd<T>) -> <T as IScriptLanguage>::Inst where T: IScriptLanguage {
    //pointer deref magic
    panic!("I dont wanna simulate ffi here")
}

[walksanatora]

this would work because godot checks if ScriptLanguages match on the c++ side (so it would return None if ScriptLanguage instances dont match).

[Bromeon]

See also #987 (comment)

[TitanNano]

@walksanatora I looked into exposing obj_get_script_instance.

What is your use case for this function?
A major issue I see with this function is that it exposes the script instance data, which is owned by the engine and could be freed at any time. It's a manually managed special struct and can't utilize the existing Gd<T>... At the moment, I don't see a good reason to expose the script instance and implement new safeguards around it.

I was thinking to just implement a

fn script_instance_exists<T: Inherits<Object>, L: Inherits<ScriptLanguage>>(object: T, language: L) -> bool

Because that would make it possible to implement IScriptExtension::instance_has.

[walksanatora]

I want obj_get_script_instance so that I can call a reload function on it to reload my code without having to do some weird pass through godot and make a weird _reload method