Only create ScriptObject inside OnCreateBaseObject & `OnRemoveBaseO…

…bject` + cleanup (#157) * fix(client, compat): Remove code duplicates * client: Only create / delete ScriptObjects in `OnCreateBaseObject` / `OnRemoveBaseObject` * fix(shared, bindings): Fix `addEntityToAll` throwing `Cannot read properties of null` * Cleanup NativeInvoker GetArg for pointers * move resource cache init to IScriptObjectHandler
altmp · Jan 3, 2024 · 4f2fbfb · 4f2fbfb
1 parent 11fe217
commit 4f2fbfb
Show file tree

Hide file tree

Showing 11 changed files with 26 additions and 25 deletions.
diff --git a/client/js/compatibility/classes/entity.js b/client/js/compatibility/classes/entity.js
@@ -16,15 +16,6 @@ export class Entity extends alt.Entity {
         extendAltEntityClass(this, SharedEntity, WorldObject, BaseObject);
     }
 
-    static getByScriptID(...args) {
-        // NOTE (xLuxy): This method in v1 only returns WorldObject
-        if (typeof super.getByScriptID == "function") {
-            return super.getByScriptID(...args);
-        }
-
-        return null;
-    }
-
     get isSpawned() {
         if (![alt.Enums.BaseObjectType.PLAYER, alt.Enums.BaseObjectType.LOCAL_PLAYER].includes(super.type)) {
             return super.isSpawned;

diff --git a/client/js/compatibility/classes/weaponObject.js b/client/js/compatibility/classes/weaponObject.js
@@ -69,6 +69,6 @@ class WeaponObject extends alt.LocalObject {
     }
 }
 
-copyStaticAltEntityClassProperties(alt.WeaponObject, WeaponObject, Entity, WorldObject, BaseObject);
+copyStaticAltEntityClassProperties(WeaponObject, Entity, WorldObject, BaseObject);
 
 cppBindings.registerCompatibilityExport("WeaponObject", WeaponObject);
diff --git a/client/src/helpers/NativeInvoker.h b/client/src/helpers/NativeInvoker.h
@@ -58,7 +58,7 @@ namespace js
             using CleanT = std::remove_pointer_t<T>;
 
             CleanT value;
-            if(!ctx.GetArg(index, value, Type::INVALID, isPointer)) return false;
+            if(!ctx.GetArg<CleanT>(index, value)) return false;
 
             if constexpr(isPointer)
             {

diff --git a/shared/js/entity.js b/shared/js/entity.js
@@ -57,6 +57,7 @@ alt.Events.onBaseObjectRemove(({ object }) => {
 export function addEntityToAll(entity) {
     addEntityToAllWithType(entity, entity.type);
 }
+
 export function removeEntityFromAll(entity) {
     entityAllSetDirty = true;
     entityAllSet.delete(entity);

diff --git a/shared/src/helpers/CallContext.h b/shared/src/helpers/CallContext.h
@@ -242,10 +242,10 @@ namespace js
 
         // If no type to check is specified, it will try to convert the value to the specified type
         template<class T>
-        bool GetArg(int index, T& outValue, Type typeToCheck = Type::INVALID, bool allowNull = false)
+        bool GetArg(int index, T& outValue, Type typeToCheck = Type::INVALID)
         {
             if(errored) return false;
-            if(index >= info.Length() && !allowNull)
+            if(index >= info.Length())
             {
                 Throw("Missing argument at index " + std::to_string(index));
                 return false;
@@ -255,12 +255,6 @@ namespace js
             std::optional<T> result = CppValue<T>(info[index]);
             if(!result.has_value())
             {
-                if (allowNull)
-                {
-                    outValue = T{};
-                    return true;
-                }
-
                 Throw("Invalid argument type at index " + std::to_string(index) + ", expected " + CppTypeToString<T>() + " but got " + TypeToString(GetArgType(index)));
                 return false;
             }

diff --git a/shared/src/helpers/Convert.cpp b/shared/src/helpers/Convert.cpp
@@ -9,7 +9,7 @@ v8::Local<v8::Value> js::JSValue(alt::IBaseObject* object)
     if(!object) return v8::Null(isolate);
     IResource* resource = GetCurrentResource(isolate);
     if(!resource) return v8::Null(isolate);
-    ScriptObject* scriptObject = resource->GetOrCreateScriptObject(resource->GetContext(), object);
+    ScriptObject* scriptObject = resource->GetScriptObject(object);
     if(!scriptObject) return v8::Null(isolate);
     return scriptObject->Get();
 }
@@ -270,7 +270,7 @@ v8::Local<v8::Value> js::MValueToJS(alt::MValueConst val)
         case alt::IMValue::Type::BASE_OBJECT:
         {
             alt::IBaseObject* ref = std::dynamic_pointer_cast<const alt::IMValueBaseObject>(val)->RawValue();
-            return resource->GetOrCreateScriptObject(ctx, ref)->Get();
+            return resource->GetScriptObject(ref)->Get();
         }
         case alt::IMValue::Type::FUNCTION:
         {

diff --git a/shared/src/helpers/ValueBuffer.cpp b/shared/src/helpers/ValueBuffer.cpp
@@ -329,7 +329,7 @@ bool js::ValueDeserializer::Entity(v8::Local<v8::Value>& value)
 #endif
 
     if(!entity) return false;
-    value = resource->GetOrCreateScriptObject(resource->GetContext(), entity)->Get();
+    value = resource->GetScriptObject(entity)->Get();
     return true;
 }
 

diff --git a/shared/src/interfaces/IAltResource.h b/shared/src/interfaces/IAltResource.h
@@ -54,7 +54,10 @@ namespace js
 
         void OnCreateBaseObject(alt::IBaseObject* object) override
         {
-            // IScriptObjectHandler::GetOrCreateScriptObject(GetContext(), object);
+            if(context.IsEmpty()) return;
+            IResource::Scope scope(this);
+
+            IScriptObjectHandler::GetOrCreateScriptObject(GetContext(), object);
         }
 
         void OnRemoveBaseObject(alt::IBaseObject* object) override
@@ -70,7 +73,7 @@ namespace js
             if(context.IsEmpty()) return;
             IResource::Scope scope(this);
 
-            if(ev->GetType() == alt::CEvent::Type::RESOURCE_STOP) DestroyResourceObject(static_cast<const alt::CResourceStopEvent*>(ev)->GetResource());
+            if (ev->GetType() == alt::CEvent::Type::RESOURCE_STOP) DestroyResourceObject(static_cast<const alt::CResourceStopEvent*>(ev)->GetResource());
 
             Event::SendEvent(ev, this);
         }

diff --git a/shared/src/interfaces/IResource.h b/shared/src/interfaces/IResource.h
@@ -43,6 +43,7 @@ namespace js
         {
             context.Get(isolate)->SetAlignedPointerInEmbedderData(ContextInternalFieldIdx, this);
             ICompatibilityHandler::Initialize();
+            IScriptObjectHandler::Initialize(context.Get(isolate));
         }
 
         virtual void Reset()

diff --git a/shared/src/interfaces/IScriptObjectHandler.h b/shared/src/interfaces/IScriptObjectHandler.h
@@ -62,6 +62,16 @@ namespace js
             return customFactoryMap.contains(type);
         }
 
+        void Initialize(v8::Local<v8::Context> context)
+        {
+            auto entites = alt::ICore::Instance().GetEntities();
+
+            for (auto entity : entites)
+            {
+                GetOrCreateScriptObject(context, entity);
+            }
+        }
+
         static void BindClassToType(alt::IBaseObject::Type type, Class* class_);
     };
 }  // namespace js
diff --git a/shared/src/modules/SharedCppBindingsModule.cpp b/shared/src/modules/SharedCppBindingsModule.cpp
@@ -70,7 +70,7 @@ static void CreateEntity(js::FunctionContext& ctx)
             return;
         }
 
-        scriptObject = resource->GetOrCreateScriptObject(ctx.GetContext(), object);
+        scriptObject = resource->GetScriptObject(object);
         if(!scriptObject)
         {
             if(!tryCatch.HasCaught()) ctx.Throw("Failed to create entity of type " + std::string(magic_enum::enum_name(type)));
@@ -89,6 +89,7 @@ static void CreateEntity(js::FunctionContext& ctx)
 static void GetAllEntities(js::FunctionContext& ctx)
 {
     std::vector<alt::IEntity*> entities = alt::ICore::Instance().GetEntities();
+
     ctx.Return(entities);
 }