nimraylib_now icon indicating copy to clipboard operation
nimraylib_now copied to clipboard

Generate bindings from the JSON files

Open planetis-m opened this issue 4 years ago • 10 comments

planetis-m avatar Dec 27 '21 12:12 planetis-m

I don't know if you agree with the last change but I decided to produce distinct cint instead of enums. Reasons are the usual: duplicate values, enums with holes used as bitsets. The last straw for me was having to support MaterialMapDiffuse and ShaderLocDiffuse, without the prefixes there are name collisions...

planetis-m avatar Jan 03 '22 22:01 planetis-m

The last straw for me was having to support MaterialMapDiffuse and ShaderLocDiffuse, without the prefixes there are name collisions...

If I'm not mistaken, there are a bunch of #define statements in C file for a bunch of things. But they all serve only a backwards-compatibility purpose because of renames. I think we should strive for better Nim and put backwards-compatibility about such renames in the Changelog notes or just special case them. At least this is a strategy I was going for so far. Looks like it works fine, I don't get a lot of complaints when the changes are reasonable.

If we don't try to be fully backwards-compatible, are we okay with overloadable enums as discussed in https://github.com/greenfork/nimraylib_now/issues/55?

greenfork avatar Jan 19 '22 05:01 greenfork

Saw latest changes, looks really great

greenfork avatar Jan 20 '22 15:01 greenfork

@planetis-m I found these functions which are not in the set of destructors in https://github.com/greenfork/nimraylib_now/pull/60, do we include them as well or there are some problems?

unloadVrStereoConfig*(config: VrStereoConfig)
unloadFileData*(data: ptr uint8)
unloadFileText*(text: cstring)
unloadImageColors*(colors: ptr Color)
unloadImagePalette*(colors: ptr Color)
unloadFontData*(chars: ptr GlyphInfo; glyphCount: cint)
unloadCodepoints*(codepoints: ptr cint)
unloadWaveSamples*(samples: ptr cfloat)
  • unloadVrStereoConfig seems alright to add
  • unloadFileData and unloadFileText seem too general
  • unloadImageColors and unloadImagePalette can use distinct ptr Color for load... functions
  • unloadFontData can partially use raylib_fields.nim accessors like []?
  • unloadCodepoints could use distinct ptr cint and related to https://github.com/greenfork/nimraylib_now/issues/69
  • unloadWaveSamples could use distinct ptr cfloat

Distinct types will have to also convert other function signatures which use these types though. And it is less maintainable regarding our automation feature of c2nim. Maybe there are some tricks we could use to avoid a lot of text replacement and somehow do it more smartly.

greenfork avatar Feb 04 '22 19:02 greenfork

  • unloadVrStereoConfig I removed it, its a noop for now.
  • unloadWaveSamples just a for loop that calls unloadWaveSample, since we used seq[Wave] it was not need.
  • unloadImageColors, unloadFontData, unloadImagePalette I think same as above
  • unloadFileData, unloadFileText it's hard to tell what to do with these, could we instead use std equivalents instead of LoadFile*
  • unloadCodepoints haven't decided what to do with codepoints yet.

planetis-m avatar Feb 05 '22 10:02 planetis-m

@planetis-m I made an attempt at using destructors https://github.com/greenfork/nimraylib_now/pull/72

greenfork avatar Feb 07 '22 15:02 greenfork

Does raylib_fields.nim work now? There's a comment about .copy pragma and I'm not sure what's going to be the best fix for this file. Just adding .copy to each proc with var parameter?

greenfork avatar Feb 07 '22 15:02 greenfork

They work, I used templates as this comment suggests https://forum.nim-lang.org/t/8871#57987 but for the [] proc the problem is still there, but it won't cause any issue. So it's good to go.

Just adding .copy to each proc with var parameter?

Not yet implemented but it's a good idea imo.

planetis-m avatar Feb 07 '22 21:02 planetis-m

On each type c2nim automatically adds bycopy pragma. I would assume that this would fix the same issue?

greenfork avatar Feb 08 '22 05:02 greenfork

Yes but now .bycopy would only be added in the .importc procs and that would allow to write a native Nim procedure using .importc types without any issue.

planetis-m avatar Feb 09 '22 10:02 planetis-m