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 Preprocessors to GDScript #7496

Open
nonunknown opened this issue Aug 14, 2023 · 19 comments · May be fixed by godotengine/godot#80619
Open

Add Preprocessors to GDScript #7496

nonunknown opened this issue Aug 14, 2023 · 19 comments · May be fixed by godotengine/godot#80619

Comments

@nonunknown
Copy link

nonunknown commented Aug 14, 2023

Describe the project you are working on

Working on Godot Kart Racing

Describe the problem or limitation you are having in your project

in order to optimize code, or have platform/editor specific code, gdscript code is the same in all situations. which leads to bloated content for end-user which isn't necessary!

Improvement of: #6340

Describe the feature / enhancement and how it helps to overcome the problem or limitation

This proposal aims to allow the user use PREPROCESSORS like in C# with code specific for each platform or editor only!

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

The preprocessors in GDScript will be fully linked with existing OS.has_feature() which means all features allowed by the engine including custom ones will be allowed to be used.

extends Node

~if debug
func _ready() -> void:
	print("debug")
	pass
	OS.has_feature()
~endif

~if editor
func _ready() -> void:
	print("editor")
	pass
	
~endif

# custom feature

~if custom_feature
func _ready() -> void:
	print("custom feature")
	pass
	
~endif 

@MBCX has a good point on not having autocomplete for non target features, my solution is having a ItemList in the script editor, where the user can select wanted features to test any situation:

image

You can choose any time you want the features for that code.

If this enhancement will not be used often, can it be worked around with a few lines of script?

nope

Is there a reason why this should be core and not an add-on in the asset library?

must modify GDScript structure

@AThousandShips
Copy link
Member

The use of ~ to denote the statements looks off to me

@Calinou
Copy link
Member

Calinou commented Aug 14, 2023

The use of ~ to denote the statements looks off to me

Since # is already taken for comments and @ is already taken for annotations, I'm not sure what else we could use. I agree we should use something else than ~ though (that's a bitwise NOT symbol).

It's pretty uncommon for languages with indent-based syntax to support preprocessor statements, so there isn't much prior art here.

@nonunknown
Copy link
Author

I tought to use ^ but its a operator too, so creativity is open here XD

@MBCX
Copy link

MBCX commented Aug 14, 2023

What about a #! like in the start of a bash script? That would make it more inline with how's in C/C++:

extends Node

#!if debug
func _ready() -> void:
	print("debug")
	pass
	OS.has_feature()
#!endif

Even though as Calinou said, it's already used for comments, to me feels most natural. And in theory, we would just check if a ! comes after the hash, and that both, as well as the statement, are joined together. So anything else, like # !Some comment. won't be considered.

@dalexeev
Copy link
Member

Hello! I made an export plugin dalexeev/gdscript-preprocessor, so I know a little about the topic. This plugin has not been properly tested and not all the features I wanted are implemented. But I do believe it could be a viable solution in production after some testing and polishing.

Quote godotengine/godot#80619:

The system doesnt affect any of gdscript internal code, basically its done by removing non used features before parsing source_code in GDScriptParser::parse

In my plugin, preprocessing is done during export, in the editor it's just comments and normal if statements. If we want to add something like this out of the box, then probably users will expect the preprocessor to also work in the editor, for example GDScript will give the correct hints.

The problem is that if you remove code before parsing by the parser, then the editor will not be able to find errors in the removed code. This will be versatile but not very convenient, you won't be able to check that your preprocessed code is correct until you run or export the project. And if the preprocessor does not remove the code in editor, then you can get false positive errors, for example, due to duplication of declarations in different branches of conditional compilation directives.

If we want to add this to the language, then a smarter parsing/analysis is needed. I think it would be not the C-style preprocessor, but the version keyword/annotation or static if/const if/when mentioned above.

@JoNax97
Copy link

JoNax97 commented Aug 14, 2023

This will be versatile but not very convenient, you won't be able to check that your preprocessed code is correct until you run or export the project. And if the preprocessor does not remove the code in editor, then you can get false positive errors, for example, due to duplication of declarations in different branches of conditional compilation directives.

This is how it works in C# IDEs, and admittedly it's a bit inconvenient, but manageable and definitely not unheard of.

@nonunknown
Copy link
Author

I already has a workaround for this, and as @JoNax97 said this is how other languages work, the code gets more gray! and you dont get autocompletion for it! Let me update the proposal with my solution!

@Vennnot
Copy link

Vennnot commented Aug 18, 2023

Is it not possible to just use / or \ ?

@MBCX
Copy link

MBCX commented Aug 18, 2023

@Vennnot / is already used for division, and \ is used to escape characters if I remember correctly.

@Jordyfel
Copy link

Jordyfel commented Aug 19, 2023

I fail to understand why this is worth using over OS.has_feature().

If I understand correctly, the benefits are that one less conditional statement is parsed at runtime, and some code can be removed at export time to make the exported project slightly smaller.

But using GDScript, an interpreted language, already creates a lot more performance overhead than one additional branching statement.

If we get AoT compiled GDScript, then it makes sense, but even then I would prefer something like having the compiler automatically inline all OS.has_feature() checks, because preprocessor syntax adds needless complexity, imo.

In that same line of thought, if we decide that the functionality is worth it now, would it not be better to have the checks "baked" at export time instead of introducing annoying preprocessor syntax in GDScript? I understand it would take more effort to implement, but it is more coherent with GDScript's simplicity prioritizing design.

@dalexeev
Copy link
Member

In that same line of thought, if we decide that the functionality is worth it now, would it not be better to have the checks "baked" at export time instead of introducing annoying macro syntax in GDScript? I understand it would take more effort to implement, but it is more coherent with GDScript's simplicity prioritizing design.

I think that in addition to performance, the point of the preprocessor is to completely remove some code during export so that it is not only not executed, but also not available to researchers.

I agree, in most cases you don't need preprocessor directives, if statements are enough. Even my plugin can remove/unwrap ifs without requiring directives inside functions. Directives are needed only for the top level, since there is no way to use ifs.

Also, my plugin takes into account comments and string literals, unlike the proposal, which introduces an additional preprocessor stage before parsing. I think it's not hard to do it inside the tokenizer. For example, you can make preprocessor directives comments starting with a special prefix (#~ or other).

But I think the C-style preprocessor is not the best solution. I would prefer static analysis of if blocks (plus there is a proposal for const if). We could implement this in the compiler, but first we need to restore bytecode export. Because at the moment there is only source code export and the only way to get different code for different export targets is to modify the source code.

I do not want to discourage the author, but in my opinion this feature should first discussed in the proposal, because there is a risk that the current implementation will not meet our goals and an implementation from scratch will be required.

@nonunknown777
Copy link

@Jordyfel thanks for you comment, the thing is that the difference between the OS.has_feature and ~if PREPROCESSOR is that the code where it doesnt belong will be removed in exported projects, also the check for the feature happens only once, not all the time for all platforms like the has_feature() function

@Shidoengie
Copy link

I think adding macros over preprocessors would be more extremely more favorable, preprocessors require learning a different syntax which greatly increases the learning curve, instead it would be more favorable to write normal code that does these things that preprocessors do

@MageCoven
Copy link

A use case for this would be when using proprietary SDKs which require an NDA, like console SDKs, since you could potentially be in legal trouble if someone reverse engineered the SDK by looking at how your game uses it. Being able to exclude code from export on some platforms or by toggling a flag could be a time saver since it otherwise would require different branches in a version control system which is a hassle and more prone to a mistake.

@idbrii
Copy link

idbrii commented Nov 3, 2023

A compile-time control construct would be much easier to read and manage code than preprocessor. I've worked a lot with C++ and nested preprocessors become very difficult to read and lead to subtle bugs with different combinations of defines. Some modern languages provide compile-time conditionals so they fit in with the syntax of the language instead of a separate system that's oblivious.

Example from #4234 (comment):

Nim uses a when keyword as a compile-time if. Maybe we can do something like this for compile-time checks in general.

when sizeof(int) == 2:
  echo "running on a 16 bit system!"
elif sizeof(int) == 4:
  echo "running on a 32 bit system!"
elif sizeof(int) == 8:
  echo "running on a 64 bit system!"
else:
  echo "cannot happen!"

And most relevant when comparing against preprocessors:

The statements that belong to the expression that evaluated to true are translated by the compiler, the other statements are not checked for semantics! However, each condition is checked for semantics.

Preprocessors generally enable monsters like:

#~if PLATFORM_IOS
if (is_jumping
#~if FEATURE_TILT
  and _is_tilting()
#~endif
):
#~else
if Input.is_action_pressed("jump"):
#~endif
  velocity += Vector3.UP

But compile-time conditionals could fit into the existing style of gdscript and would encourage clearer, more maintainable code where indents still mean something and you don't really need to understand a new way of reading code:

var want_jump := false
when PLATFORM_IOS:
  var is_tilt := false
  when FEATURE_TILT:
    is_tilt = _is_tilting()
  want_jump = is_jumping and is_tilt
else:
  want_jump = Input.is_action_pressed("jump")

if want_jump:
    velocity += Vector3.UP

On top of compile-time conditionals, you'd want to remove code that was in a false block of a conditional to provide the code stripping that preprocessors enable.

@glenfletcher
Copy link

A new compile-time control construct is not needed. Just allowing if-else and match statements in the class body (like Python) and having the compiler evaluate all const expressions at compile time would work; obviously, an if/match statement in the class body that isn't a const expression would be an error. Further, add new annotation (or keyword), @constexpr (constexpr), could be added, requiring the associated expression to be a const expression, producing a compile-time error if the compiler can't evaluate the expression similar to the use of constexpr in C++ see https://en.cppreference.com/w/cpp/language/constant_expression.

This would also serve as selective optimization, i.e. for most platforms, OS.get_name() could be defined as a const expression; thus, even without using the @constexpr, the following match would be eliminated:

match OS.get_name():
	"Windows":
		print("Welcome to Windows!")
	"macOS":
		print("Welcome to macOS!")
	"Linux", "FreeBSD", "NetBSD", "OpenBSD", "BSD":
		print("Welcome to Linux/BSD!")
	"Android":
		print("Welcome to Android!")
	"iOS":
		print("Welcome to iOS!")
	"Web":
		print("Welcome to the Web!")

Here is an example from Python of using an if statement in the class body,

import random
for i in range(10):
    print(f"Run {i+1}")
    FLAG = random.random() > 0.5
    class Test:
        if FLAG:
            def run(self):
                print("Hello")
        else:
            def do(self):
                print("Goodbye")
    test = Test()
    try:
        print("Trying test.run()")
        test.run()
    except Exception as e:
        print(f"ERROR: {e!s}")
    try:
        print("Trying test.do()")
        test.do()
    except Exception as e:
        print(f"ERROR: {e!s}")

@PerfectlyFineCode
Copy link

PerfectlyFineCode commented Jun 17, 2024

Seriously. This is much needed.
As currently I'm facing an issue with Imgui as Imgui.NET has no support for Android/IOS the Imgui script is no longer available to use and completely errors out on mobile builds.
image

If preprocessors existed it wouldn't have happened as that would be a simple

#if DESKTOP
     Imgui code here
#endif

However now it simply don't work unless I have missed something.

@m21-cerutti
Copy link

Would be great also for stripping some node logic from scene, like cheat or debug logic very easily. The best would be to have a editor flag in the node, but the preprocessor directive could solve it (and be more versatile).

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

Successfully merging a pull request may close this issue.