I think this is generally desirable but my concern is the impact of ease of contribution in general without these files, so I's say they server a sufficient purpose if they don't contribute significantly to slow down compilation etc.

For example if the headers are moved and there's an open PR adding something to this the rebase will be harder, if the file exists and is just moved then the change will be largely automatic in git assuming you haven't made too large a change

The register_types.cpp files are somewhat of a standard in modules. Not sure if they should be removed.

But the ones I wanted to remove here do absolutely nothing.
And I find it weird that they have to exist, even if they are completely empty. (I left copyright header and include, but they aren't aren't needed)

IMO, given that it's "just" 17 files out of thousands, I'd say it's not worth having a case-by-case basis for this, and we should just have the cpp files, at least for all bound classes, it's a drop in the ocean comparatively and I doubt it has any real effect on anything performance wise, but it makes some contribution less smooth

Also in some cases I'd say there's a benefit of having the static variables stored in cpp to avoid unnecessary mutation of the header

Also repeating with timothyqiu said in dev-chat:

A downside might be that some contributors will tend to add method definitions to the header because they think it's intended to be header-only.

For example if the headers are moved and there's an open PR adding something to this the rebase will be harder, if the file exists and is just moved then the change will be largely automatic in git assuming you haven't made too large a change

I guess you can check if there are open PRs with PR's by file. Still doesn't solve the issue of future PRs needing these files though.

I initially thought that these cpp files only contained empty constructors and static member variable definitions. But it turns out that there are quite a few classes in this PR that have moved their method definitions to the header, thus eliminating the need for the cpp files.

I think we should discuss the circumstances under which it is permissible to define functions in header files separately.

Ok so here's a summary of the removed files, sorted from least to most arguable removal:

servers/rendering/rendering_method.cpp - Empty constructor and destructor.
core/templates/rid_owner.cpp - Single static variable definition.
servers/rendering/rendering_server_globals.cpp - Multiple static variable definitions.
modules/godot_physics_2d/godot_broad_phase_2d.cpp/3d.cpp - Single static variable definition and an empty constructor.
scene/3d/lightmap_probe.cpp - Single empty method.
modules/freetype/register_types.cpp and modules/msdfgen/register_types.cpp - 2 effectively empty methods.
core/templates/command_queue_mt.cpp - Empty destructor and a one-line constructor that initializes single variable.
servers/xr/xr_controller_tracker.cpp - Empty method and a one-line constructor that initializes single variable.
servers/rendering/dummy/storage/texture_storage.cpp - One static variable definition and 2 trivial one-liner methods.
platform/web/ip_web.cpp - 3 empty methods and 2 trivial one-liner methods.
platform/web/net_socket_web.cpp - 2 trivial one-liner methods.
core/math/static_raycaster.cpp - Single static variable definition and one relatively simple method.

By "trivial one-liners" I mean methods that only assign or return a member. Many setters/getters fall under this group; I once brought it up on chat. I don't plan to do a cleanup for this, but we could apply this style going forward (if it's generally agreed).

And yes, in this PR it's a bit forced for the sake of removing files, hence some of the changes are more arguable. Though IMO aside from the static_raycaster.cpp change (which I can revert), and the lightmap_probe.cpp which is being changed for the first time in 5 years, other changes are fine.

Also to give more context, here are some files that could be considered for removal, but I didn't touch them various reasons:
https://github.com/godotengine/godot/blob/master/core/io/net_socket.cpp - This is similar to static_raycaster case, except the method includes an error, making it more "complex".
https://github.com/godotengine/godot/blob/master/core/math/rect2i.cpp - Two very simple methods, but the file has additional includes.
https://github.com/godotengine/godot/blob/master/core/os/thread_safe.cpp - Contains a cpp-only variable.
https://github.com/godotengine/godot/blob/master/core/templates/a_hash_map.cpp - Has additional include.
https://github.com/godotengine/godot/blob/master/core/templates/interpolated_property.cpp - Example of multiple non-trivial one-liner methods.
https://github.com/godotengine/godot/blob/master/misc/dist/ios_xcode/godot_ios/dummy.cpp - I assume it exists for a reason (though looks like something that can be generated?).

The only hard-requirements for modules are register_types.h, SCsub, and config.py:

register_types.cpp being "required" was a consequence of the buildsystem assigning a library to a module unconditionally. By only doing this when build objects exist, header-only modules can indeed be a thing. This is implemented here:

• SCons: Support header-only modules #108616

And what about the Web fail? It looks like another case besides the modules.

And what about the Web fail? It looks like another case besides the modules.

Looks like web hardcodes the .cpp files it compiles instead of globbing; should be fixed by removing these two lines:

Module fix merged; this should be good to go after a rebase!

Thanks!