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

Unify particle editors #94371

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jul 14, 2024

Unifies all particle editors to use single base class and remove code duplication.

@KoBeWi KoBeWi added this to the 4.x milestone Jul 14, 2024
@KoBeWi KoBeWi force-pushed the insane_particle_unification_ branch 2 times, most recently from f69f01a to 6c4336f Compare July 21, 2024 22:57
@KoBeWi KoBeWi marked this pull request as ready for review July 21, 2024 22:57
@KoBeWi KoBeWi requested review from a team as code owners July 21, 2024 22:57
@@ -1465,6 +1465,7 @@ void CPUParticles3D::_bind_methods() {
ClassDB::bind_method(D_METHOD("get_mesh"), &CPUParticles3D::get_mesh);

ClassDB::bind_method(D_METHOD("restart"), &CPUParticles3D::restart);
ClassDB::bind_method(D_METHOD("capture_aabb"), &CPUParticles3D::capture_aabb);
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason CPU particles did not expose this method, while GPU did.
I needed it to generate AABB.

Copy link
Contributor

Choose a reason for hiding this comment

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

But what about the 2D equivalent?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only GPU particles allow generating rect, so the code could handle a specific class. In 3D, since both types can generate AABB, I just used call().

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay then. If I could choose I'd rather have this exposed in another PR to keep it clean but it's also not that big of a deal.

@KoBeWi KoBeWi force-pushed the insane_particle_unification_ branch 3 times, most recently from 0b15aec to 28f8738 Compare July 21, 2024 23:41
@KoBeWi KoBeWi force-pushed the insane_particle_unification_ branch from 28f8738 to b00a7b0 Compare September 8, 2024 10:34
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.

Needs rebase, but then I'd say yolo?

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Oct 4, 2024
@KoBeWi KoBeWi force-pushed the insane_particle_unification_ branch from b00a7b0 to a4d724b Compare October 4, 2024 20:36
@KoBeWi KoBeWi requested a review from a team as a code owner October 4, 2024 20:36
@Repiteo Repiteo merged commit 5c00d0a into godotengine:master Oct 14, 2024
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 14, 2024

Thanks!

@KoBeWi KoBeWi deleted the insane_particle_unification_😱 branch October 14, 2024 19:26
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