Skip to content
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

Split monolithic physics class files #88862

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

smix8
Copy link
Contributor

@smix8 smix8 commented Feb 26, 2024

Splits monolithic physics class files.

This PR moves things around a lot, but what this PR primarly does is to split the monolithic physics class files into subfiles. Something that was imo overdue for a very long time. E.g. physics body was harboring all the rigid, static, character, and what-not bodies. This made the file already lag in editors and resulted in all kind of functionality that only needed a single physics base class to include near everything physics has to offer.

All the main physics classes are now in their own file in a scene/2d_or_3d/physics subfolder.
Same with bone joints, each bone joint now has its own file in a joints subfolder.
Main means the class is primarily or exclusively used with or for physics.
E.g. raycast, shape cast, physics bones, collision shapes, objects and bodies.

A few random includes that were discovered as no longer needed are also removed.
E.g. the NavigationObstacle no longer uses physics so no need to include physics classes.

@smix8 smix8 added this to the 4.x milestone Feb 26, 2024
@smix8 smix8 requested review from a team as code owners February 26, 2024 11:46
@smix8 smix8 force-pushed the split_physics_classes branch 5 times, most recently from b086523 to 47ff830 Compare February 26, 2024 13:48
@fire
Copy link
Member

fire commented Feb 26, 2024

It is very hard to review these changes. Especially when the github web interface freezes on my computer. The typical strategy is make sure the tests pass and let the testing go to the wider community.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha. I switched from Firefox to Safari and it loaded. See previous comment. Assuming that there is no functionality change.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Feb 27, 2024
@akien-mga
Copy link
Member

You'll need to add the new folders to generate_scu_files in scu_builders.py for the SCU builds, as it's currently a manual listing of suitable folders.

@lawnjelly Is this something we could worth towards automating? Are there any reasons not to process all folders recursively (at least within specific top level folders) for SCU?

@lawnjelly
Copy link
Member

@lawnjelly Is this something we could worth towards automating? Are there any reasons not to process all folders recursively (at least within specific top level folders) for SCU?

We could potentially have the option to process_folder_recursive(). 🤔

I think I erred on the side of caution here, and also control. (It's likely I tried it during development and wasn't convinced by the benefits at the time.)

There's folders such as editor/icons, translations, __pycache__ etc, which make no sense to process every time you hit compile, just in case they contain cpps. There also may be some folders that are not included currently because they pollute the global namespace and aren't worth it in terms of costs / benefits of fixing the source code (especially third party stuff which we want to be able to update easily).

Currently it doesn't break when you add new folders, it just doesn't accelerate them until they are added manually to scu_builders.py. So accelerating them would be fine for a followup PR too.

Splits monolithic physics class files.
@smix8 smix8 requested a review from a team as a code owner February 27, 2024 10:18
@smix8
Copy link
Contributor Author

smix8 commented Feb 27, 2024

Added the physics and joints folders to the scu script and fixed the gltf includes.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, and compiles fine. A quick test shows the GDQuest TPS demo with 3D physics seems to work.

@akien-mga akien-mga merged commit 21e3b21 into godotengine:master Feb 27, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@smix8 smix8 deleted the split_physics_classes branch February 27, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants