godot-nim / gdext-nim

Godot GDExtension binding for Nim
MIT License
34 stars 3 forks source link

Less boilerplate for exporting/type safe exporting #76

Open gushromp opened 6 days ago

gushromp commented 6 days ago

Hi!

I think it would be nice to add something like:

macro `@export_field`*(iden: typed) =
  iden.expectKind nnkDotExpr
  let class = iden[0]
  let variable = iden[1]
  let varType = iden.getType[1]
  let varName = variable.toStrLit
  result = quote("!") do:
    `@export` `!varName`, 
      proc(self: `!class`): `!varType` = self.`!variable`, 
      proc(self: `!class`, value: `!varType`) = self.`!variable` = value

Usage:

type Plane* = ref object of CharacterBody3D
  pitchSpeed*: float32

`@export_field` Plane.pitchSpeed

Now my Nim-macro-fu isn't the best, but the above works as-is, though it could probably be simplified/prettyfied.

I think this approach would be preferable for two reasons: 1) Less boilerplate, I think having a custom getter/setter should be opt-in and not default behavior 2) Type safety - the snippet above provides compile time safety, passing some non-existant identifier instead of Plane.pitchSpeed results in a compilation error.

Perhaps an even simpler approach would be to add a pragma to the variable itself, so something like:

type Plane* = ref object of CharacterBody3D
  pitchSpeed* {.export.}: float32

What do you think?

gushromp commented 6 days ago

I made a very crude prototype:

# userclass.nim

template gdexport*() {.pragma.} 

macro processExports(T: typed): untyped =
  let innerType = T.getType[1][1]
  let objType = innerType.getImpl
  let fields = objType[2][2]

  let module = objType[0].owner

  var stmts = newStmtList()
  for field in fields:
    if field[0].kind != nnkPragmaExpr:
      continue

    let pragma = field[0][1]
    let pragmaIdent = strip($pragma.toStrLit)

    if pragmaIdent == "{.gdexport.}":
      let objTypeInner = objType[0]
      var objTypeStr = $objTypeInner.toStrLit
      let index = objTypeStr.find(":")
      objTypeStr = objTypeStr[0..<index]

      var fieldName = $field[0][0].toStrLit
      fieldName.removeSuffix("*")

      let classIdent = newIdentNode(objTypeStr)
      let fieldIdent = newIdentNode(fieldName)

      let dotExpr = newDotExpr(newDotExpr(module, classIdent), fieldIdent)
      stmts.add quote("!") do:
          `@export_field` `!dotExpr`

  result = stmts

var registered: HashSet[StringName]
proc register*(T: typedesc) =
  let info = T.creationInfo(false, false)
  when Extension.version == (4, 1):
    interface_ClassDB_registerExtensionClass(environment.library, addr className(T), addr className(T.Super), addr info)
  else:
    interface_ClassDB_registerExtensionClass2(environment.library, addr className(T), addr className(T.Super), addr info)
  processExports T # <- new
  invokeContract T
  registered.incl className(T)

Usage:

type Plane* = ref object of CharacterBody3D
  pitchSpeed* {.gdexport.}: float32 = 1.1
  rollSpeed* {.gdexport.}: float32 = 2.5
  name* {.gdexport.}: string = "asdf"
  levelSpeed*: float32 = 4.0      # Not exported
  forwardSpeed*: float32 = 25.0   # Not exported
panno8M commented 6 days ago

Hello! Thanks for participating in the development.

I think it's a very nice feature. I dread the thought of writing a getter/setter every time to expose a property:(

I don't know yet if I can make it work in practice, but it would be great if I could unify the three names: get/set version of exporter, object.param version of exporter, and export pragma.

type Plane* = ref object of CharacterBody3D
  pitchSpeed*: float32
`@export` pitchSpeed,
  proc (self: Plane): float32 = self.pitchSpeed,
  proc (self: Plane; value: float32) = self.pitchSpeed = value,
type Plane* = ref object of CharacterBody3D
  pitchSpeed*: float32
`@export` Plane.pitchSpeed
type Plane* = ref object of CharacterBody3D
  pitchSpeed* {.`@export`.}: float32

If this is feasible, other exporters such as @export_custom and @export_range can be overloaded as well, simplifying them while retaining their inherent flexibility.

Anyway, I am very positive about this feature. Let's continue in PR.

P.S. I have granted you the write role so you can work directly off branch instead of folk.

FYI: Repository roles Favor branching over forking

gushromp commented 6 days ago

You are welcome!

The only thing from your list that doesn't seem possible is the last item:

type Plane* = ref object of CharacterBody3D
  pitchSpeed* {.`@export`.}: float32

Because field pragmas in Nim cannot be backtick quoted :( Perhaps this can be filed as a bug in the language as it doesn't seem to be by design.

Until then the only way to unify this pragma and the macros/templates would be to rename all of them from @export[_*] to something like gdexport[_*] :/

I assume you wanted it to be @export to closely resemble GDScript, but even in C# the annotation is just [Export] so I suppose renaming it a little bit isn't considered unidiomatic in terms of Godot guidelines.

I'm working on a PR for all the rest :)

panno8M commented 6 days ago

Because field pragmas in Nim cannot be backtick quoted :(

Ahh I've not know that. Then it's a bit aggressive, but let's use that PR to rename them to gdexport.*. I know it's a hassle, so you can push the renaming process to me.

gushromp commented 5 days ago

@panno8M

Hi! Just a quick update.

I've basically finished unifying the export mechanisms: 1) Same as before, with custom name, getter and setter 2) Passing a field and args (if any) 3) Pragma on the fields themselves with args (if any) 4) Renamed all export templates/macros/pragmas to gdexport[_*] 5) Added type-safety on all export kinds. So for example a gdexport_enum can only be applied to a field of type SomeInt where type SomeInt* = concept type t -> t is int|Int|PackedByteArray|PackedInt32Array|PackedInt64Array|Array 6) Automatic detection of object-type exports using default gdexport which implicitly are converted to NodePaths (same as GDScript since Godot 4). You can see that in action in the otherNode field below

Here's a quick sample:

type Plane* = ref object of CharacterBody3D
  pitchSpeed* {.gdexport.}: float32 = 1.1
  rollSpeed* {.gdexport.}: float32 = 2.5
  levelSpeed* {.gdexport.}: float32 = 4.0 
  forwardSpeed* {.gdexport.}: float32 = 25.0 
  otherNode* {.gdexport.}: Camera3D 
  enm* {.gdexport_enum(cases = "CaseA", "CaseB") .}: int 
  flags* {.gdexport_flags(flags = "Flag1", "Flag2") .}: int 
  multiline* {.gdexport_multiline.}: String
  eas* {.gdexport.} : Float
  color_no_alpha* {.gdexport_color_no_alpha.}: Color = color(1.0, 0.0, 0.0, 1.0)
  color* {.gdexport.}: Color
  navflags* {.gdexport_flags_avoidance.}: int

  color2*: Color = color(1.0, 0.0, 0.0)
  multiline2*: string = "Some\nmultiline\nstring"

gdexport_color_no_alpha Plane.color2
gdexport_multiline Plane.multiline2

Which produces:

slika

I just have to tidy up the code a little bit (especially the macros, ick) and I'll be creating a PR soon, hopefully tomorrow.

Cheers!

panno8M commented 5 days ago

Thank you for the interim report. You did it so far man, that exceeded my expects XD From that summary, it seems there is nothing I should to say.

Looking forward to your PR!

gushromp commented 2 days ago

I've made some more progress:

import gdext
import gdext/classes/gdResourceLoader

type PropTestPragmasEnum* = enum
  PropTestPragmasEnum1, PropTestPragmasEnum2, PropTestPragmasEnum3

type PropTestNodePragmas* = ptr object of Node
  icon* {.gdexport.}: gdref Texture2D
  PropTestEnum_with_export* {.gdexport.}: PropTestPragmasEnum
  string_with_export* {.gdexport.}: string = "with export"
  string_with_export_placeholder* {.gdexport.}: string
  string_with_export_dir* {.gdexport_dir.}: string = "res://nim"
  string_with_export_global_dir* {.gdexport_global_dir.}: string = "/dev"
  string_with_export_file* {.gdexport_file.}: string = "res://nim/bootstrap.nim"
  string_with_export_global_file* {.gdexport_global_file.}: string = "/dev/null"
  int_with_export_enum* 
    {.gdexport_enum(cases = "Alpha", "Beta:10", "Gamma").}
    : int
  string_with_export_enum* 
    {.gdexport_enum(cases = "Alpha", "Beta", "Gamma").}
    : string = "Alpha"
  int_with_export_flags* 
    {.gdexport_flags(flags = "Alpha", "Beta", "Gamma").}
    : int
  int_with_export_flags_some_layers*: int # multiple exports below 
  float_with_export_exp_easing* {.gdexport_exp_easing: positive_only.}: float = 2
  string_with_export_multiline* {.gdexport_multiline.} : string = """
MULTILINE-TEXT MULTILINE-TEXT MULTILINE-TEXT
MULTILINE-TEXT MULTILINE-TEXT MULTILINE-TEXT
MULTILINE-TEXT MULTILINE-TEXT MULTILINE-TEXT
MULTILINE-TEXT MULTILINE-TEXT MULTILINE-TEXT"""
  StringArray_with_export_multiline*: TypedArray[String]
  PackedStringArray_with_export_multiline*: PackedStringArray
  NodePath_with_export_node_path* {.gdexport.}: NodePath
  string_with_export_storage* {.gdexport_storage.}: string = "with export_storage"
  int_with_export_strict_range*: int = 20
  int_with_export_range*: int = 20
  radians_with_export_range_as_degrees*: float = PI/4
  color_with_export* {.gdexport.}: Color = color(1, 1, 1, 0.5)
  color_with_export_no_alpha* {.gdexport_color_no_alpha.}: Color = color(1, 1, 1)

proc get_string_with_export_through_proc(self: PropTestNodePragmas): string {.gdsync.} =
  self.string_with_export
proc set_string_with_export_through_proc(self: PropTestNodePragmas; value: string) {.gdsync.} =
  self.string_with_export = value
gdexport "string_with_export_through_proc",
    get_string_with_export_through_proc,
    set_string_with_export_through_proc

gdexport_flags_2d_navigation PropTestNodePragmas.int_with_export_flags_some_layers, alias "int_with_export_flags_2d_navigation"
gdexport_flags_2d_physics PropTestNodePragmas.int_with_export_flags_some_layers, alias "int_with_export_flags_2d_physics"
gdexport_flags_2d_render PropTestNodePragmas.int_with_export_flags_some_layers, alias "int_with_export_flags_2d_render"
gdexport_flags_3d_navigation PropTestNodePragmas.int_with_export_flags_some_layers, alias "int_with_export_flags_3d_navigation"
gdexport_flags_3d_physics PropTestNodePragmas.int_with_export_flags_some_layers, alias "int_with_export_flags_3d_physics"
gdexport_flags_3d_render PropTestNodePragmas.int_with_export_flags_some_layers, alias "int_with_export_flags_3d_render"
gdexport_flags_avoidance PropTestNodePragmas.int_with_export_flags_some_layers, alias "int_with_export_flags_avoidance"

image

image


I've added automatic enum registration, and fixed an issue where two classes couldn't have same field names (due to the getters/setters being synthesized as lambdas)

Also added alias to support setting multiple exports of the same field via the new exporting mechanics.

I just have to add support for ranges, groups and subgroups and figure out a way to preserve correct ordering (as you can see, the pragmas are evaluated last currently, so the properties end up on the bottom), then the code should be ready for a PR.

Cheers!

gushromp commented 2 days ago

Oh and I reported the issue of using backticks with pragmas on the Nim forum, and it already got fixed and merged: https://github.com/nim-lang/Nim/pull/24151

So if you prefer the old naming scheme we can revert it :) We would have to wait for a new Nim release or switch to nim-devel in the interrim

panno8M commented 2 days ago

@gushromp

Nice. Good progress.

figure out a way to preserve correct ordering...

I've updated user-class contracts #85 Now we can register pragma'd props early with using {.execon: Contract[T].pre_property.}. You may need to overload the register_property macro to take advantage of this, but it should make implementation a little easier.

So if you prefer the old naming scheme we can revert it :)

Thanks for reporting the issue! On second thought, backtick looks bad, so I don't see the need to revert it.