Skip to content

Enums: add missing binds #117205

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

Closed
ScatteredComet wants to merge 1 commit into from
Closed

Enums: add missing binds #117205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/classes/LightmapGIData.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,7 @@
<constant name="SHADOWMASK_MODE_OVERLAY" value="2" enum="ShadowmaskMode">
Shadowmasking is enabled. Directional shadows will be rendered with real-time shadows overlaid on top of the shadowmask texture. This mode makes for smoother shadow transitions when the camera moves fast, at the cost of a potential smearing effect for directional shadows that are up close (due to the real-time shadow being mixed with a low-resolution shadowmask). Objects that only have shadows baked in the shadowmask (and no real-time shadows) will keep their shadows up close.
</constant>
<constant name="SHADOWMASK_MODE_ONLY" value="3" enum="ShadowmaskMode">
</constant>
</constants>
</class>
2 changes: 2 additions & 0 deletions modules/interactive_music/audio_stream_interactive.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,8 @@ void AudioStreamInteractive::_bind_methods() {

BIND_ENUM_CONSTANT(TRANSITION_TO_TIME_SAME_POSITION);
BIND_ENUM_CONSTANT(TRANSITION_TO_TIME_START);
BIND_ENUM_CONSTANT(TRANSITION_TO_TIME_PREVIOUS_POSITION);
BIND_ENUM_CONSTANT(TRANSITION_TO_TIME_MAX);

BIND_ENUM_CONSTANT(FADE_DISABLED);
BIND_ENUM_CONSTANT(FADE_IN);
Expand Down
4 changes: 4 additions & 0 deletions modules/interactive_music/doc_classes/AudioStreamInteractive.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@
<constant name="TRANSITION_TO_TIME_START" value="1" enum="TransitionToTime">
Transition to the start of the destination clip.
</constant>
<constant name="TRANSITION_TO_TIME_PREVIOUS_POSITION" value="2" enum="TransitionToTime">
</constant>
<constant name="TRANSITION_TO_TIME_MAX" value="3" enum="TransitionToTime">
</constant>
Comment on lines +202 to +205

@wjt wjt Mar 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In #114129 I added the binding for TRANSITION_TO_TIME_PREVIOUS_POSITION along with documentation. I did not bind TRANSITION_TO_TIME_MAX. The other enums in this file do not bind that; nor does e.g. AudioStreamRandomizer.PlaybackMode. It's also illegal to pass TRANSITION_TO_TIME_MAX to e.g. add_transition. So I don't think it's correct to bind it.

@ScatteredComet ScatteredComet Mar 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I've got a bit confused; I was told that XML files are all generated using --doc-tools but it seems that ..._MAX enum values are not generated through binding in the cpp file. Sorry! It would be nice in general to have more instructions on how the documentation should be contributed to because I think I'm probably hindering more than helping!

@wjt wjt Mar 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My suggestion (bearing in mind I'm just a guy who occasionally sends 1-line PRs!)

  1. Remove the interactive_music changes from this branch - it's covered in Bind AudioStreamInteractive TRANSITION_TO_TIME_PREVIOUS_POSITION #114129 which I hope will be merged at some point!
  2. Don't bind OPENXR_ACTION_MAX
  3. Write documentation for SHADOWMASK_MODE_ONLY & OPENXR_ACTION_HAPTIC

@ScatteredComet ScatteredComet Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay cool; I was also confused at why some _MAX enum values are documented and how they are being added to the XML files if they're not bound?

@AThousandShips AThousandShips Mar 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you might be missing where it's bound because they are bound if they are added to the documentation

@ScatteredComet ScatteredComet Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm still a bit confused. e.g. why is <constant name="CLOSE_BUTTON_MAX" value="3" enum="CloseButtonDisplayPolicy"> allowed to be bound, but TRANSITION_TO_TIME_MAX is not allowed to be bound here? I don't understand how to distinguish what should/shouldn't be bound.

@ScatteredComet ScatteredComet Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

By wjt's logic, I would suspect its also not valid to pass CLOSE_BUTTON_MAX (or many other of the _MAX enums found in the docs) to their respective functions, but they are still included in the XML files. Maybe I'm wrong though and there's some MAX enum values which have actual usage beyond just representing the max size?

@AThousandShips AThousandShips Mar 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the explanation:

It's also illegal to pass TRANSITION_TO_TIME_MAX to e.g. add_transition. So I don't think it's correct to bind it.

But I'd say that's up to the person implementing the feature, so doing that without understanding the feature or the logic is not a good idea, so I think this might not be the right task for you

In the cases where they are missing, I'd suggest just going back to the PR that added them, see if there's an explanation, and if not ask somewhere about it, there's usually a good idea behind that, but not always (you can use the "blame" tab on each file in the GitHub web interface to explore the history of any file)

But the enum shouldn't generally be bound unless it's actually useful to binds, a lot of the time it's strictly used internally to do range checking, like ERR_FAIL_INDEX(p_arg, 0, SPECIFIC_ENUM_MAX); (and isn't relevant to iterating over valid values, for example if there's no value for a specific entry between 0 and X_MAX)

<constant name="FADE_DISABLED" value="0" enum="FadeMode">
Do not use fade for the transition. This is useful when transitioning from a clip-end to clip-beginning, and each clip has their begin/end.
</constant>
Expand Down
2 changes: 2 additions & 0 deletions modules/openxr/action_map/openxr_action.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ void OpenXRAction::_bind_methods() {
BIND_ENUM_CONSTANT(OPENXR_ACTION_FLOAT);
BIND_ENUM_CONSTANT(OPENXR_ACTION_VECTOR2);
BIND_ENUM_CONSTANT(OPENXR_ACTION_POSE);
BIND_ENUM_CONSTANT(OPENXR_ACTION_HAPTIC);
BIND_ENUM_CONSTANT(OPENXR_ACTION_MAX);
}

Ref<OpenXRAction> OpenXRAction::new_action(const char *p_name, const char *p_localized_name, const ActionType p_action_type, const char *p_toplevel_paths) {
Expand Down
4 changes: 4 additions & 0 deletions modules/openxr/doc_classes/OpenXRAction.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,9 @@
</constant>
<constant name="OPENXR_ACTION_POSE" value="3" enum="ActionType">
</constant>
<constant name="OPENXR_ACTION_HAPTIC" value="4" enum="ActionType">
</constant>
<constant name="OPENXR_ACTION_MAX" value="5" enum="ActionType">
</constant>
</constants>
</class>
1 change: 1 addition & 0 deletions scene/3d/lightmap_gi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ void LightmapGIData::_bind_methods() {
BIND_ENUM_CONSTANT(SHADOWMASK_MODE_NONE);
BIND_ENUM_CONSTANT(SHADOWMASK_MODE_REPLACE);
BIND_ENUM_CONSTANT(SHADOWMASK_MODE_OVERLAY);
BIND_ENUM_CONSTANT(SHADOWMASK_MODE_ONLY);
}

LightmapGIData::LightmapGIData() {
Expand Down