Shared pointer

[Roms1383]

Add basic and leaky support for red::SharedPtr to allow working with internal structs.
Safety must be audited first.

[jac3km4]

Suggested change

	this.instance = Box::leak(Box::new(value)) as *const _ as *mut _;
	this.instance = Box::leak(Box::new(value)) as *const _ as *mut _;

this is pretty bad, using this constructor repeatedely will leak lots of memory

[Roms1383]

isn't this memory eventually reclaimed in Drop impl ? (ptr::drop_in_place(ptr_instance);)

[Roms1383]

if not, what would be the appropriate approach for this @jac3km4 ? this PR has been sitting for almost a year i'm longing to finish it :) @psiberx @wopss if ever you have suggestions too I'm all ears !

[wopss]

From what I read on the internet, this part is partially correct, only that in the Drop implementation ptr::drop_in_place will not destroy the heap object.

What I saw, is that you do this:

Suggested change

	this.instance = Box::leak(Box::new(value)) as *const _ as *mut _;
	this.instance = Box::into_raw(value);

and then in Drop instead of ptr::drop_in_place, you do drop(Box::from_raw(...)).

See https://github.com/rust-lang/rust/blob/855e0fe46e68d94e9f6147531b75ac2d488c548e/library/alloc/src/boxed.rs#L1126-L1147.

Note: I haven't used Rust, so this is my understanding after some Googling and reading the docs.

[psiberx]

SharedPtr<T> only makes sense if this specific SharedPtr<T> is already used by the game (not even for every T known to the engine), and only if the right allocator is used for instance in this specific SharedPtr<T> case (or sometimes T).
It doesn't make sense for any T that wasn't used as SharedPtr<T> by the game, and won't work for any instance allocated by rust instead of original engine allocator.

[Roms1383]

thanks @wopss i think i understand your part and will make the appropriate changes.

following psiberx comment which i understand only partially @jac3km4, in Rust does it simply mean:

• making SharedPtr<T>(red::SharedPtrBase<T>) into SharedPtr<T, A>(red::SharedPtrBase<T>, PhantomData<A>);
  then, e.g. turning impl<T> Clone for SharedPtr<T> into impl<T> Clone for SharedPtr<T, crate::types::IAllocator> (for all the impl out there)
• or i need to take the extra leap and use RED4ext methods from bindgen like maybe IAllocator_Alloc / IAllocator_AllocAligned and IAllocator_Free (in IAllocator__bindgen_vtable).

these things are still out of my league, so ur help is very much appreciated ❤️

[wopss]

What @psiberx pointed out is that the memory for the shared object is Allocated using Rust's default allocator, but the memory in the game engine is allocated using a custom allocator, through IAllocator interfaces and friends.

If you allocate memory in Rust but then pass the ownership to the engine (for example, by putting the object into a game-owned DynArray), the engine won't recognize where that memory came from and might panic.

So, the correct approach is: instead of using Box::new, use the allocator from the engine (IAllocator_*).

As for the SharedPtr<T, A> idea: I'm not sure if that's a good option, my Rust knowledge is 0. @jac3km4, might be better suited to judge that part. In our C++ code, we get the allocator from type aliases or methods like GetAllocator.

[Roms1383]

I reckon that this might be simply this from my other PR @wopss : https://github.com/jac3km4/red4ext-rs/pull/87/changes#diff-6a173a8f0b02eb1ec671b6dddc0ff72617c14e04c89606b127db20ec279d7eb1R1612

[Roms1383]

Also this comment in RED4ext.SDK:

static_assert(Memory::IsDeleteCompatible<T>,
      "SharedPtr only supports types that define the allocator type and are destructible "
      "(a polymorphic type requires a virtual destructor)");

[Roms1383]

@jac3km4 @psiberx @wopss I think this PR is outdated, should we drop it?

[wopss]

@jac3km4 @psiberx @wopss I think this PR is outdated, should we drop it?

It is up to you and jekky to decide since I do not use Rust.

[psiberx]

I can't be 100% sure as I don't know rust either, but it looks to me like it's still incorrectly implemented and relies on rust allocations.

[Roms1383]

Yes I left it aside, but I do remember the reason of it in the first place now.

Currently with ISerializable inheritors they can be used perfectly when everything happens Rust-side only,
but passing them from Redscript to red4ext-rs plugin is not possible
because of some trait incompatibility and it's quite restrictive at times.

[Roms1383]

In a nutshell if I simply do this:

fn store_community_registry(community_registry: Ref<WorldCommunityRegistryNode>) {}

No compilation error.
But as soon as I do this:

exports![
    ::red4ext_rs::GlobalExport(::red4ext_rs::global!(
        c"MyMod.StoreCommunityRegistry",
        store_community_registry
    )),
]

Rust compiler complains:

the trait bound fn(Ref<...>) {store_community_registry}: GlobalInvocable<_, _> is not satisfied
the trait GlobalInvocable<_, _> is not implemented for fn item fn(red4ext_rs::types::Ref<WorldCommunityRegistryNode>) {store_community_registry}

Even though ScriptClass is implemented for the type in red4ext-rs-bindings:

#[repr(C)]
pub struct WorldCommunityRegistryNode {
    pub base: ISerializable,
    pub is_visible_in_game: bool, // 0x30
    pub is_host_only: bool, // 0x31
    pub spawn_set_name_to_community_id: GameCommunitySpawnSetNameToId, // 0x38
    pub crowd_creation_registry: Ref<GameCrowdCreationDataRegistry>, // 0x48
    pub communities_data: RedArray<WorldCommunityRegistryItem>, // 0x58
    pub workspots_persistent_data: RedArray<AiSpotPersistentData>, // 0x68
    pub represents_crowd: bool, // 0x78
}

unsafe impl ScriptClass for WorldCommunityRegistryNode {
    const NAME: &'static str = "worldCommunityRegistryNode";
    type Kind = class_kind::Native;
}

impl AsRef<ISerializable> for WorldCommunityRegistryNode {
    #[inline]
    fn as_ref(&self) -> &ISerializable {
        &self.base
    }
}

Solving it would allow me to iterate and prototype faster @jac3km4 :)

[Roms1383]

Ok at least I found a trick that allows me to get going.
Sharing here in case it's helpful to somebody else.

module MyMod
public native func LookMaICanUseISerializable(resource: ref<ISerializable>);

and

// in Plugin trait impl
exports![
    ::red4ext_rs::GlobalExport(::red4ext_rs::global!(
        c"MyMod.LookMaICanUseISerializable",
        look_ma_i_can_use_iserializable
    )),
]

// then
fn look_ma_i_can_use_iserializable(resource: Ref<ISerializable>) {
    if resource.is_null() {
        red4ext_rs::log::error!("ref<ISerializable> is null");
        return;
    }
    let Some(lanes) = (unsafe { resource.fields() }) else {
        red4ext_rs::log::error!("ref<ISerializable> fields cannot be accessed");
        return;
    };
    let lanes =
        unsafe { std::mem::transmute::<&ISerializable, &WorldTrafficPersistentResource>(lanes) };
    // do something with ref<worldTrafficPersistentResource> ...
}