Get rid of py-clone (#119)

SkyTemple · Jul 20, 2024 · 889e3e9 · 889e3e9
1 parent 9c6bf95
commit 889e3e9
Show file tree

Hide file tree

Showing 27 changed files with 392 additions and 383 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "skytemple_rust"
-version = "1.8.0"
+version = "1.8.1"
 authors = ["Marco 'Capypara' Köpcke <[email protected]>"]
 edition = "2021"
 repository = "https://github.com/SkyTemple/skytemple-rust"
@@ -24,7 +24,7 @@ compression = []
 image = []
 
 sir0 = ["anyhow"]
-kao = ["compression", "image", "arr_macro"]
+kao = ["compression", "image"]
 map_bg = ["compression", "image"]
 misc_graphics = ["image"]
 dungeon_graphics = ["image", "sir0"]
@@ -40,15 +40,14 @@ romfs = ["nitro_fs", "memmap2", "strings"]
 
 [dependencies]
 skytemple_rust_macros = { path = "skytemple_rust_macros" }
-pyo3 = { version = "0.22", features = ["extension-module", "py-clone"] }
+pyo3 = { version = "0.22", features = ["extension-module"] }
 pyo3-log = { version = "0.11" }
 itertools = "0.13"
 log = "0.4"
 bytes = "1"
 num-traits = "0.2"
 num-derive = "0.4"
 paste = "1.0"
-arr_macro = { version = "0.2", optional = true }
 encoding = "0.2"
 encoding-index-singlebyte = { version = "1", optional = true }
 pmd_wan = { version = "5.1", optional = true }

diff --git a/pyproject.toml b/pyproject.toml
@@ -3,7 +3,7 @@ requires = ["setuptools", "wheel", "setuptools-rust"]
 
 [project]
 name = "skytemple-rust"
-version = "1.8.0"
+version = "1.8.1"
 authors = [
     { name = 'Marco "Capypara" Köpcke', email = "[email protected]" },
     # see About dialog or GitHub contributors list for additional people.

diff --git a/src/bytes.rs b/src/bytes.rs
@@ -229,24 +229,15 @@ impl From<StBytesMut> for Bytes {
     }
 }
 
-pub trait AsStBytes {
-    fn as_bytes(&self) -> StBytes;
+pub trait AsStBytesPyRef {
+    fn as_bytes_pyref(&self, py: Python) -> StBytes;
 }
 
-impl<T> AsStBytes for Py<T>
+impl<T> AsStBytesPyRef for Py<T>
 where
-    Self: Into<StBytes> + Clone,
+    Self: Into<StBytes>,
 {
-    fn as_bytes(&self) -> StBytes {
-        self.clone().into()
-    }
-}
-
-impl<T> AsStBytes for &T
-where
-    T: AsStBytes,
-{
-    fn as_bytes(&self) -> StBytes {
-        T::as_bytes(self)
+    fn as_bytes_pyref(&self, py: Python) -> StBytes {
+        self.clone_ref(py).into()
     }
 }
diff --git a/src/image/tilemap_entry.rs b/src/image/tilemap_entry.rs
@@ -122,7 +122,6 @@ impl From<&TilemapEntry> for usize {
     }
 }
 
-#[derive(Clone)]
 pub struct InputTilemapEntry(pub Py<TilemapEntry>);
 
 impl<'source> FromPyObject<'source> for InputTilemapEntry {

diff --git a/src/macros.rs b/src/macros.rs
@@ -63,26 +63,27 @@ macro_rules! impl_pylist {
         ::paste::paste! {
             #[pyclass(module = $module)]
             #[allow(clippy::derive_partial_eq_without_eq)]
-            #[derive(Clone, Debug)]
+            #[derive(Debug)]
             pub struct $name(pub Vec<$itemty>);
 
 
             #[pymethods]
             impl $name {
-                pub fn __iter__(&mut self) -> [<$name Iterator>] {
-                    [<$name Iterator>]::new(self.0.clone().into_iter())
+                pub fn __iter__(&mut self, py: Python) -> [<$name Iterator>] {
+                    // TODO: This is needlessly slow probably? Rethink iterator implementation.
+                    [<$name Iterator>]::new(self.0.iter().map(|e| e.clone_ref(py)).collect::<Vec<_>>().into_iter())
                 }
                 pub fn __getitem__(&self, idx: $crate::python::SliceOrInt, py: Python) -> PyResult<PyObject> {
                     match idx {
                         $crate::python::SliceOrInt::Slice(sl) => {
-                            let pylist = ::pyo3::types::PyList::new_bound(py, self.0.iter().cloned());
+                            let pylist = ::pyo3::types::PyList::new_bound(py, self.0.iter().map(|e| e.clone_ref(py)));
                             pylist
                                 .call_method1("__getitem__", ::pyo3::types::PyTuple::new_bound(py, [sl]))
                                 .map(|v| v.into_py(py))
                         }
                         $crate::python::SliceOrInt::Int(idx) => {
                             if idx >= 0 && idx as usize <= self.0.len() {
-                                Ok(self.0[idx as usize].clone().into_py(py))
+                                Ok(self.0[idx as usize].clone_ref(py).into_py(py))
                             } else {
                                 Err(::pyo3::exceptions::PyIndexError::new_err("list index out of range"))
                             }
@@ -92,7 +93,7 @@ macro_rules! impl_pylist {
                 pub fn __setitem__(&mut self, idx: $crate::python::SliceOrInt, o: PyObject, py: Python) -> PyResult<()> {
                     match idx {
                         $crate::python::SliceOrInt::Slice(sl) => {
-                            let pylist = ::pyo3::types::PyList::new_bound(py, self.0.iter().cloned());
+                            let pylist = ::pyo3::types::PyList::new_bound(py, self.0.iter().map(|e| e.clone_ref(py)));
                             pylist.call_method1("__setitem__", ::pyo3::types::PyTuple::new_bound(py, [sl.into_py(py), o]))?;
                             self.0 = pylist
                                 .into_iter()
@@ -113,7 +114,7 @@ macro_rules! impl_pylist {
                 pub fn __delitem__(&mut self, idx: $crate::python::SliceOrInt, py: Python) -> PyResult<()> {
                     match idx {
                         $crate::python::SliceOrInt::Slice(sl) => {
-                            let pylist = ::pyo3::types::PyList::new_bound(py, self.0.iter().cloned());
+                            let pylist = ::pyo3::types::PyList::new_bound(py, self.0.iter().map(|e| e.clone_ref(py)));
                             pylist.call_method1("__delitem__", ::pyo3::types::PyTuple::new_bound(py, [sl]))?;
                             self.0 = pylist
                                 .into_iter()
@@ -178,7 +179,7 @@ macro_rules! impl_pylist {
                             x.call_method1(
                                 py,
                                 "__eq__",
-                                ::pyo3::types::PyTuple::new_bound(py, [value.clone()]),
+                                ::pyo3::types::PyTuple::new_bound(py, [value.clone_ref(py)]),
                             )
                             .and_then(|x| x.is_truthy(py))
                             .unwrap_or_default()
@@ -199,7 +200,7 @@ macro_rules! impl_pylist {
                                 x.call_method1(
                                     py,
                                     "__eq__",
-                                    ::pyo3::types::PyTuple::new_bound(py, [value.clone()]),
+                                    ::pyo3::types::PyTuple::new_bound(py, [value.clone_ref(py)]),
                                 )
                                 .and_then(|x| x.is_truthy(py))
                                 .unwrap_or_default()
@@ -215,7 +216,7 @@ macro_rules! impl_pylist {
                             x.call_method1(
                                 py,
                                 "__eq__",
-                                ::pyo3::types::PyTuple::new_bound(py, [value.clone()]),
+                                ::pyo3::types::PyTuple::new_bound(py, [value.clone_ref(py)]),
                             )
                             .and_then(|x| x.is_truthy(py))
                             .unwrap_or_default()

diff --git a/src/st_bg_list_dat.rs b/src/st_bg_list_dat.rs
@@ -185,7 +185,6 @@ impl BgListEntry {
 }
 
 #[pyclass(module = "skytemple_rust.st_bg_list_dat")]
-#[derive(Clone)]
 pub struct BgList {
     #[pyo3(get)]
     level: Vec<Py<BgListEntry>>,

diff --git a/src/st_bgp.rs b/src/st_bgp.rs
@@ -16,17 +16,19 @@
  * You should have received a copy of the GNU General Public License
  * along with SkyTemple.  If not, see <https://www.gnu.org/licenses/>.
  */
-use crate::bytes::StBytes;
-use crate::image::tiled::TiledImage;
-use crate::image::tilemap_entry::TilemapEntry;
-use crate::image::{In256ColIndexedImage, InIndexedImage, IndexedImage, PixelGenerator};
+use std::cmp::{max, min};
+use std::io::Cursor;
+use std::iter::{once, repeat, repeat_with};
+
 use bytes::{Buf, BufMut, Bytes, BytesMut};
 use itertools::Itertools;
 use pyo3::exceptions::PyValueError;
 use pyo3::prelude::*;
-use std::cmp::{max, min};
-use std::io::Cursor;
-use std::iter::{once, repeat, repeat_with};
+
+use crate::bytes::StBytes;
+use crate::image::tiled::TiledImage;
+use crate::image::tilemap_entry::TilemapEntry;
+use crate::image::{In256ColIndexedImage, InIndexedImage, IndexedImage, PixelGenerator};
 
 pub const BGP_RES_WIDTH: usize = 256;
 pub const BGP_RES_HEIGHT: usize = 192;
@@ -48,7 +50,6 @@ pub const BGP_TOTAL_NUMBER_TILES_ACTUALLY: usize = 1024;
 // NOTE: Tile 0 is always 0x0.
 
 #[pyclass(module = "skytemple_rust.st_bgp")]
-#[derive(Clone)]
 pub struct Bgp {
     #[pyo3(get, set)]
     pub palettes: Vec<Vec<u8>>,

diff --git a/src/st_bma.rs b/src/st_bma.rs
@@ -16,6 +16,16 @@
  * You should have received a copy of the GNU General Public License
  * along with SkyTemple.  If not, see <https://www.gnu.org/licenses/>.
  */
+use std::io::Cursor;
+use std::iter::{Copied, Enumerate};
+use std::slice::Iter;
+
+use bytes::{Buf, BufMut, Bytes, BytesMut};
+use itertools::Itertools;
+use pyo3::exceptions::PyValueError;
+use pyo3::prelude::*;
+use pyo3::pyclass;
+
 use crate::bytes::StBytes;
 use crate::compression::bma_collision_rle::{
     BmaCollisionRleCompressor, BmaCollisionRleDecompressor,
@@ -32,17 +42,8 @@ use crate::st_bpc::BPC_TILE_DIM;
 use crate::st_bpl::input::InputBpl;
 use crate::st_bpl::{BPL_IMG_PAL_LEN, BPL_MAX_PAL, BPL_PAL_LEN};
 use crate::util::lcm;
-use bytes::{Buf, BufMut, Bytes, BytesMut};
-use itertools::Itertools;
-use pyo3::exceptions::PyValueError;
-use pyo3::prelude::*;
-use pyo3::pyclass;
-use std::io::Cursor;
-use std::iter::{Copied, Enumerate};
-use std::slice::Iter;
 
 #[pyclass(module = "skytemple_rust.st_bma")]
-#[derive(Clone)]
 pub struct Bma {
     #[pyo3(get, set)]
     pub map_width_camera: u8,
@@ -640,8 +641,22 @@ impl Bma {
     }
 
     pub fn deepcopy(&self) -> Self {
-        // Cloning isn't enough (pyo3 weirdness).
-        self.clone()
+        Self {
+            map_width_camera: self.map_width_camera,
+            map_height_camera: self.map_height_camera,
+            tiling_width: self.tiling_width,
+            tiling_height: self.tiling_height,
+            map_width_chunks: self.map_width_chunks,
+            map_height_chunks: self.map_height_chunks,
+            number_of_layers: self.number_of_layers,
+            unk6: self.unk6,
+            number_of_collision_layers: self.number_of_collision_layers,
+            layer0: self.layer0.clone(),
+            layer1: self.layer1.clone(),
+            unknown_data_block: self.unknown_data_block.clone(),
+            collision: self.collision.clone(),
+            collision2: self.collision2.clone(),
+        }
     }
 }
 

diff --git a/src/st_bpa.rs b/src/st_bpa.rs
@@ -33,7 +33,6 @@ use crate::image::{In256ColIndexedImage, InIndexedImage, IndexedImage, PixelGene
 pub const BPA_TILE_DIM: usize = 8;
 
 #[pyclass(module = "skytemple_rust.st_bpa")]
-#[derive(Clone)]
 pub struct BpaFrameInfo {
     #[pyo3(get, set)]
     pub duration_per_frame: u16,
@@ -53,7 +52,6 @@ impl BpaFrameInfo {
 }
 
 #[pyclass(module = "skytemple_rust.st_bpa")]
-#[derive(Clone)]
 pub struct Bpa {
     #[pyo3(get, set)]
     pub number_of_tiles: u16,
@@ -285,11 +283,12 @@ impl Bpa {
                 for _ in len_finfo..(self.number_of_frames as usize) {
                     // If the length is shorter, we just copy the last entry
                     if len_finfo > 0 {
-                        let entry_before = self.frame_info[len_finfo - 1].borrow(py).clone();
-                        self.frame_info.push(Py::new(
-                            py,
-                            BpaFrameInfo::new(entry_before.duration_per_frame, entry_before.unk2),
-                        )?);
+                        let entry_before = self.frame_info[len_finfo - 1].borrow(py);
+                        let duration_per_frame = entry_before.duration_per_frame;
+                        let unk2 = entry_before.unk2;
+                        drop(entry_before);
+                        self.frame_info
+                            .push(Py::new(py, BpaFrameInfo::new(duration_per_frame, unk2))?);
                     } else {
                         self.frame_info.push(Py::new(py, BpaFrameInfo::new(10, 0))?);
                     }
@@ -385,7 +384,12 @@ pub mod input {
         }
 
         fn __get_cloned_frame_info(&self, py: Python) -> PyResult<Vec<Py<BpaFrameInfo>>> {
-            Ok(self.borrow(py).frame_info.clone())
+            Ok(self
+                .borrow(py)
+                .frame_info
+                .iter()
+                .map(|e| e.clone_ref(py))
+                .collect::<Vec<_>>())
         }
     }
 
@@ -442,12 +446,6 @@ pub mod input {
         }
     }
 
-    impl From<InputBpa> for Bpa {
-        fn from(obj: InputBpa) -> Self {
-            Python::with_gil(|py| obj.0.to_object(py).extract(py).unwrap())
-        }
-    }
-
     impl Clone for InputBpa {
         fn clone(&self) -> Self {
             Python::with_gil(|py| {