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

Expose finding valid focus neighbors of a Control by side #76027

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Apr 13, 2023

Exposes the functionality that is used for keyboard/joystick navigation to scripting, made it a constant function to match the other find_ functions

Named it find_valid_focus_neighbor as get_focus_neighbor was already taken and find_valid_ conveys the functionality better I feel and matches find_next_valid/prev_focus, find_next_valid_focus_neighbor feels a bit too long but think this is a good balance

Closes godotengine/godot-proposals#6690

@AThousandShips AThousandShips requested review from a team as code owners April 13, 2023 12:46
@YuriSizov YuriSizov added this to the 4.x milestone Apr 13, 2023
@AThousandShips AThousandShips changed the title Expose getting the focus neighbor of a Control Expose finding valid focus neighbors of a Control by direction Apr 13, 2023
@AThousandShips AThousandShips changed the title Expose finding valid focus neighbors of a Control by direction Expose finding valid focus neighbors of a Control by side Apr 13, 2023
@MewPurPur
Copy link
Contributor

There exists find_next_valid_focus(), so if what you're doing is similar, this should probably have a similar name

@AThousandShips
Copy link
Member Author

It's very similar, so will rename it as per my considered name

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Looks fine, but we should probably better document the relation between this method and get_focus_neighbor. They should be cross-linked with an appropriate note where and when to use each. Especially the behavior of the new method is worth explaining in more detail.

@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Aug 24, 2023
@AThousandShips
Copy link
Member Author

AThousandShips commented Aug 24, 2023

Will do! Unsure about the exact wording

Exposes the functionality used for ui navigation
@AThousandShips
Copy link
Member Author

Not exactly sure about the wording but added some basic details

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.

Seems ok to me.

@akien-mga akien-mga merged commit 3408aab into godotengine:master Sep 25, 2023
16 checks passed
@AThousandShips AThousandShips deleted the focus_direction branch September 25, 2023 15:21
@@ -526,6 +526,7 @@ class Control : public CanvasItem {

Control *find_next_valid_focus() const;
Control *find_prev_valid_focus() const;
Control *find_valid_focus_neighbor(Side p_size) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops 🙃

Suggested change
Control *find_valid_focus_neighbor(Side p_size) const;
Control *find_valid_focus_neighbor(Side p_side) const;

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh dang 🙃

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.

Expose a method to get the next control to be focused based on direction of the input
4 participants