Skip to content
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

Add additional macros #129

Merged
merged 5 commits into from Sep 27, 2023
Merged

Conversation

alicerunsonfedora
Copy link
Contributor

@alicerunsonfedora alicerunsonfedora commented Sep 26, 2023

I'm introducing the macro library from Indexing Your Heart's source upstream since I think they will benefit developers using this package to create scripts for Godot:

  • @NativeHandleDiscarding is a macro that initializes init(nativeHandle:) for you. This isn't meant to supersede the existing @Godot macro, but it can be used if developers prefer to do things manually.
  • #initSwiftExtension is a macro for quickly setting up an extension by writing the entrypoint methods for you, only requiring the C name and the array of types to be registered with the Godot engine.
  • #texture2DLiteral is a macro for quickly fetching an image resource and using it as a texture.
  • @PickerNameProvider is a macro for giving an enum the backing and conformances needed to register them in Godot.
  • @SceneTree is a macro that adds a get accessor to properties, referencing the scene tree. Consider this a replacement of the existing @BindNode property wrapper.

I've also updated the tutorials to utilize the new macros.

Note
Indexing Your Heart is licensed under the CNPLv7, and these macros were a part of that in a separate package. However, because I am the sole author of the game at the moment with no contributors, I have the authority to have them relicensed under SwiftGodot's existing license. I also highly doubt they can be used to harm someone directly.

Port over the macros meant to go upstream. As the sole author of the
macro library, I give permission to have these licensed under the same
license as the SwiftGodot project.
Utilizes thew new macros introduced in the previous commit.
preconditionFailure(
"Texture could not be loaded.",
file: "TestModule/test.swift",
line: 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, is there a way to see this "Texture could not be loaded." text when the game hits it? Any fatalError / preconditionFailures I've tried just leads to a crash in godot, and even the crash doesn't seem to show the actual failure text.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to my knowledge, but I could call a GD.pushError beforehand so that the error is present in the Godot console before the crash.

Copy link
Contributor

@PadraigK PadraigK Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting back to me! I just tried this and the error doesn't make it to the Godot console unfortunately. No worries anyway, this is not a reason to change the PR, which is really cool overall! Thanks for sharing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I guess it must happen so quickly that Godot doesn't even have the chance to report the error. Though I wonder if it appears in the stacktrace or logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appears this might require a flag of some kind:

message.
A string to print in a playground or -Onone build. The default is an empty string.

https://developer.apple.com/documentation/swift/preconditionfailure(_:file:line:)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A debug build should be -Onone already, I suspect it's dying before the logs are flushed. I'll open a separate issue to discuss this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #132 to discuss.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PadraigK Perhaps you can try replacing my GD.pushError with Swift's default print and see if you get something in the terminal?

Co-authored-by: Padraig  <padraig.kennedy@gmail.com>
This should emit a message to the Godot console before crashing that a
texture couldn't be loaded, with the respective file and line.
This is missing the SwiftGodotMacros import (see migueldeicaza#131).
@PadraigK
Copy link
Contributor

To get this working in my project, I had to add .product(name: "SwiftGodotMacros", package: "SwiftGodot") to my .target dependencies. It would be good to add this to the tutorial where it describes the setup of the driver package.

import SwiftSyntaxBuilder
import SwiftSyntaxMacros

public struct SceneTreeMacro: AccessorMacro {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still a bit concerned that we have duplicate functionality now, and would rather only have one way of doing things.

I am ok taking this over my existing solution, but we should at least make this default the path to being the name of the variable, so users do not have to repeat themselves, something like:

@SceneTree var myTimer: Timer

Should infer that the name of the timer in the scene is myTimer

@migueldeicaza migueldeicaza merged commit 966ef3d into migueldeicaza:main Sep 27, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants