Make mutable Script Instance functions reentrant

[TitanNano]

Closes #554

Adds a ref guard around a &mut T where T: ScriptInstance for the ScriptInstance trait. This guard exposes an API to access the script instance base object, through which reentrant calls can be achieved.

This also switches the script instance internals from a RefCell to a GdCell and introduces a new WithBase trait that can be shared between GodotClass and ScriptInstance.

---

I'm open to factor the WithBase introduction into a seperate PR if that is preferred.

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-671

[Bromeon]

Thanks a lot for the multiple iterations on this design! This looks quite clean for the user side 🧹 I'll already give some feedback, but will need to look at this deeper next time.

Not yet sure on the name SiMut. It somehow makes sense if you look at GdMut, however is it really equivalent? The former is never explicitly passed to user functions, it's always &mut T.

Please avoid links to issues/PRs inside commit messages, as they create new backreferences in the mentioned issue on every iteration and clutter the conversation. Just mention "Closes #554" once in the PR description (not its title).

[Bromeon]

Rebased to fix #669, CI still red due to changes in this PR though.

[Bromeon]

After some discussion with @lilizoey, we were thinking it would be nice to avoid introducing a new WithBase trait. While it addresses some duplication, it also adds a new symbol for everyone that was previously just dealing with GodotClass, as well as an extra indirection.

Instead, we'd propose separate structs:

// Existing:
struct BaseRef<'a, T: GodotClass> { ... }
struct BaseMut<'a, T: GodotClass> { ... }

// New (added by this PR):
struct ScriptBaseRef<'a, T: ScriptInstance> { ... }
struct ScriptBaseMut<'a, T: ScriptInstance> { ... }

We were also thinking about type aliases, but they have the problem to be leaky abstractions (i.e. error messages still contain the raw types, you cannot specify bounds on aliases, etc). So distinct types are better, since it's very unlikely that the user wants to abstract over both. Distinct types also make the relations BaseRef -> GodotClass and ScriptBaseRef -> ScriptInstance very explicit, which is a plus for understandability.

To avoid the duplication, you could use a declarative macro which substitutes some variables ($Ref:ty, $Mut:ty, $Bound:ty). It's not the most beautiful, but does the job and we do this in several places 😎

---

Also, create_script_instance would need to be turned into an unsafe function as per discussion 🙂

[TitanNano]

@Bromeon thanks for this feedback. I will see that I incorporate it into the PR.

[TitanNano]

@Bromeon I updated the BaseRef handling and added documentation for SiMut. createcreate_script_instanceis now also unsafe. Why the Clippy job is failing is unclear to me, I can't reproduce it locally.

[Bromeon]

Thanks a lot for the update!

[Bromeon]

Thanks a lot! From my side only minor things left.

Here's btw the direct link to SiMut docs from this PR:
https://godot-rust.github.io/docs/gdext/pr-671/godot/engine/struct.SiMut.html

[Bromeon]

Thanks a lot for yet another great pull request that enhances ScriptInstance functionality! 👍