search

utopia-rise / godot-kotlin-native

Kotlin bindings for Godot Engine
MIT License
290 stars 16 forks source link

Feature/pool array (DONE) #148

Closed CedNaru closed 4 years ago

CedNaru commented 4 years ago

Added a draft of a PoolArray.

Just to discuss if this template is good enough to be implemented for every PoolXArray.

I used something close to RanieJade's implementation. The one we previously had seems to have memory leak and unnecessary copies.

chippmann commented 4 years ago

I like the idea. I don't see a problem with it.

CedNaru commented 4 years ago

I tried to have NativeCall as a toplevel function. It works but I still had to implement a wrapper inside the poolArray class. It works fine now, except the fact that I had to use an abstract class and it seems that a public class (PoolArray) can't inherit from an internal class (NativeCoreType). I had to make NativeCoreType a public abstract class :x. I am not fond of this solution. image

piiertho commented 4 years ago

I tried to have NativeCall as a toplevel function. It works but I still had to implement a wrapper inside the poolArray class. It works fine now, except the fact that I had to use an abstract class and it seems that a public class (PoolArray) can't inherit from an internal class (NativeCoreType). I had to make NativeCoreType a public abstract class :x. I am not fond of this solution. image

Yep, I guess we cannot make better. I think the question is : "Is it a big problem that user can access this abstract class ?"

CedNaru commented 4 years ago

Well it's an abstract class so it's not like an user could create an instance of it by mistake. On the other side, a public abstract is like saying "You can implement your own coretype if you want" which can be misleading.

piiertho commented 4 years ago

yep that's true, what @raniejade and @chippmann do you think guys ?

chippmann commented 4 years ago

No i don't have a better idea Let's slap a "dont inherit" kdoc on it and call it a day

CedNaru commented 4 years ago

The remaining PoolArray are now done. There are 4 methods in PoolByteArray that I can't implement as they are not available in the Gdnative API. For now they are there as private methods with a Todo().

CedNaru commented 4 years ago

There were no equal methods either. I implemented a dirty one where I have to manually check each element individually. Probably slow but it works.

raniejade commented 4 years ago

I think you can add the internal modifier to the primary constructor of NativeCoreType, that should prevent inheritance outside of the module.