Merge pull request #69 from apple1417/master

make arrays and hook return values use standard property access semantics

Author: apple1417

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.25)
 
-project(pyunrealsdk VERSION 1.5.2)
+project(pyunrealsdk VERSION 1.6.0)
 
 function(_pyunrealsdk_add_base_target_args target_name)
     target_compile_features(${target_name} PUBLIC cxx_std_20)

changelog.md

@@ -1,6 +1,6 @@
 # Changelog
 
-## Upcoming
+## v1.6.0
 
 - `WrappedStruct` now supports being copied via the `copy` module.
 
@@ -11,10 +11,37 @@
 
   [10bdc130](https://github.com/bl-sdk/pyunrealsdk/commit/10bdc130)
 
+- Hook return values and array access now have the same semantics as normal property accesses. In
+  practice this means:
+
+  - Getting an enum property will convert it to a python `IntFlag` enum (rather than an int).
+  - Setting an array property will accept any sequence (rather than just wrapped arrays).
+  
+  All other property types had the same semantics already, so this is backwards compatible.
+
+  [c52a807e](https://github.com/bl-sdk/pyunrealsdk/commit/c52a807e),
+  [16ac1711](https://github.com/bl-sdk/pyunrealsdk/commit/16ac1711)
+
 - Added a `_get_address` method to `WrappedArray`, `WrappedMulticastDelegate`, and `WrappedStruct`.
 
   [1b3e9686](https://github.com/bl-sdk/pyunrealsdk/commit/1b3e9686)
 
+
+### unrealsdk v1.7.0
+For reference, the unrealsdk v1.7.0 changes this includes are:
+
+> - `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.5.2
 
 ### unrealsdk v1.6.1

libs/unrealsdk

@@ -3,6 +3,7 @@
 #include "pyunrealsdk/exports.h"
 #include "pyunrealsdk/hooks.h"
 #include "pyunrealsdk/logging.h"
+#include "pyunrealsdk/unreal_bindings/property_access.h"
 #include "unrealsdk/hook_manager.h"
 #include "unrealsdk/unreal/cast.h"
 #include "unrealsdk/unreal/prop_traits.h"
@@ -46,8 +47,9 @@ namespace {
 bool handle_py_hook(Details& hook, const py::object& callback) {
     py::object ret_arg;
     if (hook.ret.has_value()) {
-        cast(hook.ret.prop, [&hook, &ret_arg]<typename T>(const T* /*prop*/) {
-            ret_arg = py::cast(hook.ret.get<T>());
+        cast(hook.ret.prop, [&hook, &ret_arg]<typename T>(T* prop) {
+            ret_arg = pyunrealsdk::unreal::py_getattr(
+                prop, reinterpret_cast<uintptr_t>(hook.ret.ptr.get()), hook.ret.ptr);
         });
     } else {
         ret_arg = py::type::of<Unset>();
@@ -76,9 +78,15 @@ bool handle_py_hook(Details& hook, const py::object& callback) {
             if (py::type::of<Unset>().is(ret_override) || py::isinstance<Unset>(ret_override)) {
                 hook.ret.destroy();
             } else if (!py::ellipsis{}.equal(ret_override)) {
-                cast(hook.ret.prop, [&hook, &ret_override]<typename T>(const T* /*prop*/) {
-                    auto value = py::cast<typename PropTraits<T>::Value>(ret_override);
-                    hook.ret.set<T>(value);
+                cast(hook.ret.prop, [&hook, &ret_override]<typename T>(T* prop) {
+                    // Need to replicate PropertyProxy::set ourselves a bit, since we want to use
+                    // our custom python setter
+                    if (hook.ret.ptr.get() == nullptr) {
+                        hook.ret.ptr = UnrealPointer<void>(hook.ret.prop);
+                    }
+
+                    pyunrealsdk::unreal::py_setattr_direct(
+                        prop, reinterpret_cast<uintptr_t>(hook.ret.ptr.get()), ret_override);
                 });
             }
         }

src/pyunrealsdk/hook.cpp

@@ -52,16 +52,6 @@ py::object array_get(const WrappedArray& arr, size_t idx);
  */
 void array_set(WrappedArray& arr, size_t idx, const py::object& value);
 
-/**
- * @brief Ensures a value is compatible with the given array.
- * @note Intended to be used when there's extra, non-reversible, work to do before it's possible to
- *       call array_set.
- *
- * @param arr The array to check against.
- * @param value The value to check.
- */
-void array_validate_value(const WrappedArray& arr, const py::object& value);
-
 /**
  * @brief Deletes a range of entries of arbitrary type in an array.
  *

src/pyunrealsdk/unreal_bindings/wrapped_array.h

@@ -1,6 +1,8 @@
 #include "pyunrealsdk/pch.h"
+#include "pyunrealsdk/unreal_bindings/property_access.h"
 #include "pyunrealsdk/unreal_bindings/wrapped_array.h"
 #include "unrealsdk/unreal/cast.h"
+#include "unrealsdk/unreal/classes/uproperty.h"
 #include "unrealsdk/unreal/wrappers/wrapped_array.h"
 
 #ifdef PYUNREALSDK_INTERNAL
@@ -21,21 +23,35 @@ size_t convert_py_idx(const WrappedArray& arr, py::ssize_t idx) {
 }
 
 py::object array_get(const WrappedArray& arr, size_t idx) {
-    py::object ret{};
-    cast(arr.type, [&]<typename T>(const T* /*prop*/) { ret = py::cast(arr.get_at<T>(idx)); });
+    if (arr.type->Offset_Internal != 0) {
+        throw std::runtime_error(
+            "array inner property has non-zero offset, unsure how to handle, aborting!");
+    }
+    if (arr.type->ArrayDim != 1) {
+        throw std::runtime_error(
+            "array inner property is fixed array, unsure how to handle, aborting!");
+    }
 
-    return ret;
+    // Const cast is slightly naughty, but we know the internals aren't going to modify properties
+    return py_getattr(
+        const_cast<UProperty*>(arr.type),  // NOLINT(cppcoreguidelines-pro-type-const-cast)
+        reinterpret_cast<uintptr_t>(arr.base.get()->data) + (arr.type->ElementSize * idx),
+        arr.base);
 }
 
 void array_set(WrappedArray& arr, size_t idx, const py::object& value) {
-    cast(arr.type, [&]<typename T>(const T* /*prop*/) {
-        arr.set_at<T>(idx, py::cast<typename PropTraits<T>::Value>(value));
-    });
-}
+    if (arr.type->Offset_Internal != 0) {
+        throw std::runtime_error(
+            "array inner property has non-zero offset, unsure how to handle, aborting!");
+    }
+    if (arr.type->ArrayDim != 1) {
+        throw std::runtime_error(
+            "array inner property is fixed array, unsure how to handle, aborting!");
+    }
 
-void array_validate_value(const WrappedArray& arr, const py::object& value) {
-    cast(arr.type,
-         [&]<typename T>(const T* /*prop*/) { py::cast<typename PropTraits<T>::Value>(value); });
+    py_setattr_direct(
+        const_cast<UProperty*>(arr.type),  // NOLINT(cppcoreguidelines-pro-type-const-cast)
+        reinterpret_cast<uintptr_t>(arr.base.get()->data) + (arr.type->ElementSize * idx), value);
 }
 
 void array_delete_range(WrappedArray& arr, size_t start, size_t stop) {

src/pyunrealsdk/unreal_bindings/wrapped_array_helpers.cpp

@@ -11,11 +11,14 @@ using namespace unrealsdk::unreal;
 namespace pyunrealsdk::unreal::impl {
 
 void array_py_append(WrappedArray& self, const py::object& value) {
-    array_validate_value(self, value);
-
     auto size = self.size();
     self.resize(size + 1);
-    array_set(self, size, value);
+    try {
+        array_set(self, size, value);
+    } catch (...) {
+        self.resize(size);
+        throw;
+    }
 }
 
 void array_py_clear(WrappedArray& self) {
@@ -61,8 +64,6 @@ size_t array_py_index(const WrappedArray& self,
                       const py::object& value,
                       py::ssize_t start,
                       py::ssize_t stop) {
-    array_validate_value(self, value);
-
     // `list.index` method handles indexes a little differently to most methods. Essentially, any
     // index is allowed, and it's just implicitly clamped to the size of the array. You're allowed
     // to do some stupid things like `["a"].index("a", -100, -200)`, it just gives a not in list
@@ -98,45 +99,41 @@ size_t array_py_index(const WrappedArray& self,
 }
 
 void array_py_insert(WrappedArray& self, py::ssize_t py_idx, const py::object& value) {
-    array_validate_value(self, value);
-
     auto size = self.size();
 
-    // Allow specifying one past the end, to insert at the end
-    // insert(-1) should insert before the last element, so goes through the normal conversion
-    auto idx = static_cast<size_t>(py_idx) == size ? py_idx : convert_py_idx(self, py_idx);
+    if (static_cast<size_t>(py_idx) == size) {
+        // We're just appending
+        array_py_append(self, value);
+        return;
+    }
+
+    auto idx = convert_py_idx(self, py_idx);
 
     self.resize(size + 1);
 
-    // Don't move if appending
-    if (idx != size) {
-        auto data = reinterpret_cast<uintptr_t>(self.base->data);
-        auto element_size = self.type->ElementSize;
+    auto data = reinterpret_cast<uintptr_t>(self.base->data);
+    auto element_size = self.type->ElementSize;
+
+    auto src = data + (idx * element_size);
+    auto remaining_size = (size - idx) * element_size;
+    memmove(reinterpret_cast<void*>(src + element_size), reinterpret_cast<void*>(src),
+            remaining_size);
 
-        auto src = data + (idx * element_size);
-        auto remaining_size = (size - idx) * element_size;
-        memmove(reinterpret_cast<void*>(src + element_size), reinterpret_cast<void*>(src),
+    try {
+        array_set(self, idx, value);
+    } catch (...) {
+        // Move it all back
+        memmove(reinterpret_cast<void*>(src), reinterpret_cast<void*>(src + element_size),
                 remaining_size);
+        self.resize(size);
+        throw;
     }
-
-    array_set(self, idx, value);
 }
 
 py::object array_py_pop(WrappedArray& self, py::ssize_t py_idx) {
     auto idx = convert_py_idx(self, py_idx);
 
-    py::object ret{};
-    cast(self.type, [&self, &ret, idx]<typename T>(const T* /*prop*/) {
-        auto val = self.get_at<T>(idx);
-
-        // Explicitly make a copy
-        // Some types (structs) are a reference by default, which will break once we
-        // remove them otherwise
-        // NOLINTNEXTLINE(misc-const-correctness)
-        typename PropTraits<T>::Value val_copy = val;
-
-        ret = py::cast(val_copy);
-    });
+    py::object ret = array_get(self, idx);
 
     array_delete_range(self, idx, idx + 1);
 
@@ -171,15 +168,11 @@ void array_py_sort(WrappedArray& self, const py::object& key, bool reverse) {
                            []() { return py::module_::import("builtins").attr("sorted"); })
                        .get_stored();
 
-    py::sequence sorted_array = sorted(self, "key"_a = key, "reverse"_a = reverse);
-
-    cast(self.type, [&self, &sorted_array]<typename T>(const T* /*prop*/) {
-        auto size = self.size();
-        for (size_t i = 0; i < size; i++) {
-            auto val = py::cast<typename PropTraits<T>::Value>(sorted_array[i]);
-            self.set_at<T>(i, val);
-        }
-    });
+    const py::sequence sorted_array = sorted(self, "key"_a = key, "reverse"_a = reverse);
+    auto size = self.size();
+    for (size_t i = 0; i < size; i++) {
+        array_set(self, i, sorted_array[i]);
+    }
 }
 
 uintptr_t array_py_getaddress(const WrappedArray& self) {