Automatic resource management with destructors

[planetis-m]

Structs like Image, Texture, RenderTexture, Font, Mesh, Shader, Material, Model, Wave, Sound, Music, and AudioStream can make use of destructors like this:

type
  Texture = object
  ...

proc `=destroy`(x: var Texture) =
  proc UnloadTexture(texture: Texture) {.cdecl, importc, header: raylibHeader.}
  UnloadTexture(x)
proc `=copy`*(dest: var Texture; source: Texture) {.error.}

However closeWindow needs to be called last, so we are forced to introduce a dummy type.

type
  RaylibHandle = distinct bool

var
  isRaylibInitialized: bool

proc `=copy`*(dest: var RaylibHandle; source: RaylibHandle) {.error.}
proc `=destroy`(x: var RaylibHandle) =
  if isRaylibInitialized and x.bool:
    closeWindow()
    isRaylibInitialized = false

proc rinit*(width: cint; height: cint; title: cstring): RaylibHandle =
  if isRaylibInitialized:
    assert false, "Cannot initialize raylib more than once."
  else:
    initWindow(width, height, title)
    isRaylibInitialized = true
    result = RaylibHandle(true)

Let's not dwell into thread/memory safety to much, else the api would become too cumbersome to use.

[planetis-m]

These objects can use destructors:

Model
Mesh
Material
ModelAnimation

Wave
Sound
Music
AudioStream

Font

Image
Texture
RenderTexture

VrStereoConfig

Shader

Other functions that deal with unmanaged memory:

LoadModelAnimations
UnloadModelAnimations

LoadWaveSamples
UnloadWaveSamples

LoadImagePalette
UnloadImagePalette

LoadImageColors
UnloadImageColors

LoadFontData
UnloadFontData

LoadCodepoints
UnloadCodepoints

LoadMaterials

LoadFileDataCallback

A handle similar to RaylibHandle could to be created for InitAudioDevice and CloseAudioDevice.

...and all those functions that manipulate cstrings + filesystem, that can be removed.

Wave and Image have =copy procs (WaveCopy, ImageCopy), rest will be .error.

[planetis-m]

I have manually edited the file: https://gist.github.com/planetis-m/8134ab3e06e07daf4f4f238cc7a04c6e

[planetis-m]

Actually RaylibHandle is not at all necessary, a try finally block will do the trick.

[greenfork]

This is a good idea, I also wanted to implement this but never got to the specifics. I have several questions regarding this idea:

• In the very first example there will be a segfault if I do var t: Texture without the proper function to initialize. Looking at UnloadModel implementation, raylib doesn't check whether the pointers are not null. Is this correct? We will then have to write a custom implementation of the =destroy proc for each type?
• Are we going to have double frees because Model and Mesh are both destroyable and Model holds an array of Mesh objects?
• How does it work together with the new keyword? My current understanding is the new allocates the memory on the heap while var t: Texture allocates the memory on the stack. Are there any caveats with this and the new runtime?
• Is MaterialMap also a candidate for the new runtime since it holds a reference to Texture2D? And similar objects that hold a reference to an array of destroyable values? Or is this all handled automagically?

[planetis-m]

1. It seems that their Unload* functions are inconsistent, some check before freeing, most however don't. That's not an issue, we just need simple if x.field != nil: checks in our destructors.
2. There are no double frees, we override the default destructor for Model, but since it holds a ptr Mesh type the compiler doesn't track ptr[T].
3. new creates a ref object, when the ref count reaches 0, the destructor for Texture is called. We don't need to do anything, it should work.
4. The compiler creates a destructor for MaterialMap that calls the destructor for Texture, it seems to work.

[planetis-m]

So Mesh, Material, Model and ModelAnimation need if checks. However what troubles me is Shader, UnloadShader checks for rlGetShaderIdDefault but that returns 3! Meaning a var s: Shader will be freed! Anyway to prevent this?

[greenfork]

Points 2,3,4 - clear.

1 - According to comment from raysan in your PR, free(NULL) is a valid C, I assume that we don't need to check for nil values since Nim initializes ptrs to nil by default.

However what troubles me is Shader, UnloadShader checks for rlGetShaderIdDefault but that returns 3! Meaning a var s: Shader will be freed! Anyway to prevent this?

I think we can check whether shader.id == 0 since this is a fail condition for loading a shader, attention to TRACELOG in the if-branch.

So far it seems that everything is relatively clear. Would you like to implement this in a pull request? I will need some time to finish the pending #54, most probably I will just close it. I can help with anything. Currently the idea is to just add a bunch of text at the very end of raylib.nim which will going to have all the destructors since they seem quite specialized and I don't think it's worth adding an automated option as we do for most of the stuff but I'm open to suggestions. The bindings are generated automatically by parsing so this is usually a good option, see more in HACKING.

[planetis-m]

@greenfork do you think that doing:

proc `=destroy`*(x: var Shader) =
  proc UnloadShader(x: Shader) {.cdecl, importc, header: raylibHeader.}
  if x.id > 0: UnloadShader(x)

would be ok?

[greenfork]

Yeah, I think this is good. And we probably don't need to declare proc UnloadShader(x: Shader) {.cdecl, importc, header: raylibHeader.} and use already present unloadShader (I don't think we want to completely remove these functions?).

[planetis-m]

The problem is destructors need to be declared right after the type declaration, else the compiler autogenerates empty ones...

[greenfork]

https://nim-lang.org/docs/destructors.html#hook-generation Here it says that you need to declare a destructor before "use", not necessarily right after type declaration, we should be okay.

[planetis-m]

awesome you are right, I will prepare a PR!

[greenfork]

Alright, thanks! You can join nimraylib_now:matrix.org or write me on discord GreenFork#1871 if you have any questions

[greenfork]

I got this link to join https://matrix.to/#/#nimraylib_now:matrix.code0.xyz on matrix