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 a new setting to export presets for custom C# preprocessor defines #11451

Open
Grimeh opened this issue Dec 30, 2024 · 4 comments
Open

Add a new setting to export presets for custom C# preprocessor defines #11451

Grimeh opened this issue Dec 30, 2024 · 4 comments

Comments

@Grimeh
Copy link

Grimeh commented Dec 30, 2024

Describe the project you are working on

I'm currently working on a couple Godot projects that need some form of preprocessor define manipulation, however it's applicable to any project that uses a non-trivial amount of C#.

Some examples of shipped titles I've worked on that have made extensive use of this feature in other engines during development:

  • survival RTS (PC (Steam + EGS))
  • VR platformer (Quest 2/3, PSVR2)
  • mobile social party game (mobile, web)
  • visual novel (Nintendo Switch, PC/MacOS (Steam + Itch))

As you can see, it's applicable to a wide variety of genres and platforms.

This feature would also be useful for GDScript but that's dependent on preprocessors being implemented. For that reason I'll ignore it in this issue as out-of-scope.

Sidenote: I'm going to use "defines", "preprocessor defines", and "define constants" interchangeably to refer to preprocessor defines.

Describe the problem or limitation you are having in your project

There is currently no way to define a list of custom preprocessor defines in export presets (or anywhere in the editor).

This makes it extremely tedious and error prone to change build targets, as the user must edit the .csproj (in an IDE or by hand) to remove and add defines before exporting for a specific platform.

Also, a user manually overriding ConstantDefines in the .csproj can quickly run into issues since it's not immediately obvious that you must include $(ConstantDefines) to preserve the Godot-provided defines. This is currently not covered by docs but there is an open issue to rectify this.

Use cases discussed in later section.

Existing Work

Below are some related issues/PRs I found after some searching, I don't believe any of them fully satisfy the lack of this feature, so I decided to open a new proposal.

Support the addition of custom preprocessor defines in C# project files
Project-wide custom defines which would be superseded by the implementation of this feature.
PR: godotengine/godot#94939

Document the management of .csproj files in .NET projects
The contents of such docs would need to be updated if this feature is implemented in the matter described below.

Dotnet: Allow passing custom build arguments
Custom build arguments could benefit from being implemented as an export preset setting instead of a project-wide setting, but it doesn't necessarily overlap with this.
PR: godotengine/godot#93539

Adding option to use C# DefineConstants when exporting
This covers having per-export-preset custom defines, but doesn't address the workflow issues. The custom defines aren't added to project configuration and so IDEs can't pick up on them. It also doesn't offer a way to switch between custom defines in different export presets.

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

In a new tab in the settings of an export preset or under the "Scripts" tab:

  • add a section titled "Preprocessor Defines", or "Scripting Defines", or "Defines"
  • in which custom C# preprocessor defines can be added/removed/edited for the selected export preset
  • defines are listed one-per-line
  • defines can be added or removed easily
  • adding or removing a define can't easily break any other define (ie. not a comma/semicolon separated list)

I don't have any particularly strong feelings in terms of UX here, as long as it's per-export-preset, easy to add or remove defines, and not error-prone.

Add some user-local editor setting to store the currently active export preset.

  • user-local as in not stored in shared project settings
  • can be null since there can be zero defined export presets

Add a button to switch the currently active export preset, which triggers updating the dotnet project config.

On editor open or some other "project files have probably changed externally" event, trigger updating of the dotnet project config.

Use Cases

Common real-world use cases include:

  • Platform-specific plugins (eg. audio, XR, input, etc.) that are conditionally compiled and/or integrated. Preprocessor defines are extremely helpful to this end (like, you could work around their absence but it'd be a PITA).
    • This is mostly covered by the pre-existing platform-specific defines, however there are situations where two "target platforms" share an OS and so distinction between them is required.
    • Real world example: disabling Steam integration for EGS or Itch.io builds for PC/Linux/MacOS.
  • In-house debug tools (on-screen debug info, cheat buttons, etc.) are often desirable to be compiled irrespective of whether a build is built with optimisations or not:
    • Makes the lives of developers (especially QA) a lot less painful to have access to debug tools in addition to roughly representative performance on device.
      • Real world example: in-game debug tools/cheats to load into a specific level and then to teleport to any given section, unlock items/powerups/whatever, and toggle on-screen stats (perf graphs, etc.). These are indispensable tools for rapidly repro'ing niche bugs, testing fixes, and optimisation in problematic parts of the game (which ofc requires release optimisations enabled to properly measure on target hardware). Some bugs only happen in release, etc.
    • When making a demo build to send to external stakeholders it's helpful to have the option of enabling a subset of debug tools for their convenience or to empower them to bypass blocking bugs when required without waiting for a new build (which may be a full day delay before they can continue testing due to time differences).

With that in mind, each project ends up with a variety of configurations per platform per OS. For each target platform (bear in mind there may be multiple target platforms per OS) there's usually a few different build profiles, each with a different configuration of debug tools, SDKs enabled, other misc feature flags enabled, etc.
Preprocessor defines are the primary and simplest way of achieving this with C#.

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

Mockup of export preset settings (pretty rough, apologies in advance 🙂)
Image
Note the addition of the "Custom Defines" list, the "Set Active" button in the bottom left, and the active preset indicator (the tick) next to the preset name in the left pane.
The add/remove buttons are just there to indicate functionality, the actual list should use whatever the equivalent built-in widget is.

Updating Defines

This is probably the trickiest/most contentious part.

If I understand correctly, following the initial solution generation the [project name].csproj Godot won't touch it and it can be manually edited by the user. In other words the user is the sole source of truth after initial generat. There are certainly advantages to this, but makes it tricky to do any sort of config management from Godot without impeding the ability to confidently edit the .csproj manually.

Transient vs Non-Transient

Personally I'm of the mind that the editor should remain the source of truth for these files, and the user should be able to configure project settings in Godot to generate a .csproj that meets their needs. In other words generated solution/project files should be transient like they are in Unreal, Unity, etc.
I'm still new to this engine so I'm not familiar with the background/history of this decision. Are there compelling reasons for these files not being transient, beyond externalising responsibility for dotnet configuration files to external tools and thus reducing the development/maintenance burden on the editor?

Having said that, this is probably just my bias from other engines/workflows showing through. I can't think of a genuinely compelling reason to make them transient after considering the implementation I've documented below. I'm still curious to learn the rationale though.

Implementing Generated Custom Defines

We could generate a transient [project].generated.props file containing generated project properties, and import that into the main [project].csproj.

Example MyProject.generated.props:

<!-- GENERATED FILE - DO NOT EDIT MANUALLY -->
<Project>
  <PropertyGroup>
    <GodotCustomDefineConstants>ENABLE_STEAMWORKS</GodotCustomDefineConstants>
  </PropertyGroup>
</Project>

Example MyProject.csproj file:

<Project Sdk="Godot.NET.Sdk/4.3.0">
  <Import Condition="Exists('$(MSBuildProjectDirectory)\$(MSBuildProjectName).generated.props')" Project="$(MSBuildProjectDirectory)\$(MSBuildProjectName).generated.props" />
  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <TargetFramework Condition=" '$(GodotTargetPlatform)' == 'android' ">net7.0</TargetFramework>
    <TargetFramework Condition=" '$(GodotTargetPlatform)' == 'ios' ">net8.0</TargetFramework>
    <EnableDynamicLoading>true</EnableDynamicLoading>
    <DefineConstants>$(DefineConstants);$(GodotCustomDefineConstants);MANUAL_DEFINE_FOOBAR</DefineConstants>
  </PropertyGroup>
</Project>

I've done a test of this setup locally and it seems to work nicely!

This allows users to freely edit the contents of their .csproj, including directly editing the DefineConstants property, without fear that their changes will be stomped.

Upon the active export preset being changed we regenerate [project].generated.props, stomping any existing contents. This also happens on editor open to account for changes to export_presets.cfg via VCS/external tools.

VCS Considerations

Due to the transient nature of the generated file, *.generated.props should be ignored by the VCS. The example MyProject.csproj above accounts for this by checking for the existance of the generated .props file before importing it. This allows the opening of the solution in a fresh clone/checkout without first opening the Godot editor (and triggering the .props generation).

For the git integration, the .gitignore template should have *.generated.props added to it. Any other VCS integration should be similarly updated.

As for why it should be ignored: any generated file that is subject to user-local config will suffer from what I've dubbed "constantly f***ing changing" syndrome when tracked by VCS in a team environment. If you've ever been "the git/plastic/svn person" in the office, you probably have your own name for this.
For example: if I checkout the last commit from my team member who has a different active export preset than me (because they're currently working on the console port and I'm working on a PC bug, for example) then as soon as I open the editor the contents of [project].generated.props will be updated to reflect the custom defines of my active export preset. Those changes will then show up in the changelist of my VCS client.

If you've worked in a large enough team for long enough, you'll know this is a real PITA. Seems common with certain popular Unity plugins that produce indeterministic or user-specific configuration files, which must be tracked or problems abound. It (understandably) causes confusion for most developers (all disciplines) when random files they don't remember touching show up in the changelist when they go to checkin their work. In my experience most people will err on the side of caution and commit those files. These files are inevitably the cause of endless spurious conflicts, this wastes time which adds up very quickly: 2 minutes per day per developer in a team of 20 fulltime developers comes out to about 80 wasted hours over a 6 month period. It's also just really annoying to have them constantly present in your VCS client.

Most VCS solutions have a way to track a file but ignore local changes to it. Unfortunately for git it is... inelegant, not stored in shared repo config, and requires a specific command be run on each user's workstation (git update-index --assume-unchanged FILE_PATH). The editor could conceivably install Godot-specific hook scripts, I've written unity plugins that do something similar for project-specific git hooks, but it's probably beyond the scope of the official git plugin to handle this.

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

It would be used anywhere from zero times per day to dozens of times per day.
There is no workaround that isn't a significant time & cost sink and error prone.

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

Imo this clearly belongs in core. Having strong build target/export preset configuration options is an essential tool in any professional-grade engine (which Godot clearly is), it shouldn't fall to 3rd party tools to fill that gap.

An editor plugin could be used to workaround this by providing a custom build config UI to the existing export preset config data while augmenting it. However at that point it seems clear that it should just be part of the export presets.

EditorExportPlugin

At first glance this seems to be a good fit for EditorExportPlugin, by editing project config files in _export_begin.

However the API for EditorExportPlugin doesn't seem to have a way to get the ID/name of the export preset used to trigger the export, only the underlying OS platform. As discussed above, this is insufficient differentiation in some very common situations that require multiple export presets per "target platform", and multiple "target platforms" per "OS platform".

@Grimeh
Copy link
Author

Grimeh commented Dec 30, 2024

Apologies for the very large issue!
I went back and forth a couple times on how it should be implemented while writing this and ended up with something that's probably much more verbose than it needs to be.

I'll probably take a stab at implementing this and open a PR soon, assuming no one else jumps on it faster.

@raulsntos
Copy link
Member

@Grimeh
Copy link
Author

Grimeh commented Dec 31, 2024

Thanks for the link! Must have been tired last night because now I can see a few more related issues/PRs 😅

That PR would certainly help, at the very least offers a per-export-preset option for custom defines. Unfortunately it doesn't solve the problem that those defines aren't present in the project config and IDEs can't pick up on them, they're only available at build-time. That alone makes it a non-starter for me. At the very least I'd say it justifies the implementation of most of this feature (*.generated.props, active export preset).

Additionally, relying on feature defines is not flexible enough. Being forced into a GODOT_FEATURE_* prefix would cause issues with tooling and dependencies that have pre-established defines. For example Steamworks.NET has a few for platform support.

If you have source access for all of your dependencies (which is sadly rare) this could be worked around, however it would be an extra time sink on initial integration and maintenance cost when you need to upgrade your dependencies.

Theoretically it's possible to expose an option in the features list to allow a non-prefixed define for specific features, but that's probably out of scope of that PR and would unnecessarily complicate an otherwise very focused feature. I think we should avoid conflating engine features with custom defines.

TL;DR: I still think a dedicated "custom defines" section and active export preset is required. We should have both feature defines and custom defines.

@Grimeh
Copy link
Author

Grimeh commented Dec 31, 2024

Have updated issue to improve formatting/organisation and mention the PR linked by @raulsntos.

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

2 participants