-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
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
4afdc64
to
df15d63
Compare
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 |
df15d63
to
e05add9
Compare
e05add9
to
a848ffb
Compare
a848ffb
to
71985f3
Compare
71985f3
to
d2e8af4
Compare
d2e8af4
to
d386e58
Compare
d386e58
to
7b453a1
Compare
7b453a1
to
a541e97
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 >= 0x040300
checks, though that implies includingcore/version.h
in 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).
a541e97
to
8914109
Compare
8914109
to
eca3149
Compare
eca3149
to
c399424
Compare
Thanks for the work and your patience :) |
Similarly to how we have the
scene/2d
folder andscene/3d
folder for 2D and 3D nodes, this PR moves 2D and 3D resources toscene/resources/2d
andscene/resources/3d
. Also, I moved the skeleton modification resources to another subfolder,scene/resources/2d/skeleton
since 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: