Make build system more friendly when it's no longer top-level#1358
Make build system more friendly when it's no longer top-level#1358Kehom wants to merge 1 commit intogodotengine:masterfrom
Conversation
tools/godotcpp.py
Outdated
| ) | ||
| ) | ||
|
|
||
| opts.Add("profile", "Allow specification of customization file other than only custom.py", "") |
There was a problem hiding this comment.
| opts.Add("profile", "Allow specification of customization file other than only custom.py", "") | |
| opts.Add( | |
| PathVariable( | |
| key="profile", | |
| help="Allow specification of customization file other than `custom.py`.", | |
| default="" | |
| ) | |
| ) |
tools/godotcpp.py
Outdated
| PathVariable( | ||
| key="profile", | ||
| help="Allow specification of customization file other than `custom.py`.", | ||
| default="" |
There was a problem hiding this comment.
| default="" | |
| default=None, | |
| validator=validate_file, |
My bad didn't notice the other case, might also be best to do:
| default="" | |
| default=env.get("profile", None), | |
| validator=validate_file, |
SConstruct
Outdated
| customs.append(profile + ".py") | ||
| if not profile.endswith('.py'): | ||
| profile += '.py' | ||
|
|
| try: | ||
| customs += Import("customs") | ||
| except: | ||
| pass |
There was a problem hiding this comment.
This functionality seems to be completely removed - I don't see it re-added later.
See #1196 for some background on this
There was a problem hiding this comment.
Yes, I wasn't sure how to implement that functionality. But I will wait until we get some feedback (as you have requested) before performing the change + rebase.
|
I like the idea behind this functionality! We don't want to show the "unknown variables" message when used in another project that defines it's own variables. However, I think we'll need someone who is more in tune with the build system to say if this is done in the right way. @Faless @adamscott What do you think? |
|
I like the idea of having a standardized way to show "unknown variables" message. What I would suggest is creating a hook for modifying godot-cpp scons options. It could be implemented in a similar way to how the "custom_tools" is implemented. A PathVariable for example with a name "custom_options" is added here https://github.com/godotengine/godot-cpp/blob/master/tools/godotcpp.py#L222 This way the user will have a way to add their own custom options and will not loose the ability to see "unknown variables" message when inputting something unexpected. |
This PR aims to address #1334 at least partially. With the changes:
profile=some_file.pyis given as argument, that file can be placed alongside the extension SConstruct directory rather than within the godot-cpp. Without the changed script, in order to keep thesome_file.pywith the extension then the argument must be changed toprofile=../some_file.py(in other words, the path would have to be relative to godot-cpp SConstruct)A side note here. I kept the original
profile=...logic of accepting file name without.pyextension. However I have noticed that when this is the case, SCons generate a binary file with the exact same name of the profile file, but without any extension. As an example, if there is arelease.pyand the argument is given asprofile=release, then areleasefile will be created. I was unable to track down what/where this thing happens.