Move 2D and 3D resources to their own folders#50148
Conversation
59f853d to
bf25e9b
Compare
bf25e9b to
16baba4
Compare
6e7e4bd to
ded4d23
Compare
ded4d23 to
85c7751
Compare
|
SurfaceTool and MeshDataTool should be under /resources rather than /resources/3d. Both can be used in 2D as well as 3D. For example, they can be used with a MeshInstance2D or a MultiMeshInstance2D. |
f235ec8 to
c9c7dd3
Compare
c467e7f to
23b29e1
Compare
fd43597 to
38f7ec2
Compare
38f7ec2 to
2fcb90e
Compare
8658ce2 to
d50b5e6
Compare
135e4d8 to
a60b104
Compare
0756f67 to
9dca919
Compare
7d5a7c8 to
1854080
Compare
3c08ee2 to
f99dc0b
Compare
863aa06 to
9b252fa
Compare
9b252fa to
ba45cb3
Compare
ba45cb3 to
4dc40f0
Compare
4dc40f0 to
d2ede15
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@aaronfranke do you remember how we generated those photos? Back to topic: I remember this was not a minor change to merge because it causes all the godot engine prs to break. |
|
@fire https://docs.google.com/spreadsheets/d/1aWmWiA6MWoHr4Mgg22tybDFPa6BJX5oHTSjfN21plyY The data was gathered from a debug build, I don't remember the process. Then the data was added to this chart. Since the data was gathered from a debug build it may not be 100% accurate compared to a release build but it will be very close and a release build does not have the information required to get the compiled file size. |
|
What's the status of this PR? Will it be merged by the release? |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Any update on this ? |
We have several reorganization PRs, from Aaron and some other contributors. These are low priority in general, unless they improve code organization (and not just file organization), because such changes have no effect on the API and the rest of the outer world. Generally speaking, I think it's safe to say that we like the idea of better organization, especially for huge folders like that. But there is no specific date on merging this (and it was definitely not a priority for 4.0). |
|
Awhile ago this was brought up in a meeting (should've written this down then, but I forgot), and reduz wondered if instead we should have |
akien-mga
left a comment
There was a problem hiding this comment.
After discussion with maintainers, I agree with the change.
Conceptually, this is fine, a 2D/3D split marginally helps make the code organization better, but also paves the way to more possibility to decrease build size in custom 2D/3D only builds.
My main concern is with breaking compatibility on the include paths for C++ modules which use those Godot resources.
- For modules which only track a single Godot version (like in-house C++ modules for game-specific logic, etc.), it's fine, the devs can just do the same refactoring when updating to Godot 4.3.
- For general-purpose open source modules which aim to support multiple Godot branches, it's a bit trickier. They can still wrap things with
#if VERSION_HEX >= 0x040300checks, though that implies includingcore/version.hin all affected files. Thankfully with GDExtension/godot-cpp, such modules are less common than they were in 3.x, so I think the tradeoff is acceptable (compatibility is not broken for godot-cpp as their the include paths are all flattened to a single folder).
|
Thanks for the work and your patience :) |
Similarly to how we have the
scene/2dfolder andscene/3dfolder for 2D and 3D nodes, this PR moves 2D and 3D resources toscene/resources/2dandscene/resources/3d. Also, I moved the skeleton modification resources to another subfolder,scene/resources/2d/skeletonsince there were 18 2D skeleton modification resource files. Resources that are shared between 2D and 3D are still inscene/resources.This gives us the ability to easily exclude the resources in these folders when disabling 2D or 3D (but as of this PR, they are always included).
In terms of compiled size, as of the current master, the resources folder is the biggest one, so that's another reason that I think it would be nice to divide it up into 2D and 3D subfolders: