- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 3.5k
 
          Document relative paths in .gdextension files being supported
          #11381
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
  
    Document relative paths in .gdextension files being supported
  
  #11381
              Conversation
5ccec64    to
    78747d7      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to switch the example to use relative names.
We discussed this in GDExtension meetings before, and concluded (iirc) that relative paths are preferable since this supports moving around and renaming the gdextension without having to worry about the .gdextension file. This is also the reason we changed it in the template.
I don't think there are any advantages to using absolute paths, so I'm not sure I'd even mention them.
78747d7    to
    72fcb7b      
    Compare
  
    
          
 Done.  | 
    
72fcb7b    to
    c0ebfbc      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍🏻
| macos.debug = "bin/libgdexample.macos.template_debug.framework" ; Inline comments are also allowed. | ||
| macos.release = "bin/libgdexample.macos.template_release.framework" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the template, we did all the paths starting with ./, which, if I remember correctly was to fix a MacOS issue? And we also use .dylib in the template and not .framework, which I also remember discussing being to workaround some issue.
Do you folks remember the details?
Anyway, if there was a reason for those changes, we should do them in the docs as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Yea, I believe the macOS load issue is still around, so changing to ./ format would be good.
And yea, the change from framework to dylib also definitely makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Calinou ^ Gentle poke :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I've also taken the opportunity to change some paths that were still absolute elsewhere in the documentation, as well as fix some notes on feature tags.
- Mention support for comments in `.gdextension` files. - List recommended import options for GDExtension icons.
c0ebfbc    to
    386c79f      
    Compare
  
    | macos.debug = { | ||
| "res://bin/libdependency.macos.template_debug.framework" : "Contents/Frameworks" | ||
| "./bin/libdependency.macos.template_debug.dylib" : "Contents/Frameworks" | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if I should have changed this part; let me know if I should revert that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That change is included in #11405, and it requires changes to the SConstruct too. So better revert it in this PR.
.gdextensionfiles.