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

'version' keyword in GDScript for conditional compilation at build time #26649

Closed
matthew1006 opened this issue Mar 5, 2019 · 18 comments
Closed

Comments

@matthew1006
Copy link
Contributor

matthew1006 commented Mar 5, 2019

Edit: The original idea was to do the optimisations when compiling GDScript to bytecode but I realised that it makes more sense to instead do conditional AST generation to allow for optimisation even when interpreted.

I would like to propose adding version as a keyword in GDScript to allow conditional code execution and compilation based on the OS and custom features that are available.

At first glance
version (“feature”):
appears to just be syntactic sugar for
if OS.has_feature (“feature”):
and when interpreted this would be the case.

The true advantage of the version keyword would be when GDScript is compiled to gdc bytecode.
The version keyword could be detected at compile time and used to conditionally compile blocks of code depending on the feature set of the chosen export preset/template.
This would result in faster runtime speeds, smaller bytecodes and potentially faster compile times as code that is never used by your exported game will be skipped.
It also (in my opinion) results in better looking code that is arguably easier to maintain.

Here are some psudocode examples of use cases along with their current, run time counterparts:

Debug statements:
Run time

func my_function (value):
    if OS.is_debug_build():
        print(“Some debug text: Arg=”, str(value))

    #Function body here

    if OS.is_debug_build():
        print(“Result: “, result)

    return result

Compile time

func my_function (value):
    version(“debug”):
        print(“Some debug text: Arg=”, str(value))

    #Function body here

    version(“debug”):
        print(“Result: “, result)

    return result

Different code for different platforms:
Run time

#Platform independent code here

if OS.has_feature(“android”):
    #Android code here

if OS.has_feature(“ios”):
    #iOS code here

if OS.has_feature(“web”):
    #Web code here

Compile time

#Platform independent code here

version(“android”):
    #Android code here

version(“ios”):
    #iOS code here

version(“web”):
    #Web code here

Custom features & else:
Run time

if OS.has_feature(“my_custom_feature”):
    #Do something
else:
    #Do something else

Compile time

version(“my_custom_feature”):
    #Do something
else:
    #Do something else

Tool scripts:
Run time

func _draw ():
    if Engine.editor_hint:
        #Draw lines and stuff

Compile time

func _draw ():
    version(“editor”):
        #Draw lines and stuff

or

version(“editor”): # _draw function not included at all so it doesn't even get called
    func _draw ():
        #Draw lines and stuff

At compile time when a version block is found the compiler can check if the specified feature is available on the export platform (OS.has_feature) and either include the code in the block or ignore it meaning no runtime checks are required.
e.g when exporting for android

#Platform independent code here

version(“android”):
    #Android code here

version(“ios”):
    #iOS code here

version(“web”):
    #Web code here

compiles as

#Platform independent code here

#Android code here

completely omitting the iOS and web code from the compiled bytecode.

This proposal is based on the version keyword featured in D https://dlang.org/spec/version.html

@Chaosus Chaosus changed the title Feature Proposal: 'version' keyword in GDScript for conditional compilation at build time 'version' keyword in GDScript for conditional compilation at build time Mar 5, 2019
@bojidar-bg
Copy link
Contributor

Was also proposed/discussed in the now-closed #6340.

@matthew1006
Copy link
Contributor Author

I didn't spot that one when searching. Not much point keeping this open then so closing.

@bojidar-bg
Copy link
Contributor

Reopening. Note the other issue is closed; there can still be merit in discussing this one.

@bojidar-bg bojidar-bg reopened this Mar 5, 2019
@Zylann
Copy link
Contributor

Zylann commented Mar 6, 2019

So... that's basically a specialized #ifdef. Conditional script parsing is a recurring theme, notably in #26177 as well.
I like the debug-only tag, that occurs a lot in my projects and it can be tedious to chase after all debug-only code to ensure to turn off or remove before export, not to mention the fact these debug-only if sections should be carefully placed so they are not evaluated too much like in tight loops.
Also, expecting only one tag in version won't cut cases like "Not Android", "Mobile only", "OSX and Linux", or "Godot version from 3.1 to 3.2.1". I know these use cases exist because I saw them in existing games I worked on, and particularly plugins and libraries which have to support wider ranges of versions (I myself do that too with regular ifs and has_method at the moment).
I also had to break compatibility in one of my plugins because a virtual function signature started including a default argument, which made the script fail to even parse. Conditional compilation isn't pretty in that case, but at least allows to maintain compatibility.

@matthew1006
Copy link
Contributor Author

Perhaps something like D's static if would be more appropriate then. It would allow of more complex conditions such as static if not OS.has_feature(“android”) or static if OS.has_feature(“OSX”) or OS.has_feature(“X11”) although there is no reason that we couldn’t have something like this:

version(“android”) and not version(“x86”):
    #somecode

vs

static if OS.has_version(“android”) and not OS.has_version(“x86”)

but static if does allow for stuff like this:

static if false:
    #Doesn’t matter what’s here; It will not be compiled

I think that version is a better solution than static if because it can specifically focus on the availability features rather than arbitrary conditions that would require compile time code execution, is less verbose, is less likely to cause confusion (‘what’s the difference between if and static if?’ asked every new user for all of time) and should not result in issues caused by values not being known at compile time as the only valid argument type is a string constant.

As for checking the engine version that could be added to OS.has_feature i.e. OS.has_feature(“3.1”) or OS.has_feature(“>=3.0”) or something like that and assuming that version is just a compile time check of OS.has_feature it could be used like so:

version(“>=3.1”) and version(“<=3.2.1”):
    #Code only for Godot Versions 3.1 to 3.2.1

In comparison to #ifdef and #ifndef an important difference between the C preprocessor and D’s version and static if is that the C preprocessor has to run over all of the code first before compiling happens while D does it all in place during the compilation step AFAIK. I’m proposing to do this the D way, in a single pass as part of the compilation step with no preprocessor required. If the condition isn't met then just skip the block entirely.

Now that I think about it there would need to be warnings for when code outside of a version block references code inside the block as that code may not be included in the build and the version block likely wouldn’t have its own scope.

version (“android”):
    var x = 10

print(x) # Error. Variable x undefined on non-android platforms.

@matthew1006
Copy link
Contributor Author

After some thinking I realised that this could work by skipping unused branches when building the AST. This would make version more efficient than if even when interpreted and not just when the code is compiled to bytecode as if the feature specified in the version statement isn’t supported on that platform then that entire branch of the AST is never generated.

@dalexeev
Copy link
Member

dalexeev commented Oct 6, 2019

I would prefer a shorter syntax, for example:

$$debug:
    print("debug")

$$mobile:
    print("mobile")

@Calinou
Copy link
Member

Calinou commented Oct 6, 2019

@dalexeev In GDScript, we tend to favor easy-to-read constructs. Code is read way more often than it is written 🙂

@dalexeev
Copy link
Member

dalexeev commented Oct 6, 2019

@Calinou There is already a short syntax for get_node(). The dollar sign is often used in programming languages to denote a variable. Feature tags are also kind of variables: the variables that are used when exporting a build. Why not use two dollar signs?

Also easiness is a relative concept:

#!ifdef debug
func _ready():
    # ...
#!endif

In any case, the functionality is primary, not the name.

ADD. @Calinou Which is easier to read: typeof(a) == typeof(b) and a == b OR a == b (#26375)?

@dalexeev
Copy link
Member

dalexeev commented Oct 6, 2019

At the very least, it would be nice to give up quotes and brackets. And it is advisable to choose a shorter keyword.

version debug:
    print("debug")

only debug:
    print("debug")

at debug:
    print("debug")

@Calinou
Copy link
Member

Calinou commented Oct 6, 2019

Nim uses the when keyword as a compile-time if statement, we could borrow that to use a more generic keyword.

@matthew1006
Copy link
Contributor Author

I think that we should focus on clarity rather than verbosity. Personally, I think that version provides the clearest explanation of what is happening in the code (only works too). If a symbol were to be used then it should be shorthand for a keyword and not similar to an existing symbol. I think that $$ is too similar to $ and would make code confusing for people who are unfamiliar with it. I don't think @ is used anywhere, that could be used as a shorthand if there is demand for it.

version Windows:
    Windows Stuff

version X11:
    Linux and BSD Stuff

version OSX:
    Mac Stuff

or

@32:
    const MAX_INT = 2147483647

@64:
    const MAX_INT = 9223372036854775807

@Calinou
Copy link
Member

Calinou commented Oct 7, 2019

I don't think @ is used anywhere

Actually, it's used to create NodePaths 🙂

@matthew1006
Copy link
Contributor Author

I don't think @ is used anywhere

Actually, it's used to create NodePaths slightly_smiling_face

You learn something new everyday.
How about ?

?debug:
    print("debug")
?release:
    print("release")

@Calinou
Copy link
Member

Calinou commented Oct 7, 2019

@matthew1006 We should prefer using fully spelled-out keywords to symbols for language features that aren't used very often.

@matthew1006
Copy link
Contributor Author

matthew1006 commented Oct 7, 2019

@Calinou I agree. I was just humouring some of the other suggestions for an alternative shorthand.

@dalexeev

This comment has been minimized.

@clayjohn
Copy link
Member

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants