Allow for platform Thread implementation override

[lucypero]

EDIT: Implements godotengine/godot-proposals#3277.

[RandomShaper]

Looks good to me, besides the formatting issues.

[lucypero]

Fixed formatting and added comment.

[Listwon]

@akien-mga isn't this PR against the contribution best practices? Reduz mentioned them recently on Twitter https://docs.godotengine.org/en/stable/community/contributing/best_practices_for_engine_contributors.html

These changes are useful only in very special cases like console porting. Dangling #include "platform_thread.h" without any stub is a bit misleading. If you are porting engine to special platform, you can handle these tiny additions on your private repository.

[akien-mga]

Seems good to me, please squash the commits into one after final edits (see PR workflow for instructions).

[akien-mga]

These changes are useful only in very special cases like console porting. Dangling #include "platform_thread.h" without any stub is a bit misleading. If you are porting engine to special platform, you can handle these tiny additions on your private repository.

I don't think console porting is a "very special case". It's something a significant number of users need to do or get access to to publish their games on consoles. Adding a documented opt-in preprocessor define for that doesn't strike me as particularly problematic.

For console porters, or people porting to some niche platforms, that means that they don't need to do a hard fork of the engine to add a platform port (instead, they can just drop their platform code in the platforms folder and compile). Whether that's possible in practice, I don't know, but that's how at least some porters used to work significantly in the past.

I don't think this PR goes against the contribution best practices. It doesn't modify everyone's Thread implementation for a niche use case - it adds an entry point to allow defining a custom one, which is not even compiled if not defined.

[Listwon]

In that case "platform_thread.h" doesn't make sense anyway, should be stubbed somehow with some informative comments, thread class stubs, asserts, defines. Previously it was virtual class implemented per platform and now it's #include of nonexistent header in root path.

[reduz]

To clarify from my side. The general policy is to integrate what is needed by the community, this includes companies using the engine, even those that need this to make ports to consoles easier to maintain.

If something is not wanted by the community, or it is not possible to add it due to legal reasons (like this case), at least the project will always have a compromise to make hooks available for this to happen and, thus, minimize the amount of forked codebase needed.

[reduz]

I would still add a comment in that line of code that reads like "Overriding the platform implementation is required in some proprietary platforms".

[akien-mga]

In that case "platform_thread.h" doesn't make sense anyway, should be stubbed somehow with some informative comments, thread class stubs, asserts, defines. Previously it was virtual class implemented per platform and now it's #include of nonexistent header in root path.

I don't think it's necessary to make a stub for this. Custom implementation just need to match the interface of core/os/thread.h which they replace.

However, it's true that enforcing a "platform_thread.h" header which may or may not be in the include path is not the most convenient.
It could be #include PLATFORM_CUSTOM_THREAD_H which can be #define'd to the platform's relevant file path in platform_config.h.

[reduz]

Agreed, just a single PLATFORM_CUSTOM_THREAD_H define that points to the include is probably enough.

[lucypero]

Replaced PLATFORM_THREAD_OVERRIDE with PLATFORM_CUSTOM_THREAD_H

[akien-mga]

Looks great. Just remember to update the #endif // PLATFORM_THREAD_OVERRIDE comments in both files, and squash the commits.

[lucypero]

Looks great. Just remember to update the #endif // PLATFORM_THREAD_OVERRIDE comments in both files, and squash the commits.

Done.

[akien-mga]

Thanks!

[akien-mga]

Cherry-picked for 3.4.