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

Initialize class variables with default values in scene/ [1/2] #44630

Merged
merged 1 commit into from
Feb 8, 2021

Conversation

qarmin
Copy link
Contributor

@qarmin qarmin commented Dec 23, 2020

Part of #43636
Fixes in 4.0 #35877

Also moved initialization of data from constructors.

@qarmin qarmin marked this pull request as draft December 28, 2020 12:26
@qarmin qarmin marked this pull request as ready for review December 28, 2020 15:30
@Calinou Calinou added this to the 4.0 milestone Jan 5, 2021
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

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

Physics classes look fine (I haven't checked other classes).

scene/2d/animated_sprite_2d.h Outdated Show resolved Hide resolved
scene/2d/animated_sprite_2d.h Outdated Show resolved Hide resolved
scene/2d/area_2d.h Outdated Show resolved Hide resolved
scene/2d/audio_stream_player_2d.h Outdated Show resolved Hide resolved
scene/2d/audio_stream_player_2d.h Outdated Show resolved Hide resolved
scene/2d/joints_2d.h Outdated Show resolved Hide resolved
scene/2d/parallax_layer.h Outdated Show resolved Hide resolved
scene/2d/parallax_layer.h Show resolved Hide resolved
scene/3d/camera_3d.h Outdated Show resolved Hide resolved
scene/3d/camera_3d.cpp Show resolved Hide resolved
scene/3d/skeleton_3d.h Outdated Show resolved Hide resolved
scene/3d/vehicle_body_3d.h Outdated Show resolved Hide resolved
scene/3d/vehicle_body_3d.h Outdated Show resolved Hide resolved
scene/3d/vehicle_body_3d.cpp Outdated Show resolved Hide resolved
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 great overall, thanks for the huge amount of work (I did the same for core and I know how tedious it is :D).

Found a couple bugs introduced by unwillingly changing the default value of some properties.

I initially commented about floats that should have .0 to make their type clear, but then stopped at some point as there's a ton of them. Would still be worth changing IMO, possibly by grepping the git show for (float|real_t) .* = .

scene/animation/animation_blend_tree.h Outdated Show resolved Hide resolved
bool zero_y;
float cell_size = 1;
float radius = 10;
bool use_in_game = false;
Copy link
Member

Choose a reason for hiding this comment

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

Unused, can be removed.

@akien-mga akien-mga merged commit cea42fa into godotengine:master Feb 8, 2021
@akien-mga
Copy link
Member

Thanks! Amazing work!

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.

4 participants