make UnrealPointer support pointers to properties, rework PropertyProxy to use it

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.25)
 
-project(unrealsdk VERSION 1.6.1)
+project(unrealsdk VERSION 1.7.0)
 
 set(UNREALSDK_UE_VERSION "UE4" CACHE STRING "The unreal engine version to build the SDK for. One of 'UE3' or 'UE4'.")
 set(UNREALSDK_ARCH "x64" CACHE STRING "The architecture to build the sdk for. One of 'x86' or 'x64'.")

changelog.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## 1.7.0
+
+- `unrealsdk::unreal::cast` now copies the const-ness of its input object to its callbacks.
+
+  [779d75ea](https://github.com/bl-sdk/unrealsdk/commit/779d75ea)
+
+- Reworked `PropertyProxy` to be based on `UnrealPointer` (and reworked it too). This fixes some
+  issues with ownership and possible use after frees.
+
+  *This breaks binary compatibility*, though existing code should work pretty much as is after a
+  recompile.
+
+  [49bff4a4](https://github.com/bl-sdk/unrealsdk/commit/49bff4a4)
+
 ## v1.6.1
 
 - Handled `UClass::Interfaces` also having a different offset between BL2 and TPS.

src/unrealsdk/unreal/cast.h

@@ -125,7 +125,7 @@ template <typename InputType,
           bool check_inherited_types,
           typename ClassTuple,
           size_t i>
-void cast_impl(const InputType* obj,
+void cast_impl(InputType* obj,
                const UStruct* working_class,
                const Function& func,
                const Fallback& fallback) {
@@ -149,12 +149,17 @@ void cast_impl(const InputType* obj,
         using cls = std::tuple_element_t<i, ClassTuple>;
 
         // If this class inherits from the input type
-        if constexpr (std::is_base_of_v<InputType, cls>
-                      && (include_input_type || !std::is_same_v<InputType, cls>)) {
+        if constexpr (std::is_base_of_v<std::remove_const_t<InputType>, cls>
+                      && (include_input_type
+                          || !std::is_same_v<std::remove_const_t<InputType>, cls>)) {
             // If the class name matches
             if (working_class->Name == cls_fname<cls>()) {
                 // Run the callback
-                return func.template operator()<cls>(reinterpret_cast<const cls*>(obj));
+                if constexpr (std::is_const_v<InputType>) {
+                    return func.template operator()<cls>(reinterpret_cast<const cls*>(obj));
+                } else {
+                    return func.template operator()<cls>(reinterpret_cast<cls*>(obj));
+                }
             }
         }
 
@@ -222,22 +227,22 @@ struct cast_options {
  *
  * @tparam Options The options to use for this cast.
  * @tparam InputType The type of the input object, the least derived type which may be cast to.
- * @tparam Function The type of the callback function. The signature should be `void(const T* obj)`,
- *                  where `T` is a template arg which will derive from (or be equal to) `InputType`.
+ * @tparam Function The type of the callback function. The signature should be `void(T* obj)` (or
+ *                  `void(const T* obj)` if `InputType` is const), where `T` is a template arg which
+ *                  will derive from (or be equal to) `std::remove_const_t<InputType>`.
  * @tparam Fallback The fallback function's type, only templated for auto deduction. The signature
-                    should be `void(const InputType* obj)`.
+                    should be `void(InputType* obj)` or `void(const InputType* obj)`.
  * @param obj The object to cast.
  * @param func The templated callback function to call.
  * @param fallback A function to call when casting fails. Defaults to throwing a runtime error.
  */
 template <typename Options = cast_options<>,
           typename InputType,
           typename Function,
-          typename Fallback = std::function<void(const InputType*)>,
-          typename = std::enable_if_t<std::is_base_of_v<UObject, InputType>>>
-void cast(const InputType* obj,
-          const Function& func,
-          const Fallback& fallback = default_cast_fallback) {
+          typename Fallback = std::function<void(InputType*)>>
+void cast(InputType* obj, const Function& func, const Fallback& fallback = default_cast_fallback)
+    requires(std::is_base_of_v<UObject, InputType>)
+{
     if (obj == nullptr) {
         throw std::invalid_argument("Tried to cast null object!");
     }

src/unrealsdk/unreal/wrappers/property_proxy.cpp

@@ -10,38 +10,16 @@
 
 namespace unrealsdk::unreal {
 
-/*
-Property proxies are primarily intended to hold hook return values.
-We generally expect these to be small values, such as bools, enums/ints, or object pointers, which
-would be a bit off a waste to allocate space on the heap for.
-Instead, we take inspiration from short string optimization, and store short properties inside the
-value pointer.
-We need to access the property to get it's offset whenever we try read/write the value anyway, so
-also checking size doesn't even add any extra pointer accesses.
-*/
-
-PropertyProxy::PropertyProxy(UProperty* prop) : prop(prop), value(nullptr), been_set(false) {
-    if (prop != nullptr) {
-        const size_t size = this->prop->ElementSize * this->prop->ArrayDim;
-        if (size > sizeof(void*)) {
-            this->value = unrealsdk::u_malloc(size);
-        }
-    }
-}
-PropertyProxy::PropertyProxy(const PropertyProxy& other) : PropertyProxy(other.prop) {
-    if (other.been_set) {
+PropertyProxy::PropertyProxy(UProperty* prop) : prop(prop), ptr(nullptr) {}
+PropertyProxy::PropertyProxy(const PropertyProxy& other) : prop(other.prop), ptr(nullptr) {
+    if (this->prop != nullptr && other.has_value()) {
         cast(this->prop, [this, &other]<typename T>(const T* prop) {
             for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) {
                 this->set<T>(i, other.get<T>(i));
             }
         });
     }
 }
-PropertyProxy::PropertyProxy(PropertyProxy&& other) noexcept
-    : prop(std::exchange(other.prop, nullptr)),
-      value(std::exchange(other.value, nullptr)),
-      been_set(std::exchange(other.been_set, false)) {}
-
 PropertyProxy& PropertyProxy::operator=(const PropertyProxy& other) {
     if (other.prop != this->prop) {
         throw std::runtime_error("Property proxy is not instance of "
@@ -56,64 +34,26 @@ PropertyProxy& PropertyProxy::operator=(const PropertyProxy& other) {
     }
     return *this;
 }
-PropertyProxy& PropertyProxy::operator=(PropertyProxy&& other) noexcept {
-    std::swap(this->prop, other.prop);
-    std::swap(this->value, other.value);
-    std::swap(this->been_set, other.been_set);
-    return *this;
-}
-
-PropertyProxy::~PropertyProxy() {
-    if (this->prop == nullptr) {
-        // Nothing to do
-        return;
-    }
-
-    auto addr = this->get_value_addr();
-    cast(this->prop, [addr]<typename T>(const T* prop) {
-        for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) {
-            destroy_property<T>(prop, i, addr);
-        }
-    });
-
-    const size_t size = this->prop->ElementSize * this->prop->ArrayDim;
-    if (size > sizeof(void*)) {
-        unrealsdk::u_free(this->value);
-    }
-}
-
-uintptr_t PropertyProxy::get_value_addr(void) const {
-    if (this->prop == nullptr) {
-        throw std::runtime_error("Property does not exist!");
-    }
-    const size_t size = this->prop->ElementSize * this->prop->ArrayDim;
-
-    return (size > sizeof(void*) ? reinterpret_cast<uintptr_t>(this->value)
-                                 : reinterpret_cast<uintptr_t>(&this->value))
-           - this->prop->Offset_Internal;
-}
 
 bool PropertyProxy::has_value(void) const {
-    return this->been_set;
+    return this->ptr.get() != nullptr;
 }
 
 void PropertyProxy::destroy(void) {
-    // We're lying a little here, we'll simply act as if we don't have a value set, and only
-    // actually destroy it in the destructor
-    // In case of fixed arrays, just going to consider uninitialized entries to have an undefined
-    // value, so doing `destroy(); set(0, ...); get(1);` is your own fault.
-    this->been_set = false;
+    this->ptr = UnrealPointer<void>{nullptr};
 }
 
-void PropertyProxy::copy_to(uintptr_t addr) {
-    if (!this->been_set) {
+void PropertyProxy::copy_to(uintptr_t addr) const {
+    if (!this->has_value()) {
         throw std::runtime_error("Cannot copy empty property!");
     }
-    cast(this->prop, [this, addr]<typename T>(const T* prop) {
-        for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) {
-            set_property<T>(prop, i, addr, this->get<T>(i));
-        }
-    });
+    if (this->prop != nullptr) {
+        cast(this->prop, [this, addr]<typename T>(const T* prop) {
+            for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) {
+                set_property<T>(prop, i, addr, this->get<T>(i));
+            }
+        });
+    }
 }
 
 void PropertyProxy::copy_from(uintptr_t addr) {

src/unrealsdk/unreal/wrappers/property_proxy.h

@@ -5,28 +5,19 @@
 
 #include "unrealsdk/unreal/class_name.h"
 #include "unrealsdk/unreal/prop_traits.h"
+#include "unrealsdk/unreal/wrappers/unreal_pointer.h"
+#include "unrealsdk/unreal/wrappers/unreal_pointer_funcs.h"
 #include "unrealsdk/unrealsdk.h"
 
 namespace unrealsdk::unreal {
 
 class UProperty;
 
-class PropertyProxy {
+struct PropertyProxy {
    public:
     UProperty* prop;
+    UnrealPointer<void> ptr;
 
-   private:
-    void* value;
-    bool been_set;
-
-    /**
-     * @brief Get the address access to the value should use.
-     *
-     * @return The address access to the value should use.
-     */
-    [[nodiscard]] uintptr_t get_value_addr(void) const;
-
-   public:
     /**
      * @brief Constructs a new property proxy.
      *
@@ -35,7 +26,7 @@ class PropertyProxy {
      */
     PropertyProxy(UProperty* prop);
     PropertyProxy(const PropertyProxy& other);
-    PropertyProxy(PropertyProxy&& other) noexcept;
+    PropertyProxy(PropertyProxy&& other) noexcept = default;
 
     /**
      * @brief Assigns to the property proxy.
@@ -45,12 +36,12 @@ class PropertyProxy {
      * @return A reference to this property proxy.
      */
     PropertyProxy& operator=(const PropertyProxy& other);
-    PropertyProxy& operator=(PropertyProxy&& other) noexcept;
+    PropertyProxy& operator=(PropertyProxy&& other) noexcept = default;
 
     /**
      * @brief Destroy the property proxy.
      */
-    ~PropertyProxy();
+    ~PropertyProxy() = default;
 
     /**
      * @brief Checks if we have a value stored.
@@ -68,46 +59,53 @@ class PropertyProxy {
      */
     template <typename T>
     [[nodiscard]] typename PropTraits<T>::Value get(size_t idx = 0) const {
-        if (!this->been_set) {
+        if (!this->has_value()) {
             throw std::runtime_error(
                 "Tried to get value of a property proxy which is yet to be set!");
         }
 
-        return get_property<T>(validate_type<T>(this->prop), idx, this->get_value_addr());
+        return get_property<T>(validate_type<T>(this->prop), idx,
+                               reinterpret_cast<uintptr_t>(this->ptr.get()), this->ptr);
     }
 
     /**
      * @brief Sets the stored value.
      *
      * @tparam T The property type.
      * @param idx The fixed array index to get the value at. Defaults to 0.
-     * @param value The new stored value.
+     * @param new_value The new stored value.
      */
     template <typename T>
-    void set(const typename PropTraits<T>::Value& value) {
-        this->set<T>(0, value);
+    void set(const typename PropTraits<T>::Value& new_value) {
+        this->set<T>(0, new_value);
     }
     template <typename T>
-    void set(size_t idx, const typename PropTraits<T>::Value& value) {
+    void set(size_t idx, const typename PropTraits<T>::Value& new_value) {
         if (this->prop == nullptr) {
-            throw std::runtime_error("Property does not exist!");
+            throw std::runtime_error("Cannot set null property!");
         }
 
-        set_property<T>(validate_type<T>(this->prop), idx, this->get_value_addr(), value);
-        this->been_set = true;
+        if (!this->has_value()) {
+            this->ptr = UnrealPointer<void>{this->prop};
+        }
+        set_property<T>(validate_type<T>(this->prop), idx,
+                        reinterpret_cast<uintptr_t>(this->ptr.get()), new_value);
     }
 
     /**
      * @brief Destroys the stored value.
      */
     void destroy(void);
 
+    // The following functions aren't too useful for user code, but sdk internals still find them
+    // handy to have available.
+
     /**
      * @brief Copies the stored property to another address.
      *
      * @param addr The address to copy to.
      */
-    void copy_to(uintptr_t addr);
+    void copy_to(uintptr_t addr) const;
 
     /**
      * @brief Copies the value of another address to the stored property.

src/unrealsdk/unreal/wrappers/unreal_pointer.cpp

@@ -1,4 +1,8 @@
-#include "unrealsdk/unreal/wrappers/unreal_pointer.h"
+#include "unrealsdk/pch.h"
+#include "unrealsdk/unreal/cast.h"
+#include "unrealsdk/unreal/classes/uproperty.h"
+#include "unrealsdk/unreal/classes/ustruct.h"
+#include "unrealsdk/unrealsdk.h"
 
 namespace unrealsdk::unreal::impl {
 
@@ -17,4 +21,36 @@ size_t UnrealPointerControl::dec_ref(void) {
     return --this->refs;
 }
 
+void UnrealPointerControl::destroy_object(void) {
+    switch (this->pointer_type) {
+        case PointerType::STRUCT: {
+            auto struct_type = this->metadata.struct_type;
+            if (struct_type != nullptr) {
+                // Need to reconstruct the base address, since our pointer may be elsewhere
+                auto struct_base =
+                    reinterpret_cast<uintptr_t>(this) + sizeof(impl::UnrealPointerControl);
+
+                destroy_struct(struct_type, struct_base);
+            }
+            return;
+        }
+
+        case PointerType::PROPERTY: {
+            auto prop = this->metadata.prop;
+            if (prop != nullptr) {
+                // Need to reconstruct the base address, since our pointer may be elsewhere
+                auto prop_base = reinterpret_cast<uintptr_t>(this)
+                                 + sizeof(impl::UnrealPointerControl) - prop->Offset_Internal;
+
+                cast(prop, [prop_base]<typename T>(const T* prop) {
+                    for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) {
+                        destroy_property<T>(prop, i, prop_base);
+                    }
+                });
+            }
+            return;
+        }
+    }
+}
+
 }  // namespace unrealsdk::unreal::impl