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.

Sign up for GitHub

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

Jump to bottom

[3.x] Add Color + alpha constructor for Color #74973

Merged
merged 1 commit into from
Apr 11, 2023
Merged

[3.x] Add Color + alpha constructor for Color #74973

merged 1 commit into from
Apr 11, 2023
+14 −5

Conversation

timothyqiu
Copy link
Member

Closes godotengine/godot-proposals#6492

Note: the existing Color_init3 and Color_init4 seems unused.

@timothyqiu timothyqiu added this to the 3.6 milestone Mar 16, 2023
@timothyqiu timothyqiu requested review from a team as code owners March 16, 2023 06:57
lawnjelly
lawnjelly reviewed Mar 19, 2023
View reviewed changes
core/variant_call.cpp
}

static void Color_init4(Variant &r_ret, const Variant **p_args) {
r_ret = Color::hex(*p_args[0]);
Copy link
Member

Choose a reason for hiding this comment

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

Were these previous functions unused? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Not used since the "GODOT IS OPEN SOURCE" commit.

lawnjelly
lawnjelly approved these changes Mar 19, 2023
View reviewed changes
Copy link
Member

@lawnjelly lawnjelly left a comment

Choose a reason for hiding this comment

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

This looks fine to me (although I'm not super well versed in variant_call.cpp).

@lawnjelly
Copy link
Member

Just to note that if we merge this PR, we should ideally have a later PR to change the calling sites mentioned in godotengine/godot-proposals#6492 (comment) .

@timothyqiu
Copy link
Member Author

C++ code is not affected, this only creates a Variant color constructor for scripts.

@akien-mga akien-mga merged commit 0facd88 into godotengine:3.x Apr 11, 2023
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga mentioned this pull request Apr 11, 2023
Closed
@timothyqiu timothyqiu deleted the a-color branch April 11, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga approved these changes

@lawnjelly lawnjelly lawnjelly approved these changes

Assignees
No one assigned
Labels
enhancement topic:core
Projects
None yet
Milestone
3.6
Development

Successfully merging this pull request may close these issues.
3 participants
@timothyqiu @lawnjelly @akien-mga