C#: Let platforms signal if they support the mono module or not #88245
Conversation
|
Since the platform support is usually implemented in the .NET module, I feel like it makes more sense for the .NET module to say if it supports it or not. But I agree that for your use case it may make sense to be able to signal support from your end since you're the one implemented that support, I don't know how that looks like though so I'm not sure I can come up with alternative suggestions. That said we already have .NET-specific code in the platform export plugins to detect unsupported configuration (e.g.: Android 32-bit architectures) so, since we're already "leaking", I'm not strongly against it. However, if we add this, I do think it'd be preferable to be a "supported" flag rather than "unsupported" so that other platforms that know nothing about .NET don't accidentally signal support, if that makes sense. |
Yeah, it's a tough one. When all the platforms and modules are in one single monolithic codebase, it's easy to have that responsibility on the module side. But when we're adding platforms out of source, it suddenly just doesn't scale to have a ton of local patches saying "oh and this platform too" scattered everywhere outside platform folders. I'm trying to move these "if platform == X" checks that we have everywhere, where the module/driver/etc is the one making the decision, to a model where the platform signals what it supports (via flags or defines or similar generic checks). It will be much easier to add new platforms, or variants of existing platforms, when we're no longer hardcoding platform names.
Yeah, I was going back and forth on that, whether to add a supported flag to all of them minus web, or this. I can do the supported flag, it makes sense. |
Instead of hardcoding platform names that support C#, let platforms set a flag indicating if they support it. All public platforms except web already support it, and it's a pain to maintain a patch for this list just to add additional names of proprietary console platforms. This makes adding new platforms or variants or existing platforms much easier, as the platform can signal what it supports/doesn't support directly, and we can avoid harcoding platform names.
shana
force-pushed
the
simplify-mono-support-detection
branch
from
30272a0
to
21e524a
Compare
|
I've updated this to use a Note: this will, of course, not solve the whole problem of letting platforms control their mono support, because we still hardcode dynamic library extensions in gd_mono.cpp and harcoded platform_ENABLED ifdefs, but I do have some other upcoming patches to make those bits more flexible for new platforms 🙂 |
shana
changed the title
That's the first thing that popped to mind when I've seen this PR. My take would be, in true Godot style, to not try to predict the future and just stick with the simple After all we're not working with "user-space" APIs here ;) Edit: I just reviewed the thing and I can see that it didn't take much to make the thing "generic" (I thought that it would've been more complex). It's perfectly fine like this :D |
|
Thanks! |
Instead of hardcoding platform names that support C#, let platforms set a flag indicating if they support it or not. All public platforms except web already support it, and it's a pain to maintain a patch for this list just to add additional names of proprietary console platforms, or when adding new platforms in general.
It will make it much easier to add new platforms or variants of existing platforms if we prefer having the platform signal what it supports instead of having hardcoded "if env.platform == X" checks. Platform flag checking scales much better in the long run and lets us keep both platforms and modules more clean and portable.
/cc @akien-mga @raulsntos @mhilbrunner
Contrbuted by W4Games ❤️